-
Notifications
You must be signed in to change notification settings - Fork 148
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
mark vinyl-fs as non-flaky #444
Comments
We need to run tests and make sure it passes.
Will dig into this after I get back to computer
On Jun 21, 2017 2:22 PM, "Matteo Collina" <notifications@github.com> wrote:
We recently discovered a regression on streams (nodejs/node#13850
<nodejs/node#13850>) in the 8 line (and
readable-stream 2.3.1), that we would have caught if this module was added.
What can we do to mark it as non-flaky anymore?
cc @phated <https://github.com/phated>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#444>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAecV3OSLDwbyfe8fVOo9RIP0-aZxlaDks5sGYmYgaJpZM4OBiTs>
.
|
I have a vague memory of something thrashing with our setup/teardown because we have certain tests that rely on permissions |
Yeah, if the test suite is trying anything outside of the temp directory it
is going to be no bueno. These executors are shared in ci
…On Jun 21, 2017 2:33 PM, "Blaine Bublitz" ***@***.***> wrote:
I have a vague memory of something thrashing with our setup/teardown
because we have certain tests that rely on permissions
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#444 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAecV8kkOwaa7J8ySMzz3MkJ4qM7l2ejks5sGYw0gaJpZM4OBiTs>
.
|
We generate constants for all our paths in https://github.com/gulpjs/vinyl-fs/blob/master/test/utils/test-constants.js - maybe we can add a conditional to work with temp on citgm? |
What would be the benefit of not always doing this in a tempdir? I'm not
sure how we could easily let you know the suite is running on citgm
…On Jun 21, 2017 2:43 PM, "Blaine Bublitz" ***@***.***> wrote:
We generate constants for all our paths in https://github.com/gulpjs/
vinyl-fs/blob/master/test/utils/test-constants.js - maybe we can add a
conditional to work with temp on citgm?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#444 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAecV-ncS4CnajpjRq36Mg4hknVxcBNVks5sGY6DgaJpZM4OBiTs>
.
|
@MylesBorins we can always use environment variables in the |
I'm probably missing something, but all those paths look like they're relative paths pointing to folders inside the test directory. Which specific paths are the issue? |
It looks like there was also an issue with drifting timestamps but a user did further research and thinks it has something to do with node versions. See gulpjs/vinyl-fs#208 (comment) for more info. |
The drifting timestamp thing should be fixed in our master. Guessing fs stuff still needs to be looked into? |
We recently discovered a regression on streams (nodejs/node#13850) in the 8 line (and readable-stream 2.3.1), that we would have caught if this module was added.
What can we do to mark it as non-flaky anymore?
cc @phated
The text was updated successfully, but these errors were encountered: