-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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 setup helper added #1482
Test setup helper added #1482
Conversation
This reverts commit a688977.
Hello again @Aniket-Engg, thanks for working on this! I wouldn't call that helper Also, keep in mind that some tests need not only the And finally, while it is true that we may be importing the same file multiple times (when also importing e.g. |
@nventuro I assume i got your words right. Have made changes accordingly, please review. |
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.
Hey @Aniket-Engg, thanks for following up with this! Left a couple comments clarifying what I meant in my previous comment, but this looks almost ready :)
@nventuro I think now this is ready to be implemented in other files. What you think? |
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.
Looks great, I think we can add this everywhere now!
Looks like the CI job wasn't triggered :/ This is rather weird, I'll look into it. |
Seems like it was related to the PR not being mergeable - after creating a merge commit it is now working again. |
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.
Awesome, thanks a lot @Aniket-Engg! At first I thought a couple BigNumber
exports were missing, but that was just because it was needed for chai-bignumber
.
Yeah, i checked each file deeply. 😉 |
Fixes #1205
I have added the helper named as
chai.js
.Used it in
expectEvent
andshouldFail
helper.Also tests who require
expectEvent
andshouldFail
helper do not need to requirechai
modules again. I tested them inOwnable
andPaymentSplitter
tests.If chai helper lives upto the expectation, I can work further on removing extra requires from other files of testing.