-
-
Notifications
You must be signed in to change notification settings - Fork 32
Record start time of last started tasks #17
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
Conversation
020c16b
to
e2c80a9
Compare
The files used to compare to lastRun time might get updated during the execution of a task. Instead of recording the stop time, Undertaker should record the start time, before something like gulp.src start to read the content of a file.
e2c80a9
to
0ce3ed9
Compare
expect(taker.lastRun('test1')).to.be.equal(since); | ||
|
||
done(); | ||
}); |
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.
That last test case in only meant to get 100% coverage (there must be a start event matching a stop or error event).
Maybe I should not test if _lastRuns[task]
is set in the error and stop handler instead.
Looking at the integration test that fails on io.js:
|
36fcc39
to
0ea2c40
Compare
@dinoboff looks like you got it fixed, what was the problem? |
0ea2c40
to
f9e8d8a
Compare
f9e8d8a
to
3144540
Compare
@phated I changed the test to check that if a source file changes, using the I think files' mtime and node time (or the time the extension
Note that the file get modified after 12:10:05.484Z, but its mtime is 12:10:05.000Z. Seems be the same for travis node 0.10 On travis nodejs 0.12, it's not as bad, but it's still inconsistent:
On Travis iojs the task start time and the file mtime are the same. |
Reading around:
I don't know if the issue with node 0.12 (mtime < start task in that travis run) is due to a bug with node 0.12, with travis container or with the extension Should |
|
||
The default time resolution is `1000` on node v0.10, `0` on node 0.11+ and iojs. | ||
it can be overwritten using `UNDERTAKER_TIME_RESOLUTION` environment variable. | ||
|
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.
@phated What do 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.
I dig it. I didn't know about these cases. Thanks!
@dinoboff this PR is awesome. thanks for all the work and research put into this |
Record start time of last started tasks
Record start time of last started tasks
The files used to compare to lastRun time might get updated during the execution of a task.
Instead of recording the stop time, Undertaker should record the start time, before something like gulp.src start to read the content of a file.