From b8ab430f5871065d7b2c006d229a0ab89bcfbe2e Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Mon, 2 Oct 2017 05:23:35 -0700 Subject: [PATCH 01/35] build: call setlocal in vcbuild.bat Currently the variables set in vcbuild.bat are mostly global and escape/leak out into the calling process. For example, running vcbuild.bat test and then echoing the config variable gives: vcbuild.bat test ... echo %config% Release After this change the same command give: vcbuild.bat test ... echo %config% %config% PR-URL: https://github.com/nodejs/node/pull/15754 Reviewed-By: James M Snell --- vcbuild.bat | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vcbuild.bat b/vcbuild.bat index 70f94cf0145dd4..4b86da757ca3ab 100644 --- a/vcbuild.bat +++ b/vcbuild.bat @@ -1,5 +1,7 @@ @echo off +setlocal EnableExtensions + cd %~dp0 if /i "%1"=="help" goto help From 3991953e3bc1da49121b9ae842fa11ae23920dc5 Mon Sep 17 00:00:00 2001 From: tshemsedinov Date: Sat, 7 Oct 2017 03:28:37 +0300 Subject: [PATCH 02/35] doc: fix incorrect vm.createContext usage In code example `vm.createContext` called with new operator by mistake. It is not a constructor. PR-URL: https://github.com/nodejs/node/pull/16059 Reviewed-By: Anna Henningsen Reviewed-By: Timothy Gu Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: Minwoo Jung --- doc/api/vm.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/vm.md b/doc/api/vm.md index cd47bc04ea85f6..592c78148e7040 100644 --- a/doc/api/vm.md +++ b/doc/api/vm.md @@ -108,7 +108,7 @@ const sandbox = { const script = new vm.Script('count += 1; name = "kitty";'); -const context = new vm.createContext(sandbox); +const context = vm.createContext(sandbox); for (let i = 0; i < 10; ++i) { script.runInContext(context); } From c405f95862bb024313251c1f67350b158eb9fc8c Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Tue, 3 Oct 2017 20:51:12 -0700 Subject: [PATCH 03/35] src: fix ^ in stack trace with vm's columnOffset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While VM module's columnOffset option does succeed in applying an offset to the column number in the stack trace, the wavy diagram printed does not account for potential offsets, resulting in erroneous location of `^` in the first line of the script. Before: ``` > vm.runInThisContext('throw new Error()', { columnOffset: 5 }) evalmachine.:1 throw new Error() ^ Error at evalmachine.:1:12 at ContextifyScript.Script.runInThisContext (vm.js:44:33) at Object.runInThisContext (vm.js:116:38) ``` After: ``` > vm.runInThisContext('throw new Error()', { columnOffset: 5 }) evalmachine.:1 throw new Error() ^ Error at evalmachine.:1:12 at ContextifyScript.Script.runInThisContext (vm.js:50:33) at Object.runInThisContext (vm.js:139:38) at repl:1:4 ``` PR-URL: https://github.com/nodejs/node/pull/15771 Refs: https://github.com/tmpvar/jsdom/pull/2003 Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott Reviewed-By: Colin Ihrig Reviewed-By: Tobias Nießen --- src/node.cc | 9 +++++++++ test/parallel/test-vm-context.js | 5 +++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/node.cc b/src/node.cc index 882c72a67eb330..f4218ca6795933 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1585,6 +1585,7 @@ void AppendExceptionLine(Environment* env, } // Print (filename):(line number): (message). + ScriptOrigin origin = message->GetScriptOrigin(); node::Utf8Value filename(env->isolate(), message->GetScriptResourceName()); const char* filename_string = *filename; int linenum = message->GetLineNumber(); @@ -1613,8 +1614,16 @@ void AppendExceptionLine(Environment* env, // sourceline to 78 characters, and we end up not providing very much // useful debugging info to the user if we remove 62 characters. + int script_start = + (linenum - origin.ResourceLineOffset()->Value()) == 1 ? + origin.ResourceColumnOffset()->Value() : 0; int start = message->GetStartColumn(env->context()).FromMaybe(0); int end = message->GetEndColumn(env->context()).FromMaybe(0); + if (start >= script_start) { + CHECK_GE(end, start); + start -= script_start; + end -= script_start; + } char arrow[1024]; int max_off = sizeof(arrow) - 2; diff --git a/test/parallel/test-vm-context.js b/test/parallel/test-vm-context.js index 33116f52bb08e7..a9cf7a1c8adab5 100644 --- a/test/parallel/test-vm-context.js +++ b/test/parallel/test-vm-context.js @@ -72,13 +72,14 @@ assert.strictEqual(script.runInContext(ctx), false); // Error on the first line of a module should // have the correct line and column number assert.throws(() => { - vm.runInContext('throw new Error()', context, { + vm.runInContext(' throw new Error()', context, { filename: 'expected-filename.js', lineOffset: 32, columnOffset: 123 }); }, (err) => { - return /expected-filename\.js:33:130/.test(err.stack); + return /^ \^/m.test(err.stack) && + /expected-filename\.js:33:131/.test(err.stack); }, 'Expected appearance of proper offset in Error stack'); // https://github.com/nodejs/node/issues/6158 From c81b425a63354487404c030ef0f4347ab72b68da Mon Sep 17 00:00:00 2001 From: Saeed H Date: Fri, 6 Oct 2017 11:51:37 -0700 Subject: [PATCH 04/35] test: remove messages in assert.strictEqual MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/16014 Reviewed-By: Colin Ihrig Reviewed-By: Joyee Cheung Reviewed-By: Benjamin Gruenbaum Reviewed-By: Gibson Fahnestock Reviewed-By: Tobias Nießen --- test/parallel/test-tls-pfx-gh-5100-regr.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/parallel/test-tls-pfx-gh-5100-regr.js b/test/parallel/test-tls-pfx-gh-5100-regr.js index ba38f02cdcf540..112a00ea2e89e9 100644 --- a/test/parallel/test-tls-pfx-gh-5100-regr.js +++ b/test/parallel/test-tls-pfx-gh-5100-regr.js @@ -19,11 +19,7 @@ const server = tls.createServer({ requestCert: true, rejectUnauthorized: false }, common.mustCall(function(c) { - assert.strictEqual( - c.authorizationError, - null, - 'authorizationError must be null' - ); + assert.strictEqual(c.authorizationError, null); c.end(); })).listen(0, function() { const client = tls.connect({ From 6d49154258a8852cc9b13a13953fc4b9fd3fec19 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 5 Oct 2017 12:19:33 -0700 Subject: [PATCH 05/35] doc: update style guide for markdown extension There are no longer files in the repository that use the `.markdown` extension so remove mention of them. PR-URL: https://github.com/nodejs/node/pull/15786 Reviewed-By: Vse Mozhet Byt Reviewed-By: Lance Ball Reviewed-By: Gibson Fahnestock Reviewed-By: Richard Lau --- doc/STYLE_GUIDE.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/STYLE_GUIDE.md b/doc/STYLE_GUIDE.md index 32be23d126f101..8dbc1412a97dad 100644 --- a/doc/STYLE_GUIDE.md +++ b/doc/STYLE_GUIDE.md @@ -6,8 +6,6 @@ topic the document will describe (e.g., `child_process`). * Filenames should be **lowercase**. * Some files, such as top-level markdown files, are exceptions. - * Older files may use the `.markdown` extension. These may be ported to `.md` - at will. **Prefer `.md` for all new documents.** * Documents should be word-wrapped at 80 characters. * The formatting described in `.editorconfig` is preferred. * A [plugin][] is available for some editors to automatically apply these From d0157b7c260bf7108212424f07a33f3c53636107 Mon Sep 17 00:00:00 2001 From: Luke Greenleaf Date: Fri, 6 Oct 2017 10:46:46 -0700 Subject: [PATCH 06/35] test: alter assert.strictEqual to default message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/15978 Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum Reviewed-By: Ruben Bridgewater Reviewed-By: Tobias Nießen --- test/parallel/test-zlib-from-string.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-zlib-from-string.js b/test/parallel/test-zlib-from-string.js index cf24d2a804cc73..7ed575d1971c0d 100644 --- a/test/parallel/test-zlib-from-string.js +++ b/test/parallel/test-zlib-from-string.js @@ -34,8 +34,7 @@ const expectedBase64Gzip = 'H4sIAAAAAAAAA11RS05DMQy8yhzg6d2BPSAkJPZu4laWkjiN4' + 'sHnHNzRtagj5AQAA'; zlib.deflate(inputString, common.mustCall((err, buffer) => { - assert.strictEqual(buffer.toString('base64'), expectedBase64Deflate, - 'deflate encoded string should match'); + assert.strictEqual(buffer.toString('base64'), expectedBase64Deflate); })); zlib.gzip(inputString, common.mustCall((err, buffer) => { @@ -46,19 +45,16 @@ zlib.gzip(inputString, common.mustCall((err, buffer) => { // result that we're expecting, and this should match what we get // from inflating the known valid deflate data. zlib.gunzip(buffer, common.mustCall((err, gunzipped) => { - assert.strictEqual(gunzipped.toString(), inputString, - 'Should get original string after gzip/gunzip'); + assert.strictEqual(gunzipped.toString(), inputString); })); })); let buffer = Buffer.from(expectedBase64Deflate, 'base64'); zlib.unzip(buffer, common.mustCall((err, buffer) => { - assert.strictEqual(buffer.toString(), inputString, - 'decoded inflated string should match'); + assert.strictEqual(buffer.toString(), inputString); })); buffer = Buffer.from(expectedBase64Gzip, 'base64'); zlib.unzip(buffer, common.mustCall((err, buffer) => { - assert.strictEqual(buffer.toString(), inputString, - 'decoded gunzipped string should match'); + assert.strictEqual(buffer.toString(), inputString); })); From 0c0717c47a89bec03f020578b3f22b0e60141846 Mon Sep 17 00:00:00 2001 From: Barry Tam Date: Fri, 6 Oct 2017 10:54:24 -0700 Subject: [PATCH 07/35] test: replacing assert message with template PR-URL: https://github.com/nodejs/node/pull/15974 Reviewed-By: Ruben Bridgewater --- test/parallel/test-cluster-disconnect-suicide-race.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-cluster-disconnect-suicide-race.js b/test/parallel/test-cluster-disconnect-suicide-race.js index 15d07002252ee9..b65af6453e3f61 100644 --- a/test/parallel/test-cluster-disconnect-suicide-race.js +++ b/test/parallel/test-cluster-disconnect-suicide-race.js @@ -8,7 +8,7 @@ const cluster = require('cluster'); if (cluster.isMaster) { cluster.on('exit', (worker, code) => { - assert.strictEqual(code, 0, 'worker exited with error'); + assert.strictEqual(code, 0, `worker exited with code: ${code}, expected 0`); }); return cluster.fork(); From bf1bf2878fcc2bc1a3f62798c543a48e2db67a08 Mon Sep 17 00:00:00 2001 From: Eric Pemberton Date: Fri, 6 Oct 2017 10:38:56 -0700 Subject: [PATCH 08/35] test: improve assert messages PR-URL: https://github.com/nodejs/node/pull/15972 Reviewed-By: Ruben Bridgewater --- test/sequential/test-child-process-execsync.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js index cccd3b7e77d6f3..e7c978406c883e 100644 --- a/test/sequential/test-child-process-execsync.js +++ b/test/sequential/test-child-process-execsync.js @@ -32,7 +32,8 @@ try { assert.strictEqual(e.errno, 'ETIMEDOUT'); err = e; } finally { - assert.strictEqual(ret, undefined, 'we should not have a return value'); + assert.strictEqual(ret, undefined, + `should not have a return value, received ${ret}`); assert.strictEqual(caught, true, 'execSync should throw'); const end = Date.now() - start; assert(end < SLEEP); @@ -53,11 +54,11 @@ cmd = `"${process.execPath}" -e "console.log('${msg}');"`; ret = execSync(cmd); assert.strictEqual(ret.length, msgBuf.length); -assert.deepStrictEqual(ret, msgBuf, 'execSync result buffer should match'); +assert.deepStrictEqual(ret, msgBuf); ret = execSync(cmd, { encoding: 'utf8' }); -assert.strictEqual(ret, `${msg}\n`, 'execSync encoding result should match'); +assert.strictEqual(ret, `${msg}\n`); const args = [ '-e', @@ -69,8 +70,7 @@ assert.deepStrictEqual(ret, msgBuf); ret = execFileSync(process.execPath, args, { encoding: 'utf8' }); -assert.strictEqual(ret, `${msg}\n`, - 'execFileSync encoding result should match'); +assert.strictEqual(ret, `${msg}\n`); // Verify that the cwd option works - GH #7824 { From d3544c1e2b2eea5482289a6a6ecf9b4723435bc1 Mon Sep 17 00:00:00 2001 From: Emily Platzer Date: Fri, 6 Oct 2017 10:42:35 -0700 Subject: [PATCH 09/35] doc: add clearer setup description MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updated console example to follow style of rest of the examples PR-URL: https://github.com/nodejs/node/pull/15962 Reviewed-By: Ruben Bridgewater Reviewed-By: Anna Henningsen Reviewed-By: Michaël Zasso --- BUILDING.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/BUILDING.md b/BUILDING.md index e6579f3e4ef2c1..a2f1a4393ea23a 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -129,6 +129,9 @@ To run the tests: $ make test ``` +At this point you are ready to make code changes and re-run the tests! +Optionally, continue below. + To run the tests and generate code coverage reports: ```console @@ -145,7 +148,7 @@ and overwrites the `lib/` directory. To clean up after generating the coverage reports: ```console -make coverage-clean +$ make coverage-clean ``` To build the documentation: From b43fed41a0eaeefc2ced04273612f6d7e70a7ff9 Mon Sep 17 00:00:00 2001 From: Brant Barger Date: Fri, 6 Oct 2017 10:34:33 -0700 Subject: [PATCH 10/35] test: replace literal with template string PR-URL: https://github.com/nodejs/node/pull/15957 Reviewed-By: Ruben Bridgewater --- .../test-cluster-server-restart-rr.js | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-cluster-server-restart-rr.js b/test/parallel/test-cluster-server-restart-rr.js index 3b26562c5dfa01..63edaf9fac7b8d 100644 --- a/test/parallel/test-cluster-server-restart-rr.js +++ b/test/parallel/test-cluster-server-restart-rr.js @@ -10,15 +10,31 @@ if (cluster.isMaster) { worker1.on('listening', common.mustCall(() => { const worker2 = cluster.fork(); worker2.on('exit', (code, signal) => { - assert.strictEqual(code, 0, 'worker2 did not exit normally'); - assert.strictEqual(signal, null, 'worker2 did not exit normally'); + assert.strictEqual( + code, + 0, + `worker${worker2.id} did not exit normally. Exit with code: ${code}` + ); + assert.strictEqual( + signal, + null, + `worker${worker2.id} did not exit normally. Exit with signal: ${signal}` + ); worker1.disconnect(); }); })); worker1.on('exit', common.mustCall((code, signal) => { - assert.strictEqual(code, 0, 'worker1 did not exit normally'); - assert.strictEqual(signal, null, 'worker1 did not exit normally'); + assert.strictEqual( + code, + 0, + `worker${worker1.id} did not exit normally. Exit with code: ${code}` + ); + assert.strictEqual( + signal, + null, + `worker${worker1.id} did not exit normally. Exit with code: ${signal}` + ); })); } else { const net = require('net'); From 97cf4f157e6fbd97d69ec8ec8433c5664bfb0543 Mon Sep 17 00:00:00 2001 From: Sarah Meyer Date: Fri, 6 Oct 2017 10:42:22 -0700 Subject: [PATCH 11/35] tools: use template literals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/15956 Reviewed-By: Daijiro Wachi Reviewed-By: Ruben Bridgewater Reviewed-By: Tobias Nießen Reviewed-By: Colin Ihrig --- tools/doc/html.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/doc/html.js b/tools/doc/html.js index c4ed0b0eda7bbe..34903e70040e3d 100644 --- a/tools/doc/html.js +++ b/tools/doc/html.js @@ -15,7 +15,7 @@ const DOC_CREATED_REG_EXP = /