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

coverage: expose native V8 coverage #22527

Closed
wants to merge 1 commit into from
Closed

Conversation

bcoe
Copy link
Contributor

@bcoe bcoe commented Aug 25, 2018

This pull request is again part of an ongoing initiative to improve the Node.js for those building developer tools (CC: @bengl, @boneskull).


What is this?

For some time now, V8 has exposed test coverage through the inspector protocol. This pull request makes it possible to have this information output to disk by setting the environment variable NODE_V8_COVERAGE=./coverage-directory.

By moving this logic into Node.js itself, we're able to better hook process.exit(), signal-handling, and child_process.spawn(); this is awesome, because this behavior was previously facilitated by a handful of horrible hacks:

Going forward a tool similar to nyc will simply be able to consume the raw V8 coverage information, and output Istanbul coverage reports.

Help with code review

@nodejs/v8-inspector I rely on some of the behavior that the inspector API is currently exhibiting, mainly that coverage is output in a single tick of the event loop:

    session.post('Profiler.takePreciseCoverage', (err, coverageInfo) => {
      try {
        if (err) throw err;
        writeFileSync(target, JSON.stringify(coverageInfo));
      } catch (err) {
        console.error(err);
      }
    });

It would be wonderful to have some folks double check my work.

I'm also looping in several folks who've been involved with Istanbul and nyc over the years.

@isaacs, @addaleax, @schuay.

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

@bcoe bcoe added semver-minor PRs that contain new features and should be released in the next minor version. test Issues and PRs related to the tests. inspector Issues and PRs related to the V8 inspector protocol labels Aug 25, 2018
@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Aug 25, 2018
@bcoe bcoe requested review from addaleax, Trott and MylesBorins August 25, 2018 23:05
@@ -496,6 +496,12 @@ function normalizeSpawnArguments(file, args, options) {
var env = options.env || process.env;
var envPairs = [];

// process.env.NODE_V8_COVERAGE always propagates, making it possible to
// collect coverage for programs that spawn with white-listed environment.
if (process.env.NODE_V8_COVERAGE) {
Copy link
Member

Choose a reason for hiding this comment

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

is there a way to stop this from happening?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is not at this time? any recommendations.

It's almost always the case that you will be setting NODE_V8_COVERAGE in the context of running a test suite, and that you'd probably like to er on the side of the variable propagating, even though it's pretty common in tests to selectively set env, e.g., maybe your test is exercising a specific environment.

Copy link
Member

Choose a reason for hiding this comment

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

I'm sure it's quite common, but we should support both sides. perhaps checking if the property already exists (so you can set it to an empty string) would work.

Copy link
Contributor Author

@bcoe bcoe Aug 26, 2018

Choose a reason for hiding this comment

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

I think that's a smart idea, I was thinking even for our own test suite we might want to be able to remove the variable.

@@ -625,6 +625,15 @@ Path to the file used to store the persistent REPL history. The default path is
`~/.node_repl_history`, which is overridden by this variable. Setting the value
to an empty string (`''` or `' '`) disables persistent REPL history.

### `NODE_V8_COVERAGE=dir`

When set, Node.js will begin outputting [V8 JavaScript code coverage][] to the
Copy link
Member

Choose a reason for hiding this comment

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

neither the link nor the addition here explain what to do with the output coverage file. Generally people just use the inspector UI. does the output file even have a governed format? do we have to semver major when v8 changes the format?

Copy link
Contributor Author

@bcoe bcoe Aug 25, 2018

Choose a reason for hiding this comment

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

Generally people just use the inspector UI

I would argue it's very rare for people writing command line tools and modules to interact with the inspector, as evidenced by the fact that nyc has over 31,000 dependents and continues to grow in popularity (getting downloaded over 400,000 times a week) despite native test coverage having been available for quite some time.

As the lead maintainer of nyc and Istanbul, I would love to move people away from using these libraries when writing Node.js applications, and towards V8's coverage -- but I think the burden of kicking off and interacting with an inspector session is a usability rough-edge that will prevent adoption.

do we have to semver major when v8 changes the format?

I think you're right that we should point users to a breakdown of what to expect in the output format -- @schuay can you recommend a better document to link to.

The output format is standardized in V8, and I don't expect that there will be major breaking changes to it (and if there are it will be very rare).

Copy link
Member

Choose a reason for hiding this comment

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

sorry I wasn't clear. I'm saying there need to be docs for the format available from the node docs, not that people should only use the inspector UI. the link you have doesn't give any docs on the format of the file.

On the semver-major front, I just want to make sure we're careful, if you think it's fine that's good enough for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's let @schuay or someone else on V8's side chime in too (@TimothyGu?), I think your concern is valid and it would be good to have someone speak to how stable the API is on the Chromium side of things.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I'm not too involved in the V8 development process… though it might be telling that the coverage API is already in the RC version of the devtools protocol without an Experimental flag on.

Copy link

@schuay schuay Aug 27, 2018

Choose a reason for hiding this comment

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

Great to see this 👍

The inspector protocol documentation here is the source of truth. In particular, takePreciseCoverage will return an array of ScriptCoverage objects.

I'm not sure what DevTools' policies on protocol changes are. You could try asking on their mailing list: https://groups.google.com/forum/#!forum/google-chrome-developer-tools. For V8 coverage specifically, we currently don't have any changes in the output format planned that I'm aware of.

Copy link
Member

Choose a reason for hiding this comment

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

Generally we are very careful with changing CDP due to backwards compatibility concerns. Unless unavoidable, we will not change the coverage format. If that happens however, the CDP version would be bumped as well.

return;
}

const filename = `coverage-${process.pid}-${Date.now()}.json`;
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to account for the worker thread id here, and test interaction with it, as well…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mind expanding a bit, pseudo code would be awesome 👍

Copy link
Member

Choose a reason for hiding this comment

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

Basically, I’m asking for require('internal/worker').threadId to be part of the format string here :)

