[backport to v6] test: begin normalizing fixtures use

BUILDING.md

@@ -91,7 +91,7 @@ On macOS you will need to install the `Xcode Command Line Tools` by running
 installed, you can find them under the menu `Xcode -> Open Developer Tool ->
 More Developer Tools...`. This step will install `clang`, `clang++`, and
 `make`.
-* You may want to setup [firewall rules](tools/macosx-firewall.sh)
+* After building, you may want to setup [firewall rules](tools/macosx-firewall.sh)
 to avoid popups asking to accept incoming network connections when running tests:
 
 If the path to your build directory contains a space, the build will likely fail.
@@ -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:

README.md

@@ -395,6 +395,8 @@ For more information about the governance of the Node.js project, see
 **Julian Duque** <[email protected]> (he/him)
 * [JungMinu](https://github.com/JungMinu) -
 **Minwoo Jung** <[email protected]> (he/him)
+* [kfarnung](https://github.com/kfarnung) -
+**Kyle Farnung** <[email protected]> (he/him)
 * [kunalspathak](https://github.com/kunalspathak) -
 **Kunal Pathak** <[email protected]>
 * [lance](https://github.com/lance) -

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

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);
 }

doc/onboarding.md

@@ -172,6 +172,11 @@ onboarding session.
     * [https://github.com/nodejs/readable-stream](https://github.com/nodejs/readable-stream)
     * [https://github.com/nodejs/LTS](https://github.com/nodejs/LTS)
     * [https://github.com/nodejs/citgm](https://github.com/nodejs/citgm)
+  * The Node.js Foundation hosts regular summits for active contributors to the Node.js
+    project, where we have face-to-face discussion about our work on the project.
+    The foundation has travel funds to cover participants' expenses including
+    accommodation, transportation, visa fees etc. if needed. Check out the
+    [summit](https://github.com/nodejs/summit) repository for details.
 
 [Code of Conduct]: https://github.com/nodejs/TSC/blob/master/CODE_OF_CONDUCT.md
 [`core-validate-commit`]: https://github.com/evanlucas/core-validate-commit

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;

test/common/README.md

@@ -316,6 +316,37 @@ Decrements the `Countdown` counter.
 Specifies the remaining number of times `Countdown.prototype.dec()` must be
 called before the callback is invoked.
 
+## Fixtures Module
+
+The `common/fixtures` module provides convenience methods for working with
+files in the `test/fixtures` directory.
+
+### fixtures.fixturesDir
+
+* [<String>]
+
+The absolute path to the `test/fixtures/` directory.
+
+### fixtures.path(...args)
+
+* `...args` [<String>]
+
+Returns the result of `path.join(fixtures.fixturesDir, ...args)`.
+
+### fixtures.readSync(args[, enc])
+
+* `args` [<String>] | [<Array>]
+
+Returns the result of
+`fs.readFileSync(path.join(fixtures.fixturesDir, ...args), 'enc')`.
+
+### fixtures.readKey(arg[, enc])
+
+* `arg` [<String>]
+
+Returns the result of
+`fs.readFileSync(path.join(fixtures.fixturesDir, 'keys', arg), 'enc')`.
+
 ## WPT Module
 
 The wpt.js module is a port of parts of

test/common/fixtures.js

@@ -0,0 +1,28 @@
+/* eslint-disable required-modules */
+'use strict';
+
+const path = require('path');
+const fs = require('fs');
+
+const fixturesDir = path.join(__dirname, '..', 'fixtures');
+
+function fixturesPath(...args) {
+  return path.join(fixturesDir, ...args);
+}
+
+function readFixtureSync(args, enc) {
+  if (Array.isArray(args))
+    return fs.readFileSync(fixturesPath(...args), enc);
+  return fs.readFileSync(fixturesPath(args), enc);
+}
+
+function readFixtureKey(name, enc) {
+  return fs.readFileSync(fixturesPath('keys', name), enc);
+}
+
+module.exports = {
+  fixturesDir,
+  path: fixturesPath,
+  readSync: readFixtureSync,
+  readKey: readFixtureKey
+};

test/common/index.js

@@ -8,13 +8,15 @@ const { exec, execSync, spawn, spawnSync } = require('child_process');
 const stream = require('stream');
 const util = require('util');
 const Timer = process.binding('timer_wrap').Timer;
+const { fixturesDir } = require('./fixtures');
 
 const testRoot = process.env.NODE_TEST_DIR ?
   fs.realpathSync(process.env.NODE_TEST_DIR) : path.resolve(__dirname, '..');
 
 const noop = () => {};
 
-exports.fixturesDir = path.join(__dirname, '..', 'fixtures');
+exports.fixturesDir = fixturesDir;
+
 exports.tmpDirName = 'tmp';
 // PORT should match the definition in test/testpy/__init__.py.
 exports.PORT = +process.env.NODE_COMMON_PORT || 12346;

test/doctool/test-doctool-html.js

@@ -103,18 +103,20 @@ testData.forEach((item) => {
           assert.ifError(err);
 
           const actual = output.replace(spaces, '');
+          const scriptDomain = 'google-analytics.com';
           // Assert that the input stripped of all whitespace contains the
           // expected list
           assert(actual.includes(expected));
 
           // Testing the insertion of Google Analytics script when
           // an analytics id is provided. Should not be present by default
           if (includeAnalytics) {
-            assert(actual.includes('google-analytics.com'),
-                   'Google Analytics script was not present');
+            assert(actual.includes(scriptDomain),
+                   `Google Analytics script was not present in "${actual}"`);
           } else {
-            assert.strictEqual(actual.includes('google-analytics.com'), false,
-                               'Google Analytics script was present');
+            assert.strictEqual(actual.includes(scriptDomain), false,
+                               'Google Analytics script was present in ' +
+                               `"${actual}"`);
           }
         }));
     }));

test/parallel/test-buffer-indexof.js

@@ -209,26 +209,26 @@ assert.strictEqual(
 
   // Test single char pattern
   assert.strictEqual(0, twoByteString.indexOf('\u039a', 0, 'ucs2'));
-  assert.strictEqual(2, twoByteString.indexOf('\u0391', 0, 'ucs2'),
-                     'Alpha');
-  assert.strictEqual(4, twoByteString.indexOf('\u03a3', 0, 'ucs2'),
-                     'First Sigma');
-  assert.strictEqual(6, twoByteString.indexOf('\u03a3', 6, 'ucs2'),
-                     'Second Sigma');
-  assert.strictEqual(8, twoByteString.indexOf('\u0395', 0, 'ucs2'),
-                     'Epsilon');
-  assert.strictEqual(-1, twoByteString.indexOf('\u0392', 0, 'ucs2'),
-                     'Not beta');
+  let index = twoByteString.indexOf('\u0391', 0, 'ucs2');
+  assert.strictEqual(2, index, `Alpha - at index ${index}`);
+  index = twoByteString.indexOf('\u03a3', 0, 'ucs2');
+  assert.strictEqual(4, index, `First Sigma - at index ${index}`);
+  index = twoByteString.indexOf('\u03a3', 6, 'ucs2');
+  assert.strictEqual(6, index, `Second Sigma - at index ${index}`);
+  index = twoByteString.indexOf('\u0395', 0, 'ucs2');
+  assert.strictEqual(8, index, `Epsilon - at index ${index}`);
+  index = twoByteString.indexOf('\u0392', 0, 'ucs2');
+  assert.strictEqual(-1, index, `Not beta - at index ${index}`);
 
   // Test multi-char pattern
-  assert.strictEqual(
-    0, twoByteString.indexOf('\u039a\u0391', 0, 'ucs2'), 'Lambda Alpha');
-  assert.strictEqual(
-    2, twoByteString.indexOf('\u0391\u03a3', 0, 'ucs2'), 'Alpha Sigma');
-  assert.strictEqual(
-    4, twoByteString.indexOf('\u03a3\u03a3', 0, 'ucs2'), 'Sigma Sigma');
-  assert.strictEqual(
-    6, twoByteString.indexOf('\u03a3\u0395', 0, 'ucs2'), 'Sigma Epsilon');
+  index = twoByteString.indexOf('\u039a\u0391', 0, 'ucs2');
+  assert.strictEqual(0, index, `Lambda Alpha - at index ${index}`);
+  index = twoByteString.indexOf('\u0391\u03a3', 0, 'ucs2');
+  assert.strictEqual(2, index, `Alpha Sigma - at index ${index}`);
+  index = twoByteString.indexOf('\u03a3\u03a3', 0, 'ucs2');
+  assert.strictEqual(4, index, `Sigma Sigma - at index ${index}`);
+  index = twoByteString.indexOf('\u03a3\u0395', 0, 'ucs2');
+  assert.strictEqual(6, index, `Sigma Epsilon - at index ${index}`);
 }
 
 const mixedByteStringUtf8 = Buffer.from('\u039a\u0391abc\u03a3\u03a3\u0395');
@@ -255,16 +255,18 @@ for (let i = 0; i < longBufferString.length - pattern.length; i += 7) {
   assert.strictEqual((i + 15) & ~0xf, index,
                      `Long ABACABA...-string at index ${i}`);
 }
-assert.strictEqual(510, longBufferString.indexOf('AJABACA'),
-                   'Long AJABACA, First J');
-assert.strictEqual(
-  1534, longBufferString.indexOf('AJABACA', 511), 'Long AJABACA, Second J');
+
+let index = longBufferString.indexOf('AJABACA');
+assert.strictEqual(510, index, `Long AJABACA, First J - at index ${index}`);
+index = longBufferString.indexOf('AJABACA', 511);
+assert.strictEqual(1534, index, `Long AJABACA, Second J - at index ${index}`);
 
 pattern = 'JABACABADABACABA';
+index = longBufferString.indexOf(pattern);
+assert.strictEqual(511, index, `Long JABACABA..., First J - at index ${index}`);
+index = longBufferString.indexOf(pattern, 512);
 assert.strictEqual(
-  511, longBufferString.indexOf(pattern), 'Long JABACABA..., First J');
-assert.strictEqual(
-  1535, longBufferString.indexOf(pattern, 512), 'Long JABACABA..., Second J');
+  1535, index, `Long JABACABA..., Second J - at index ${index}`);
 
 // Search for a non-ASCII string in a pure ASCII string.
 const asciiString = Buffer.from(

test/parallel/test-buffer-sharedarraybuffer.js

@@ -16,15 +16,15 @@ arr2[1] = 4000;
 const arr_buf = Buffer.from(arr1.buffer);
 const ar_buf = Buffer.from(arr2.buffer);
 
-assert.deepStrictEqual(arr_buf, ar_buf, 0);
+assert.deepStrictEqual(arr_buf, ar_buf);
 
 arr1[1] = 6000;
 arr2[1] = 6000;
 
-assert.deepStrictEqual(arr_buf, ar_buf, 0);
+assert.deepStrictEqual(arr_buf, ar_buf);
 
 // Checks for calling Buffer.byteLength on a SharedArrayBuffer
 
-assert.strictEqual(Buffer.byteLength(sab), sab.byteLength, 0);
+assert.strictEqual(Buffer.byteLength(sab), sab.byteLength);
 
 assert.doesNotThrow(() => Buffer.from({buffer: sab}));

test/parallel/test-buffer-zero-fill-reset.js

@@ -15,5 +15,5 @@ function testUint8Array(ui) {
 for (let i = 0; i < 100; i++) {
   Buffer.alloc(0);
   const ui = new Uint8Array(65);
-  assert.ok(testUint8Array(ui), 'Uint8Array is not zero-filled');
+  assert.ok(testUint8Array(ui), `Uint8Array is not zero-filled: ${ui}`);
 }

test/parallel/test-child-process-detached.js

@@ -1,11 +1,10 @@
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
-const path = require('path');
+const fixtures = require('../common/fixtures');
 
 const spawn = require('child_process').spawn;
-const childPath = path.join(common.fixturesDir,
-                            'parent-process-nonpersistent.js');
+const childPath = fixtures.path('parent-process-nonpersistent.js');
 let persistentPid = -1;
 
 const child = spawn(process.execPath, [ childPath ]);

test/parallel/test-child-process-execfile.js

@@ -2,10 +2,10 @@
 const common = require('../common');
 const assert = require('assert');
 const execFile = require('child_process').execFile;
-const path = require('path');
 const uv = process.binding('uv');
+const fixtures = require('../common/fixtures');
 
-const fixture = path.join(common.fixturesDir, 'exit.js');
+const fixture = fixtures.path('exit.js');
 
 {
   execFile(

test/parallel/test-child-process-exit-code.js

@@ -2,18 +2,17 @@
 const common = require('../common');
 const assert = require('assert');
 const spawn = require('child_process').spawn;
-const path = require('path');
+const fixtures = require('../common/fixtures');
 
-const exitScript = path.join(common.fixturesDir, 'exit.js');
+const exitScript = fixtures.path('exit.js');
 const exitChild = spawn(process.argv[0], [exitScript, 23]);
 exitChild.on('exit', common.mustCall(function(code, signal) {
   assert.strictEqual(code, 23);
   assert.strictEqual(signal, null);
 }));
 
 
-const errorScript = path.join(common.fixturesDir,
-                              'child_process_should_emit_error.js');
+const errorScript = fixtures.path('child_process_should_emit_error.js');
 const errorChild = spawn(process.argv[0], [errorScript]);
 errorChild.on('exit', common.mustCall(function(code, signal) {
   assert.ok(code !== 0);

test/parallel/test-child-process-fork-close.js

@@ -2,8 +2,9 @@
 const common = require('../common');
 const assert = require('assert');
 const fork = require('child_process').fork;
+const fixtures = require('../common/fixtures');
 
-const cp = fork(`${common.fixturesDir}/child-process-message-and-exit.js`);
+const cp = fork(fixtures.path('child-process-message-and-exit.js'));
 
 let gotMessage = false;
 let gotExit = false;

test/parallel/test-child-process-fork.js

@@ -3,8 +3,9 @@ const common = require('../common');
 const assert = require('assert');
 const fork = require('child_process').fork;
 const args = ['foo', 'bar'];
+const fixtures = require('../common/fixtures');
 
-const n = fork(`${common.fixturesDir}/child-process-spawn-node.js`, args);
+const n = fork(fixtures.path('child-process-spawn-node.js'), args);
 assert.deepStrictEqual(args, ['foo', 'bar']);
 
 n.on('message', function(m) {

test/parallel/test-child-process-fork3.js

@@ -1,5 +1,6 @@
 'use strict';
-const common = require('../common');
+require('../common');
 const child_process = require('child_process');
+const fixtures = require('../common/fixtures');
 
-child_process.fork(`${common.fixturesDir}/empty.js`); // should not hang
+child_process.fork(fixtures.path('empty.js')); // should not hang

test/parallel/test-child-process-ipc.js

@@ -1,13 +1,12 @@
 'use strict';
 
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 
-const spawn = require('child_process').spawn;
+const { spawn } = require('child_process');
+const fixtures = require('../common/fixtures');
 
-const path = require('path');
-
-const sub = path.join(common.fixturesDir, 'echo.js');
+const sub = fixtures.path('echo.js');
 
 let gotHelloWorld = false;
 let gotEcho = false;