-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add getChainTips rpc #205
Add getChainTips rpc #205
Conversation
Deploy preview for jellyfish-defi ready! Built with commit db1724e |
size-limit report 📦
|
Codecov Report
@@ Coverage Diff @@
## main #205 +/- ##
==========================================
+ Coverage 96.14% 96.27% +0.12%
==========================================
Files 64 64
Lines 1558 1612 +54
Branches 217 217
==========================================
+ Hits 1498 1552 +54
Misses 60 60
Continue to review full report at Codecov.
|
packages/jellyfish-api-core/__tests__/category/blockchain.test.ts
Outdated
Show resolved
Hide resolved
data.status === 'invalid' || | ||
data.status === 'headers-only' || | ||
data.status === 'valid-headers' || | ||
data.status === 'valid-fork' || | ||
data.status === 'active' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use .includes
to be cleaner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise, .includes has the same problem like .toContain
Fuxing: It must always be like this, testing is like reading a narrative.
expect (actual) to be (expected)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expect(['invalid', 'headers-only', ...].includes(data.status)).toBe(true)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems similar to the code I wrote earlier.
expect(['invalid', 'headers-only', 'valid-headers', 'valid-fork', 'active']).toContain(data.status)
I followed what was taught here https://jestjs.io/docs/expect#tocontainitem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I shall use this -> expect(['invalid', 'headers-only', ...].includes(data.status)).toBe(true). Will commit and push before end of today
/kind feature
Add getChainTips rpc