const output = spawnSync(process.execPath, [
require.resolve('../fixtures/v8-coverage/exit-1')
], { env: { NODE_V8_COVERAGE: coverageDirectory } });
assert.strictEqual(output.status, 1, 'exit code not 1');
Copy link
Member

Choose a reason for hiding this comment

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

We generally try to avoid using static message parameters for assertions, and use comments instead – most of the time, a static message string just hides the most useful information (the actual values in the assertion)

doc/api/cli.md Outdated
@@ -695,3 +704,4 @@ greater than `4` (its current default value). For more information, see the
[emit_warning]: process.html#process_process_emitwarning_warning_type_code_ctor
[experimental ECMAScript Module]: esm.html#esm_loader_hooks
[libuv threadpool documentation]: http://docs.libuv.org/en/latest/threadpool.html
[V8 JavaScript code coverage]: https://v8project.blogspot.com/2017/12/javascript-code-coverage.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this reference should go after [REPL]: repl.html in ASCII sort order.

doc/api/cli.md Outdated
directory provided as an argument.

`NODE_V8_COVERAGE` will automatically propagate to subprocesses, making it
easier to instrument applications that call the `child_process.spawn` family
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: `child_process.spawn` -> `child_process.spawn()`

// process.env.NODE_V8_COVERAGE always propagates, making it possible to
// collect coverage for programs that spawn with white-listed environment.
if (process.env.NODE_V8_COVERAGE &&
!(options.env || {}).hasOwnProperty('NODE_V8_COVERAGE')) {
Copy link
Member

Choose a reason for hiding this comment

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

please use Object.prototype.hasOwnProperty.call

@devsnek devsnek dismissed their stale review August 28, 2018 01:32

changes

doc/api/cli.md Outdated

When set, Node.js will begin outputting [V8 JavaScript code coverage][] to the
directory provided as an argument. Coverage is output as an array of
[ScriptCoverage][] objects.
Copy link
Member

@addaleax addaleax Aug 28, 2018

Choose a reason for hiding this comment

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

Just wondering, would an object with a single entry make sense? That would leaves a bit of room for adding more information, e.g. metadata?

Copy link
Contributor Author

@bcoe bcoe Aug 28, 2018

Choose a reason for hiding this comment

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

the object is shaped like this:

{
  "results": [ScriptCoverage, ScriptCoverage]
}

Copy link
Member

Choose a reason for hiding this comment

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

Okay, great – that just wasn’t obvious from the documentation :)

@bcoe
Copy link
Contributor Author

bcoe commented Aug 29, 2018

@nodejs/testing 👋 thought this would be pertinent to your interests -- a next logical step once this lands would be moving Node.js' test coverage over to the approach. I've benchmarked it and it's 300% faster 😮

@evanlucas
Copy link
Contributor

I’m super thrilled to see this! Awesome work!


