-
Notifications
You must be signed in to change notification settings - Fork 285
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(ethereum): refactor jest test negative test cases #3476
test(ethereum): refactor jest test negative test cases #3476
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.
@ashnashahgrover LGTM with comments
...src/test/typescript/integration/geth-contract-deploy-and-invoke-using-json-object-v1.test.ts
Outdated
Show resolved
Hide resolved
...src/test/typescript/integration/geth-contract-deploy-and-invoke-using-json-object-v1.test.ts
Outdated
Show resolved
Hide resolved
...um/src/test/typescript/integration/geth-contract-deploy-and-invoke-using-keychain-v1.test.ts
Outdated
Show resolved
Hide resolved
...um/src/test/typescript/integration/geth-contract-deploy-and-invoke-using-keychain-v1.test.ts
Outdated
Show resolved
Hide resolved
...ledger-connector-ethereum/src/test/typescript/integration/geth-invoke-web3-method-v1.test.ts
Outdated
Show resolved
Hide resolved
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.
Besides Peter's comments, LGTM!
1280003
to
f01487a
Compare
f01487a
to
032e7ae
Compare
@ashnashahgrover Please fix the lint check and then we are good to merge |
This is done @petermetz |
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 Looks like it is still failing on the CI unfortunately. Another thing: Please rebase and squash the commits instead of putting merge commits on the PR (right now it has 2 commits, it should only have the 1 which is the actual change)
Primary Changes --------------- 1. Refactored negative test case exception assertions for cactus-plugin-ledger-connector-ethereum. Removed try-catch blocks, replaced with declarations through jest-extended's own API. 2. Made comments on specific tests where the tests should fail but are actually passing and thus cannot be refactored before being investigated further. Fixes hyperledger-cacti#3475 Signed-off-by: ashnashahgrover <ashnashahgrover777@gmail.com>
76b2e41
to
c034070
Compare
Closing this due to inactivity. Happy to reopen it anytime in the future as needed. |
Commit to be reviewed
test(ethereum): refactor jest test negative test cases
Fixes #3475
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.