-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
chore: add complete tests around abi-contract
program
#3246
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
import { bn } from 'fuels'; | ||
import type { BNInput } from 'fuels'; | ||
|
||
export const toEqualBn = (_received: BNInput, _argument: BNInput) => { | ||
const received = bn(_received); | ||
const argument = bn(_argument); | ||
|
||
const pass = received.eq(argument); | ||
|
||
if (pass) { | ||
return { | ||
message: () => `Expected ${received.toString()} not to equal ${argument.toString()}`, | ||
pass: true, | ||
}; | ||
} | ||
return { | ||
message: () => `expected ${received.toString()} to equal ${argument.toString()}`, | ||
pass: false, | ||
}; | ||
}; |
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.
🚀
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.
@nedsalk said this wasn't going to go down well with the team
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.
The annoying (ugly) part of adding these custom assertions is usually making TS aware of them.
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.
@nedsalk said he'd sort this
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.
@arboleya are you happy with the Vitest matcher, or would you favour a standalone function?
We're experiencing a stack overflow (related Sway issue) |
* @group node | ||
*/ | ||
describe('AbiCoder', () => { | ||
let contract: AbiContract; |
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.
I'd favor creating a setupContract()
helper like here and using it in every test instead of reusing the same contract/node across all tests.
using contract = await setupContract();
There's nothing wrong with the approach you've taken here given that all these calls are read-only, but if we decide to parallelize these tests in the future we'll have to do the setupContract
approach or we'd be getting issues related to already spent coins. It's better to just have a hard rule of always spinning up a node for every test.
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.
I favour this also - just used this approach for speed of development.
Summary
abi-contract
) that taking in fixed inputs, and returns an expected value.@fuel-ts/abi-coder
.Interface
for our new refactored package with absolute certainty of no regression.toEqualBn
to make it easier to check returned BN values.Couple issues
// @ts-expect-error: Custom matcher 'toEqualBn'
) for now.Checklist