const filename = `coverage-${process.pid}-${Date.now()}.json`;
try {
mkdirSync(process.env.NODE_V8_COVERAGE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this use the recursive option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to switch to recursive as soon as this issue is resolved, and we know the shape of the API won't be changing.

This is actually a perfect example of why mkdirp is a good addition to the API :)

@Trott
Copy link
Member

Trott commented Aug 31, 2018

I suspect some members of @nodejs/build may be familiar with coverage stuff. Ping!

// process.env.NODE_V8_COVERAGE always propagates, making it possible to
// collect coverage for programs that spawn with white-listed environment.
if (process.env.NODE_V8_COVERAGE &&
!Object.prototype.hasOwnProperty.call(options.env || {}, 'NODE_V8_COVERAGE')) {
Copy link
Member

Choose a reason for hiding this comment

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

No-blocking nit: !options.env || !Object.prototype.hasOwnProperty.call(options.env, 'NODE_V8_COVERAGE')

doc/api/cli.md Outdated

`NODE_V8_COVERAGE` will automatically propagate to subprocesses, making it
easier to instrument applications that call the `child_process.spawn()` family
of functions.
Copy link
Member

Choose a reason for hiding this comment

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

Adding a note that this is currently not supported for worker threads is likely a good thing as people might otherwise maybe run into this as long as it's not supported.

Copy link
Member

Choose a reason for hiding this comment

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

It should also be documented that the coverage generation can be deactivated for specific child processes by explicitly setting the env flag to an empty string.

try {
session.post('Profiler.takePreciseCoverage', (err, coverageInfo) => {
try {
if (err) throw err;
Copy link
Member

Choose a reason for hiding this comment

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

Moving this one line up would be better:

if (err) return console.error(err);
try { ... } catch (err) { ... }

function getFixtureCoverage(fixtureFile, coverageDirectory) {
const coverageFiles = fs.readdirSync(coverageDirectory);
for (let i = 0, coverageFile; (coverageFile = coverageFiles[i]) !== undefined;
i++) {
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it should be written with for of loops instead:

for (const coverageFile of coverageFiles) {
  ...
  for (const fixtureCoverage of coverage.result) {
    ...
  }
}

const coverageDirectory = path.join(tmpdir.path, nextdir());
const output = spawnSync(process.execPath, [
require.resolve('../fixtures/v8-coverage/basic')
], { env: extendEnv({ NODE_V8_COVERAGE: coverageDirectory }) });
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to use the object spread notation instead of using a helper function here.

{ env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } }


function writeCoverage() {
if (!hasInspector || !session) {
return;
Copy link
Member

Choose a reason for hiding this comment

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

The second part of the condition is not possible to reach: either there is no inspector in which case the session is empty or the session will be set (if that would not be the case, the property access below would throw).

And should we not log an error here? I would be surprised if I pass in the environment variable but the coverage files are not generated.


function setup() {
if (!hasInspector) {
return;
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to throw in error in this case. Otherwise the user would not know that no data will be collected.

In that case the hasInspector check in writeCoverage would also be obsolete.

Copy link
Member

Choose a reason for hiding this comment

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

I’m not a fan of throwing here. If necessary, we can print a warning, but in practice, this situation really only occurs within Workers (for now).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a warning, which I feel is a reasonable compromise; It would be miserable to be unable to run your test suite with coverage because executing code in a worker results in exceptions being thrown.

When we support workers in the future, perhaps we could switch to throwing an exception?

// process.env.NODE_V8_COVERAGE always propagates, making it possible to
// collect coverage for programs that spawn with white-listed environment.
if (process.env.NODE_V8_COVERAGE &&
!Object.prototype.hasOwnProperty.call(options.env || {}, 'NODE_V8_COVERAGE')) {
Copy link
Member

Choose a reason for hiding this comment

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

bad indentation

@rvagg
Copy link
Member

rvagg commented Aug 31, 2018

I imagine this will let us replace lib/internal/process/write-coverage.js functionality, does that have to be a separate step?

session.connect();
session.post('Profiler.enable');
session.post('Profiler.startPreciseCoverage', { callCount: true,
detailed: true });
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to also expose a way to use other coverage modes? This primarily has performance implications. E.g. callCount: false would allow a function to be inlined after it has been covered. detailed: true would not require coverage-related bytecode to be emitted.

Copy link

Choose a reason for hiding this comment

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

I'd argue for simplicity, exposing only the most detailed mode, and making sure it is as fast as possible. callCount: true / detailed: true should also allow inlining (it does require special bytecode emission though).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm with @schuay on this one, I'd lean towards starting simple. I can imagine following up with the option for more complex configuration.

@addaleax
Copy link
Member

addaleax commented Sep 2, 2018

@bcoe Heads up, this needs a rebase (presumably around the options help texts being moved to lib/internal/print_help.js)

@bcoe
Copy link
Contributor Author

bcoe commented Sep 3, 2018

@rvagg I'd like to replace write-coverage.js and the instrumentation process we apply with this approach eventually (it will most likely be the thing I work on immediately after we land this, with @addaleax's help).

I'd prefer to spread this out over two pull requests, for a few reasons:

  1. a few tests will need to be debugged and potentially rewritten a bit; specifically tests that exercise async_hooks are having trouble passing in Node's suite, because they mock some of the internal libraries used by the inspector.
  2. all the coverage bits in the Makefile will need to be rewritten and fiddled with.
  3. I need to actually write the dependent library that easily converts from the inspector format to the format consumed by our reports (I have most of the pieces written, but it will be easier to finish off this dependency once we land this work).

tldr; I think there are enough sticky bits to getting Node consuming the inspector coverage format, that it will be easier to consume across two pull requests.

Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

LGTM with my last two comments addressed.

@@ -28,8 +28,8 @@ function writeCoverage() {

try {
session.post('Profiler.takePreciseCoverage', (err, coverageInfo) => {
if (err) console.error(err);
Copy link
Member

Choose a reason for hiding this comment

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

If an error exists, the code should not continue, otherwise the write would still be triggered.

Adding a return statement fixes this: if (err) return console.error(err);

@@ -9,7 +9,7 @@ const hasInspector = process.config.variables.v8_enable_inspector === 1 &&
let session;

function writeCoverage() {
if (!hasInspector || !session) {
if (!session) {
return;
Copy link
Member

@BridgeAR BridgeAR Sep 4, 2018

Choose a reason for hiding this comment

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

The session check can not be reached due to either being set to a truthy value if the inspector is present, or not being set and the setup function throws and the code does not continue. If the latter is the case, the writeCoverage function is never called at all.

Therefore it's safe to remove this check as well.

I missed that the function is exported.

@@ -20,6 +20,18 @@ function nextdir() {
const output = spawnSync(process.execPath, [
require.resolve('../fixtures/v8-coverage/basic')
], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } });
try {
console.info('--- DEBUG ---');
Copy link
Member

Choose a reason for hiding this comment

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

are the console.info() statements part of the test or purely informational? If informational, I'd prefer to omit them and use code comments instead. If they are part of the test, then please add a comment indicating that so that they are not accidentally removed later.

Copy link
Contributor Author

@bcoe bcoe Sep 4, 2018

Choose a reason for hiding this comment

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

there's a new build environment that fails for coverage, added since I last ran the suite. This is purely to see what the heck is going on:

node-test-commit-linux-containered

will remove as soon as I see what the underlying exception is.

native V8 coverage reports can now be written to disk by setting the
variable NODE_V8_COVERAGE=dir
@bcoe
Copy link
Contributor Author

bcoe commented Sep 5, 2018

I believe comments have been addressed, and I intend to land this in the next day or so 👍

@bcoe
Copy link
Contributor Author

bcoe commented Sep 5, 2018

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 5, 2018
@bcoe bcoe changed the title coverage: expose native V8 coverage test: expose native V8 coverage Sep 5, 2018
@bcoe bcoe changed the title test: expose native V8 coverage coverage: expose native V8 coverage Sep 5, 2018
bcoe pushed a commit that referenced this pull request Sep 5, 2018
native V8 coverage reports can now be written to disk by setting the
variable NODE_V8_COVERAGE=dir

PR-URL: #22527
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@bcoe
Copy link
Contributor Author

bcoe commented Sep 5, 2018

Landed in c9d6e3f

@bcoe bcoe closed this Sep 5, 2018
targos pushed a commit that referenced this pull request Sep 5, 2018
native V8 coverage reports can now be written to disk by setting the
variable NODE_V8_COVERAGE=dir

PR-URL: #22527
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
@bcoe bcoe deleted the coverage branch September 5, 2018 18:30
targos added a commit that referenced this pull request Sep 5, 2018
Notable changes:

* child_process:
  * `TypedArray` and `DataView` values are now accepted as input by
    `execFileSync` and `spawnSync`. #22409
* coverage:
  * Native V8 code coverage information can now be output to disk by setting the
    environment variable `NODE_V8_COVERAGE` to a directory. #22527
* deps:
  * The bundled npm was upgraded to version 6.4.1. #22591
    * Changelogs:
      [6.3.0-next.0](https://github.com/npm/cli/releases/tag/v6.3.0-next.0)
      [6.3.0](https://github.com/npm/cli/releases/tag/v6.3.0)
      [6.4.0](https://github.com/npm/cli/releases/tag/v6.4.0)
      [6.4.1](https://github.com/npm/cli/releases/tag/v6.4.1)
* fs:
  * The methods `fs.read`, `fs.readSync`, `fs.write`, `fs.writeSync`,
    `fs.writeFile` and `fs.writeFileSync` now all accept `TypedArray` and
    `DataView` objects. #22150
  * A new boolean option, `withFileTypes`, can be passed to to `fs.readdir` and
    `fs.readdirSync`. If set to true, the methods return an array of directory
    entries. These are objects that can be used to determine the type of each
    entry and filter them based on that without calling `fs.stat`. #22020
* http2:
  * The `http2` module is no longer experimental. #22466
* os:
  * Added two new methods: `os.getPriority` and `os.setPriority`, allowing to
    manipulate the scheduling priority of processes. #22394
* process:
  * Added `process.allowedNodeEnvironmentFlags`. This object can be used to
    programmatically validate and list flags that are allowed in the
    `NODE_OPTIONS` environment variable. #19335
* src:
  * Deprecated option variables in public C++ API. #22392
  * Refactored options parsing. #22392
* vm:
  * Added `vm.compileFunction`, a method to create new JavaScript functions from
    a source body, with options similar to those of the other `vm` methods. #21571
* Added new collaborators:
  * [lundibundi](https://github.com/lundibundi) - Denys Otrishko

PR-URL: #22716
targos pushed a commit that referenced this pull request Sep 6, 2018
native V8 coverage reports can now be written to disk by setting the
variable NODE_V8_COVERAGE=dir

PR-URL: #22527
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
targos added a commit that referenced this pull request Sep 6, 2018
Notable changes:

* child_process:
  * `TypedArray` and `DataView` values are now accepted as input by
    `execFileSync` and `spawnSync`. #22409
* coverage:
  * Native V8 code coverage information can now be output to disk by setting the
    environment variable `NODE_V8_COVERAGE` to a directory. #22527
* deps:
  * The bundled npm was upgraded to version 6.4.1. #22591
    * Changelogs:
      [6.3.0-next.0](https://github.com/npm/cli/releases/tag/v6.3.0-next.0)
      [6.3.0](https://github.com/npm/cli/releases/tag/v6.3.0)
      [6.4.0](https://github.com/npm/cli/releases/tag/v6.4.0)
      [6.4.1](https://github.com/npm/cli/releases/tag/v6.4.1)
* fs:
  * The methods `fs.read`, `fs.readSync`, `fs.write`, `fs.writeSync`,
    `fs.writeFile` and `fs.writeFileSync` now all accept `TypedArray` and
    `DataView` objects. #22150
  * A new boolean option, `withFileTypes`, can be passed to to `fs.readdir` and
    `fs.readdirSync`. If set to true, the methods return an array of directory
    entries. These are objects that can be used to determine the type of each
    entry and filter them based on that without calling `fs.stat`. #22020
* http2:
  * The `http2` module is no longer experimental. #22466
* os:
  * Added two new methods: `os.getPriority` and `os.setPriority`, allowing to
    manipulate the scheduling priority of processes. #22407
* process:
  * Added `process.allowedNodeEnvironmentFlags`. This object can be used to
    programmatically validate and list flags that are allowed in the
    `NODE_OPTIONS` environment variable. #19335
* src:
  * Deprecated option variables in public C++ API. #22515
  * Refactored options parsing. #22392
* vm:
  * Added `vm.compileFunction`, a method to create new JavaScript functions from
    a source body, with options similar to those of the other `vm` methods. #21571
* Added new collaborators:
  * [lundibundi](https://github.com/lundibundi) - Denys Otrishko

PR-URL: #22716
targos added a commit that referenced this pull request Sep 6, 2018
Notable changes:

* child_process:
  * `TypedArray` and `DataView` values are now accepted as input by
    `execFileSync` and `spawnSync`. #22409
* coverage:
  * Native V8 code coverage information can now be output to disk by setting the
    environment variable `NODE_V8_COVERAGE` to a directory. #22527
* deps:
  * The bundled npm was upgraded to version 6.4.1. #22591
    * Changelogs:
      [6.3.0-next.0](https://github.com/npm/cli/releases/tag/v6.3.0-next.0)
      [6.3.0](https://github.com/npm/cli/releases/tag/v6.3.0)
      [6.4.0](https://github.com/npm/cli/releases/tag/v6.4.0)
      [6.4.1](https://github.com/npm/cli/releases/tag/v6.4.1)
* fs:
  * The methods `fs.read`, `fs.readSync`, `fs.write`, `fs.writeSync`,
    `fs.writeFile` and `fs.writeFileSync` now all accept `TypedArray` and
    `DataView` objects. #22150
  * A new boolean option, `withFileTypes`, can be passed to to `fs.readdir` and
    `fs.readdirSync`. If set to true, the methods return an array of directory
    entries. These are objects that can be used to determine the type of each
    entry and filter them based on that without calling `fs.stat`. #22020
* http2:
  * The `http2` module is no longer experimental. #22466
* os:
  * Added two new methods: `os.getPriority` and `os.setPriority`, allowing to
    manipulate the scheduling priority of processes. #22407
* process:
  * Added `process.allowedNodeEnvironmentFlags`. This object can be used to
    programmatically validate and list flags that are allowed in the
    `NODE_OPTIONS` environment variable. #19335
* src:
  * Deprecated option variables in public C++ API. #22515
  * Refactored options parsing. #22392
* vm:
  * Added `vm.compileFunction`, a method to create new JavaScript functions from
    a source body, with options similar to those of the other `vm` methods. #21571
* Added new collaborators:
  * [lundibundi](https://github.com/lundibundi) - Denys Otrishko

PR-URL: #22716
targos added a commit that referenced this pull request Sep 6, 2018
Notable changes:

* child_process:
  * `TypedArray` and `DataView` values are now accepted as input by
    `execFileSync` and `spawnSync`. #22409
* coverage:
  * Native V8 code coverage information can now be output to disk by setting the
    environment variable `NODE_V8_COVERAGE` to a directory. #22527
* deps:
  * The bundled npm was upgraded to version 6.4.1. #22591
    * Changelogs:
      [6.3.0-next.0](https://github.com/npm/cli/releases/tag/v6.3.0-next.0)
      [6.3.0](https://github.com/npm/cli/releases/tag/v6.3.0)
      [6.4.0](https://github.com/npm/cli/releases/tag/v6.4.0)
      [6.4.1](https://github.com/npm/cli/releases/tag/v6.4.1)
* fs:
  * The methods `fs.read`, `fs.readSync`, `fs.write`, `fs.writeSync`,
    `fs.writeFile` and `fs.writeFileSync` now all accept `TypedArray` and
    `DataView` objects. #22150
  * A new boolean option, `withFileTypes`, can be passed to to `fs.readdir` and
    `fs.readdirSync`. If set to true, the methods return an array of directory
    entries. These are objects that can be used to determine the type of each
    entry and filter them based on that without calling `fs.stat`. #22020
* http2:
  * The `http2` module is no longer experimental. #22466
* os:
  * Added two new methods: `os.getPriority` and `os.setPriority`, allowing to
    manipulate the scheduling priority of processes. #22407
* process:
  * Added `process.allowedNodeEnvironmentFlags`. This object can be used to
    programmatically validate and list flags that are allowed in the
    `NODE_OPTIONS` environment variable. #19335
* src:
  * Deprecated option variables in public C++ API. #22515
  * Refactored options parsing. #22392
* vm:
  * Added `vm.compileFunction`, a method to create new JavaScript functions from
    a source body, with options similar to those of the other `vm` methods. #21571
* Added new collaborators:
  * [lundibundi](https://github.com/lundibundi) - Denys Otrishko

PR-URL: #22716
@addaleax addaleax added the coverage Issues and PRs related to native coverage support. label Sep 18, 2018
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. coverage Issues and PRs related to native coverage support. inspector Issues and PRs related to the V8 inspector protocol lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.