Skip to content
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

test: use tmpdir.refresh() in test-esm-windows.js #30997

Merged
merged 1 commit into from
Dec 18, 2019

Conversation

richardlau
Copy link
Member

Use tmpdir.refresh() in test/es-module/test-esm-windows.js so
that the temporary directory is cleaned before use and when the test
exits.

The existing test can otherwise fail if the temporary directory is polluted
before running the test:

-bash-4.2$ mkdir -p test/.tmp.0/node_modules
-bash-4.2$ ./tools/test.py es-module/test-esm-windows
=== release test-esm-windows ===
Path: es-module/test-esm-windows
(node:81631) ExperimentalWarning: The ESM module loader is experimental.
/home/users/riclau/sandbox/github/nodejs/test/common/index.js:685
const crashOnUnhandledRejection = (err) => { throw err; };
                                             ^

[Error: EEXIST: file already exists, mkdir '/home/users/riclau/sandbox/github/nodejs/test/.tmp.0/node_modules'] {
  errno: -17,
  code: 'EEXIST',
  syscall: 'mkdir',
  path: '/home/users/riclau/sandbox/github/nodejs/test/.tmp.0/node_modules'
}
Command: out/Release/node /home/users/riclau/sandbox/github/nodejs/test/es-module/test-esm-windows.js
[00:00|% 100|+   0|-   1]: Done
-bash-4.2$

and has been observed failing on the CI: https://ci.nodejs.org/job/node-test-commit-arm/28228/nodes=centos7-arm64-gcc6/consoleFull

16:33:34 not ok 48 es-module/test-esm-windows
16:33:34   ---
16:33:34   duration_ms: 0.411
16:33:34   severity: fail
16:33:34   exitcode: 1
16:33:34   stack: |-
16:33:34     (node:55274) ExperimentalWarning: The ESM module loader is experimental.
16:33:34     /home/iojs/build/workspace/node-test-commit-arm/nodes/centos7-arm64-gcc6/test/common/index.js:704
16:33:34     const crashOnUnhandledRejection = (err) => { throw err; };
16:33:34                                                  ^
16:33:34     
16:33:34     [Error: EEXIST: file already exists, mkdir '/home/iojs/node-tmp/.tmp.1/node_modules'] {
16:33:34       errno: -17,
16:33:34       code: 'EEXIST',
16:33:34       syscall: 'mkdir',
16:33:34       path: '/home/iojs/node-tmp/.tmp.1/node_modules'
16:33:34     }
16:33:34   ...
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 16, 2019
@nodejs-github-bot

This comment has been minimized.

test/es-module/test-esm-windows.js Outdated Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 17, 2019

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 17, 2019
@Trott
Copy link
Member

Trott commented Dec 18, 2019

Worthy of fast-tracking? 👍 here if you're a Collaborator and you agree. Comment if you're a Collaborator and you disagree.

@Trott Trott added the fast-track PRs that do not need to wait for 48 hours to land. label Dec 18, 2019
Use `tmpdir.refresh()` in `test/es-module/test-esm-windows.js` so
that the temporary directory is cleaned before use and when the test
exits.

PR-URL: nodejs#30997
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@richardlau richardlau merged commit e23aebc into nodejs:master Dec 18, 2019
@richardlau
Copy link
Member Author

Landed in e23aebc.

BridgeAR pushed a commit that referenced this pull request Jan 3, 2020
Use `tmpdir.refresh()` in `test/es-module/test-esm-windows.js` so
that the temporary directory is cleaned before use and when the test
exits.

PR-URL: #30997
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
MylesBorins pushed a commit that referenced this pull request Jan 12, 2020
Use `tmpdir.refresh()` in `test/es-module/test-esm-windows.js` so
that the temporary directory is cleaned before use and when the test
exits.

PR-URL: #30997
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
Use `tmpdir.refresh()` in `test/es-module/test-esm-windows.js` so
that the temporary directory is cleaned before use and when the test
exits.

PR-URL: #30997
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 12, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 12, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 15, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 18, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 21, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 21, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 24, 2020
codebytere added a commit to electron/electron that referenced this pull request Feb 24, 2020
* chore: bump node in DEPS to v12.16.0

* Fixup asar support setup patch

nodejs/node#30862

* Fixup InternalCallbackScope patch

nodejs/node#30236

* Fixup GN buildfiles patch

nodejs/node#30755

* Fixup low-level hooks patch

nodejs/node#30466

* Fixup globals require patch

nodejs/node#31643

* Fixup process stream patch

nodejs/node#30862

* Fixup js2c modification patch

nodejs/node#30755

* Fixup internal fs override patch

nodejs/node#30610

* Fixup context-aware warn patch

nodejs/node#30336

* Fixup Node.js with ltcg config

nodejs/node#29388

* Fixup oaepLabel patch

nodejs/node#30917

* Remove redundant ESM test patch

nodejs/node#30997

* Remove redundant cli flag patch

nodejs/node#30466

* Update filenames.json

* Remove macro generation in GN build files

nodejs/node#30755

* Fix some compilation errors upstream

* Add uvwasi to deps

nodejs/node#30258

* Fix BoringSSL incompatibilities

* Fixup linked module patch

nodejs/node#30274

* Add missing sources to GN uv build

libuv/libuv#2347

* Patch some uvwasi incompatibilities

* chore: bump Node.js to v12.6.1

* Remove mark_arraybuffer_as_untransferable.patch

nodejs/node#30549

* Fix uvwasi build failure on win

* Fixup --perf-prof cli option error

* Fixup early cjs module loading

* fix: initialize diagnostics properly

nodejs/node#30025

* Disable new esm syntax specs

nodejs/node#30219

* Fixup v8 weakref hook spec

nodejs/node#29874

* Fix async context timer issue

* Disable monkey-patch-main spec

It relies on nodejs/node#29777, and we don't
override prepareStackTrace.

* Disable new tls specs

nodejs/node#23188

We don't support much of TLS owing to schisms between BoringSSL and
OpenSSL.

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants