Skip to content

test: add fs.watchFile() + worker.terminate() test - #21179

Closed
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:worker-fs-watch-file
Closed

test: add fs.watchFile() + worker.terminate() test#21179
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:worker-fs-watch-file

Conversation

@addaleax

Copy link
Copy Markdown
Member

Refs: #21093 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@addaleaxaddaleax added fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests. worker Issues and PRs related to Worker support. labels Jun 6, 2018
@addaleax
addaleax requested a review from apapirovskiJune 6, 2018 21:24
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Jun 6, 2018
Comment threadtest/parallel/test-worker-fs-stat-watcher.js Outdated
@addaleax

Copy link
Copy Markdown
MemberAuthor

Getting related CI failures like https://ci.nodejs.org/job/node-test-commit-linuxone/1994/nodes=rhel72-s390x/testReport/junit/(root)/test/parallel_test_worker_fs_stat_watcher/:

out/Release/node[29153]: ../src/node_worker.cc:307:virtualnode::worker::Worker::~Worker(): Assertion `(uv_loop_close(&loop_)) == (0)' failed. 1: node::Abort() [out/Release/node] 2: 0x15084c4 [out/Release/node] 3: node::worker::Worker::~Worker() [out/Release/node] 4: node::Environment::RunCleanup() [out/Release/node]

I’ll look into it.

@addaleaxaddaleax added the blocked PRs that are blocked by other issues or PRs. label Jun 7, 2018
@addaleax

Copy link
Copy Markdown
MemberAuthor

Ok, so, this seems to expose a libuv bug, and I see no way of hacking around that. Guess this PR will have to wait for now.

libuv/libuv#1869

@addaleax
addaleaxforce-pushed the worker-fs-watch-file branch from a03e9bf to 3f00235CompareMarch 18, 2019 20:26
@addaleax

Copy link
Copy Markdown
MemberAuthor

With libuv 1.27.0 (#26707) merged, this should now finally be ready!

CI: https://ci.nodejs.org/job/node-test-pull-request/21643/

@addaleaxaddaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed blocked PRs that are blocked by other issues or PRs. labels Mar 18, 2019
@addaleax

Copy link
Copy Markdown
MemberAuthor

Landed in d398e89 🎉

@addaleax
addaleax deleted the worker-fs-watch-file branch March 18, 2019 21:25
addaleax added a commit that referenced this pull request Mar 18, 2019
Refs: #21093 (comment)
PR-URL: #21179
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Mar 27, 2019
Refs: nodejs#21093 (comment)
PR-URL: nodejs#21179
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Mar 27, 2019
Refs: #21093 (comment)
PR-URL: #21179
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targostargos mentioned this pull request Mar 27, 2019
BethGriggs pushed a commit that referenced this pull request Apr 17, 2019
Refs: #21093 (comment)
PR-URL: #21179
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BethGriggsBethGriggs mentioned this pull request May 1, 2019
@addaleax

Copy link
Copy Markdown
MemberAuthor

@BethGriggs I think this is causing failures in v10.x-staging because it depends on #26707… I think either this has to be removed or the libuv update has to be included in v10.x as well

MylesBorins pushed a commit that referenced this pull request May 16, 2019
Refs: #21093 (comment)
PR-URL: #21179
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.fsIssues and PRs related to the fs subsystem / file system.testIssues and PRs related to the tests.workerIssues and PRs related to Worker support.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@addaleax@apapirovski@jasnell@Fishrock123@benjamingr@cjihrig@hiroppy@JungMinu@trivikr@nodejs-github-bot