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

All: Update node-watch dependency and test fixtures for Node 12+ #1448

Merged
merged 2 commits into from
Jun 25, 2020

Conversation

Krinkle
Copy link
Member

@Krinkle Krinkle commented Jun 20, 2020

Test fixtures

Node 6-10:

at process._tickCallback (internal/process/next_tick.js…)
at ontimeout (timers.js…)

Node 12-14:

at processTicksAndRejections (internal/process/task_queues.js…)
at listOnTimeout (internal/timers.js…)

Test memory

Also update the memory-leak test to use a different strategy
because the V8 native %GetWeakSetValues function no longer
exists as of V8 7.1 (or 8.1, not sure) per
https://chromium.googlesource.com/v8/v8/+/0cf4a0f82f8f810519ba0d4b3b01adef0a0a6c1d
https://chromium-review.googlesource.com/c/v8/v8/+/1238574.

Instead, inspect a heap snapshot and validate it that way.
Also expand the test so that we first verify our logic actually
works, for easier debugging in the future.

Recursive watch

As of Node 14, fs.watch can throw ERR_FEATURE_UNAVAILABLE_ON_PLATFORM,
which is handled by node-watch 0.6.4 per
yuanchuan/node-watch@fd5d4655ca47db56.

Without this, Node 14 fails as follows:

CLI Watch > runs tests and waits until SIGTERM

TypeError [ERR_FEATURE_UNAVAILABLE_ON_PLATFORM]:
The feature watch recursively is unavailable on the current platform, …
at Object.watch (fs.js)
at hasNativeRecursive (…/node_modules/node-watch/lib/has-native-recursive.js)
at Watcher.watchDirectory (…/node_modules/node-watch/lib/watch.js)
at watch (…/node_modules/node-watch/lib/watch.js)
at Function.watch (qunitjs/qunit/src/cli/run.js)

Fixes #1430.

== Test fixtures ==

Node 6-10:
> at process._tickCallback (internal/process/next_tick.js…)
> at ontimeout (timers.js…)

Node 12-14:
> at processTicksAndRejections (internal/process/task_queues.js…)
> at listOnTimeout (internal/timers.js…)

== Test memory ==

Also update the memory-leak test to use a different strategy
because the V8 native `%GetWeakSetValues` function no longer
exists as of V8 7.1 (or 8.1, not sure) per
<https://chromium.googlesource.com/v8/v8/+/0cf4a0f82f8f810519ba0d4b3b01adef0a0a6c1d>
<https://chromium-review.googlesource.com/c/v8/v8/+/1238574>.

Instead, inspect a heap snapshot and validate it that way.
Also expand the test so that we first verify our logic actually
works, for easier debugging in the future.

== Recursive watch ==

As of Node 14, `fs.watch` can throw ERR_FEATURE_UNAVAILABLE_ON_PLATFORM,
which is handled by node-watch 0.6.4 per
<yuanchuan/node-watch@fd5d4655ca47db56>.

Without this, Node 14 fails as follows:

> CLI Watch > runs tests and waits until SIGTERM
>
> TypeError [ERR_FEATURE_UNAVAILABLE_ON_PLATFORM]:
> The feature watch recursively is unavailable on the current platform, …
> at Object.watch (fs.js)
> at hasNativeRecursive (…/node_modules/node-watch/lib/has-native-recursive.js)
> at Watcher.watchDirectory (…/node_modules/node-watch/lib/watch.js)
> at watch (…/node_modules/node-watch/lib/watch.js)
> at Function.watch (qunitjs/qunit/src/cli/run.js)

Fixes #1430.
Copy link
Member

@trentmwillis trentmwillis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Thanks for working on it.

@Krinkle Krinkle merged commit 91b6995 into master Jun 25, 2020
@Krinkle Krinkle deleted the node12 branch June 25, 2020 01:20
@Krinkle Krinkle mentioned this pull request Jul 21, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Assertion stack traces off by one in node 12
2 participants