CI : Move from sleep to watch exists #748
Conversation
We can move to |
|
it would be really cool!
Il 1 dic 2019, 12:01 +0100, Vincent LE GOFF <notifications@github.com>, ha scritto:
… > lgtm.
> However we should really change these tests to not have a sleep.
We can move to fs.watchfile and hook on the creation of the file? This can be added into an util func with a timeout argument. https://nodejs.org/docs/latest-v6.x/api/fs.html#fs_fs_watchfile_filename_options_listener
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Ok so don't merge this one. I'll tackle it! |
|
So i've extended the timeout for the wait but it resolves when it finds the file, so on most of platforms it would be really quicker. I've moved to I'm ok with this current approach. What about you? |
test/basic.test.js
Outdated
| reject(new Error(`${filename} was not created with a Timeout:${timeout}`)) | ||
| }, | ||
| timeout | ||
| ) |
There was a problem hiding this comment.
Would you mind removing the timer and just add a counter in the interval?
we can also move the timeout to a constant inside this function.
There was a problem hiding this comment.
I've pushed a change. I thinks it fit your review, or may i've misunderstood 🙃
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
As mentionned here #744 (comment)
Windows CI seems a bit flaky on the IO. I've extended the sleep time for
pino.destinationtests.Hope this time it will be stable on all the runs.