-
Notifications
You must be signed in to change notification settings - Fork 12.3k
Added time helper tests. #1521
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
Added time helper tests. #1521
Conversation
test/helpers/test/time.test.js
Outdated
| await time.increase(time.duration.hours(1)); | ||
|
|
||
| const end = this.start + time.duration.hours(1); | ||
| (await time.latest()).should.be.within(end - 20, end + 20); // +-20 sec tolerance |
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.
We can use closeTo. Can we define the tolerance in a file-wide constant?
Also, I'm curious, experimentally, what did you find the actual time increase to be?
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.
Didn't know about closeTo, thanks!
The time tolerance comes from the last block (and therefore the timestamp) being mined in the previous test, and the time between tests is variable (in particular, the time between ganache being set-up and the first test running may be very large).
I ended up making this more explicit by adding an advanceBlock call before calling latest(), but we should make sure the documentation explains this behavior properly.
test/helpers/test/time.test.js
Outdated
|
|
||
| describe('increaseTo', function () { | ||
| it('increases time to a time in the future', async function () { | ||
| await time.increaseTo(this.start + time.duration.hours(1)); |
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.
Why not use the end variable defined below?
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.
Because it seems I'm not very smart 😅
frangio
left a comment
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!
Part of #1078.