Tests stability improvements #2362
Conversation
…oop followed by an exception
…ull folder otherwise
| @@ -41,13 +41,13 @@ export async function run( | |||
| const registry = config.registries[registryName]; | |||
| const object = rootManifests[registryName].object; | |||
|
|
|||
| for (const type of constants.DEPENDENCY_TYPES) { | |||
There was a problem hiding this comment.
This construct was crashing node on ubuntu on windows.
I know it is not a good idea to patch Yarn because of a very niche environment but I could not find a nicer workaround
There was a problem hiding this comment.
Actually this only happens only on Node 7 on ubuntu on windows.
Probably needs to be reported with Node.
There was a problem hiding this comment.
Yeah, I'll revert this change and wait for Node to be stable then
There was a problem hiding this comment.
Most likely related to nodejs/node#9419
…for-of loop followed by an exception" This reverts commit 68179d0.
| } else { | ||
| cwd = path.join( | ||
| os.tmpdir(), | ||
| `yarn-${Math.random()}`, |
There was a problem hiding this comment.
Does Node.js provide a way to call the system's native temporary directory creation function (such as mkdtemp on GNU/Linux or GetTempFileName on Windows)? The issue with creating temporary directories like this is that two different processes (for example, running different tests in parallel) could generate the same random number.
Failing that, a GUID would be better too.
There was a problem hiding this comment.
Just found that it does support it natively in Nodejs 5.10+: https://nodejs.org/api/fs.html#fs_fs_mkdtemp_prefix_options_callback
There was a problem hiding this comment.
Use fs.mkdtemp if available, rather than yarn-${Math.random()}. The Math.random() could be used as a fallback for old Node.js versions (fs.mkdtemp is only available in Node.js 5 and above).
|
Yeah, sounds reasonable. |
|
Actually nodejs/node#6142 makes me think that mdirtmp is quite useless for our case. |
…folder name collisions
|
@Daniel15, how about now? |
|
This looks good to me, as Another option is to use something like node-tmp. In any case, feel free to merge this whenever you like :) |
Summary
I am working on Ubuntu on Windows and had to fix a few issues with tests that were broken.
globaltests were failing for non root users