-
Notifications
You must be signed in to change notification settings - Fork 295
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
test(besu): refactor jest test negative test cases #3468
test(besu): refactor jest test negative test cases #3468
Conversation
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.
EDIT - Sorry, set wrong status by mistake :)
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 test you've modified is failing in the CI, please investigate
...pescript/integration/plugin-ledger-connector-besu/deploy-contract/get-record-locator.test.ts
Outdated
Show resolved
Hide resolved
f70e0f5
to
bc68d59
Compare
...pescript/integration/plugin-ledger-connector-besu/deploy-contract/get-record-locator.test.ts
Outdated
Show resolved
Hide resolved
@ashnashahgrover please fix the failing CI tests |
f4a4bda
to
6af2416
Compare
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.
@ashnashahgrover Getting closer! I see that the lint is still failing, you could most likely fix that by running the lint locally and then committing the diff it generates.
6af2416
to
21d0f78
Compare
Just did this, by running yarn run lint. |
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.
LGTM, thank you for the updates!
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.
@ashnashahgrover Looking good to me now. Tip for next time: Use the re-request review button to put it back in my review queue, otherwise it doesn't show up in my feed.
LGTM |
Changes incorporated. Approved by Peter
Primary Changes ---------------- 1. Refactored all negative test case exception assertions for cactus-plugin-ledger-connector-besu. Removed try-catch blocks, replaced with declarations through jest and jest-extended's own API. 2. Noted two tests within openapi-validation.test.ts (GetPastLogsEndpoint and GetBesuRecordEndpointV1 with empty parameters) where the status code returned should be 400 but is 200. This could be investigated in a seperate issue. Fixes hyperledger-cacti#3467 Signed-off-by: ashnashahgrover <[email protected]>
21d0f78
to
e7a8590
Compare
Commit to be reviewed
test(besu): refactor jest test negative test cases
Fixes #3467
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.