Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: replacing all generic errors with
FuelError
#2549chore: replacing all generic errors with
FuelError
#2549Changes from 12 commits
ff480d1
11e16a9
cd0d4cd
5f110f4
a9fe3b9
41ddf2d
7926966
1973e6d
bc5bbaa
8527ce1
06aadf6
d5b1568
1f95d39
7928957
7548f6f
47aa686
7cc585b
8c0dbee
fa97d88
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@msensys the
expectToThrowFuelError
utility internally checks formessage
(and any other property) equality. You shouldn't be passing this as its second argument:but rather 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.
I don't know why, but the test isn't passing
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 look at here:
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 don't see any failing tests; CI is green for all workflows.
But as @nedsalk said, you should not assert the error with an Object:
fuels-ts/packages/fuel-gauge/src/predicate/predicate-configurables.test.ts
Lines 211 to 214 in 06aadf6
Instead, you should use
FuelError
, as you did on the other occurrences:fuels-ts/packages/fuel-gauge/src/predicate/predicate-configurables.test.ts
Lines 230 to 233 in 06aadf6
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 committed the change you mentioned. Take a look at CI
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.
@msensys the test is failing because of this line catching and rethrowing the error with an updated message. To fix the failing test, change the expected message in your test to
Error setting configurable constants: Predicate has no configurable constants to be set.
; you were missing theError setting configurable constants:
part. We should maybe rethink this try/catch here, but it's out of the scope of this issue. Just changing the expected message is enough.