test,tools: move Flags: comment processing from Python to JavaScript

doc/guides/writing-tests.md

@@ -248,18 +248,13 @@ fs.readFile('test-file').then(
 ### Flags
 
 Some tests will require running Node.js with specific command line flags set. To
-accomplish this, add a `// Flags: ` comment in the preamble of the
-test followed by the flags. For example, to allow a test to require some of the
-`internal/*` modules, add the `--expose-internals` flag.
-A test that would require `internal/freelist` could start like this:
+accomplish this, use [`common.requireFlags()`][].
 
 ```javascript
 'use strict';
 
-// Flags: --expose-internals
-
-require('../common');
-const assert = require('assert');
+const common = require('../common');
+common.requireFlags('--expose-internals');
 const freelist = require('internal/freelist');
 ```
 
@@ -399,6 +394,7 @@ To generate a test coverage report, see the
 [ASCII]: http://man7.org/linux/man-pages/man7/ascii.7.html
 [Google Test]: https://github.com/google/googletest
 [`common` module]: https://github.com/nodejs/node/blob/master/test/common/README.md
+[`common.requireFlags()`]: https://github.com/nodejs/node/blob/master/test/common/README.md#requireflagsflags
 [all maintained branches]: https://github.com/nodejs/lts
 [node.green]: http://node.green/
 [test fixture]: https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#test-fixtures-using-the-same-data-configuration-for-multiple-tests

test/abort/test-addon-uv-handle-leak.js

@@ -1,6 +1,7 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../common');
+common.requireFlags('--experimental-worker');
+
 const assert = require('assert');
 const fs = require('fs');
 const path = require('path');

test/addons/buffer-free-callback/test.js

@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
 const common = require('../../common');
+common.requireFlags('--expose-gc');
 const binding = require(`./build/${common.buildType}/binding`);
 
 function check(size, alignment, offset) {

test/addons/hello-world/test-worker.js

@@ -1,6 +1,6 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../../common');
+common.requireFlags('--experimental-worker');
 const assert = require('assert');
 const path = require('path');
 const { Worker } = require('worker_threads');

test/addons/null-buffer-neuter/test.js

@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
 const common = require('../../common');
+common.requireFlags('--expose-gc');
 const binding = require(`./build/${common.buildType}/binding`);
 
 binding.run();

test/addons/stringbytes-external-exceed-max/test-stringbytes-external-exceed-max-by-1-binary.js

@@ -1,11 +1,12 @@
-// Flags: --expose-gc
 'use strict';
 
 const common = require('../../common');
 const skipMessage = 'intensive toString tests due to memory confinements';
 if (!common.enoughTestMem)
   common.skip(skipMessage);
 
+common.requireFlags('--expose-gc');
+
 const binding = require(`./build/${common.buildType}/binding`);
 const assert = require('assert');
 

test/addons/worker-addon/test.js

@@ -1,6 +1,6 @@
-// Flags: --experimental-worker
 'use strict';
 const common = require('../../common');
+common.requireFlags('--experimental-worker');
 const assert = require('assert');
 const child_process = require('child_process');
 const path = require('path');

test/async-hooks/init-hooks.js

@@ -1,5 +1,4 @@
 'use strict';
-// Flags: --expose-gc
 
 const common = require('../common');
 const assert = require('assert');

test/async-hooks/test-emit-before-after.js

@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-internals
 
 const common = require('../common');
+common.requireFlags('--expose-internals');
 const assert = require('assert');
 const spawnSync = require('child_process').spawnSync;
 const async_hooks = require('internal/async_hooks');

test/async-hooks/test-emit-init.js

@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-internals
 
 const common = require('../common');
+common.requireFlags('--expose-internals');
 const assert = require('assert');
 const spawnSync = require('child_process').spawnSync;
 const async_hooks = require('internal/async_hooks');

test/async-hooks/test-httpparser.request.js

@@ -1,7 +1,7 @@
-// Flags: --expose-internals
 'use strict';
 
 const common = require('../common');
+common.requireFlags('--expose-internals');
 const assert = require('assert');
 const tick = require('../common/tick');
 const initHooks = require('./init-hooks');

test/async-hooks/test-httpparser.response.js

@@ -1,7 +1,7 @@
-// Flags: --expose-internals
 'use strict';
 
 const common = require('../common');
+common.requireFlags('--expose-internals');
 const assert = require('assert');
 const tick = require('../common/tick');
 const initHooks = require('./init-hooks');

test/async-hooks/test-no-assert-when-disabled.js

@@ -1,10 +1,11 @@
 'use strict';
-// Flags: --no-force-async-hooks-checks --expose-internals
 const common = require('../common');
 
 if (!common.isMainThread)
   common.skip('Workers don\'t inherit per-env state like the check flag');
 
+common.requireFlags('--expose-internals', '--no-force-async-hooks-checks');
+
 const async_hooks = require('internal/async_hooks');
 
 // Negative asyncIds and invalid type name

test/async-hooks/test-pipewrap.js

@@ -1,18 +1,17 @@
 // NOTE: this also covers process wrap as one is created along with the pipes
 // when we launch the sleep process
 'use strict';
-// Flags: --expose-gc
 
 const common = require('../common');
+if (!common.isMainThread)
+  common.skip('Worker bootstrapping works differently -> different async IDs');
+common.requireFlags('--expose-gc');
 const assert = require('assert');
 const tick = require('../common/tick');
 const initHooks = require('./init-hooks');
 const { checkInvocations } = require('./hook-checks');
 const { spawn } = require('child_process');
 
-if (!common.isMainThread)
-  common.skip('Worker bootstrapping works differently -> different async IDs');
-
 const hooks = initHooks();
 
 hooks.enable();

test/async-hooks/test-querywrap.js

@@ -1,7 +1,7 @@
 'use strict';
-// Flags: --expose-gc
 
 const common = require('../common');
+common.requireFlags('--expose-gc');
 const assert = require('assert');
 const tick = require('../common/tick');
 const initHooks = require('./init-hooks');

test/async-hooks/test-zlib.zlib-binding.deflate.js

@@ -1,7 +1,7 @@
-// Flags: --expose-internals
 'use strict';
 
 const common = require('../common');
+common.requireFlags('--expose-internals');
 const assert = require('assert');
 const initHooks = require('./init-hooks');
 const { checkInvocations } = require('./hook-checks');

test/code-cache/test-code-cache.js

@@ -1,11 +1,11 @@
 'use strict';
 
-// Flags: --expose-internals
 // This test verifies that if the binary is compiled with code cache,
 // and the cache is used when built in modules are compiled.
 // Otherwise, verifies that no cache is used when compiling builtins.
 
-require('../common');
+const common = require('../common');
+common.requireFlags('--expose-internals');
 const assert = require('assert');
 const {
   cachableBuiltins,

test/common/README.md

@@ -304,6 +304,17 @@ const { spawn } = require('child_process');
 spawn(...common.pwdCommand, { stdio: ['pipe'] });
 ```
 
+### requireFlags(...flags)
+* `...flags` [<string>]
+
+Checks that the provided command-line flags have been provided. If not, it
+relaunches the test adding the missing flags.
+
+```js
+const common = require('../common');
+common.requireFlags('--preserve-symlinks', '--expose-internals');
+```
+
 ### rootDir
 * [<string>]
 

test/common/index.js

@@ -46,47 +46,6 @@ const isMainThread = (() => {
   return true;
 })();
 
-// Check for flags. Skip this for workers (both, the `cluster` module and
-// `worker_threads`) and child processes.
-if (process.argv.length === 2 &&
-    isMainThread &&
-    module.parent &&
-    require('cluster').isMaster) {
-  // The copyright notice is relatively big and the flags could come afterwards.
-  const bytesToRead = 1500;
-  const buffer = Buffer.allocUnsafe(bytesToRead);
-  const fd = fs.openSync(module.parent.filename, 'r');
-  fs.readSync(fd, buffer, 0, bytesToRead);
-  fs.closeSync(fd);
-  const source = buffer.toString();
-
-  const flagStart = source.indexOf('// Flags: --') + 10;
-  if (flagStart !== 9) {
-    let flagEnd = source.indexOf('\n', flagStart);
-    // Normalize different EOL.
-    if (source[flagEnd - 1] === '\r') {
-      flagEnd--;
-    }
-    const flags = source
-      .substring(flagStart, flagEnd)
-      .replace(/_/g, '-')
-      .split(' ');
-    const args = process.execArgv.map((arg) => arg.replace(/_/g, '-'));
-    for (const flag of flags) {
-      if (!args.includes(flag) &&
-          // If the binary was built without-ssl then the crypto flags are
-          // invalid (bad option). The test itself should handle this case.
-          hasCrypto &&
-          // If the binary is build without `intl` the inspect option is
-          // invalid. The test itself should handle this case.
-          (process.config.variables.v8_enable_inspector !== 0 ||
-            !flag.startsWith('--inspect'))) {
-        throw new Error(`Test has to be started with the flag: '${flag}'`);
-      }
-    }
-  }
-}
-
 const isWindows = process.platform === 'win32';
 const isAIX = process.platform === 'aix';
 const isLinuxPPCBE = (process.platform === 'linux') &&
@@ -378,6 +337,7 @@ function _mustCallInner(fn, criteria = 1, field) {
 }
 
 function hasMultiLocalhost() {
+  requireFlags('--expose-internals');
   const { internalBinding } = require('internal/test/binding');
   const { TCP, constants: TCPConstants } = internalBinding('tcp_wrap');
   const t = new TCP(TCPConstants.SOCKET);
@@ -722,6 +682,25 @@ function runWithInvalidFD(func) {
   printSkipMessage('Could not generate an invalid fd');
 }
 
+function requireFlags(...flags) {
+  if (require('cluster').isWorker)
+    return;
+  let missing = flags.filter((flag) => !process.execArgv.includes(flag));
+
+  // Special handling for worker_threads.
+  if (!isMainThread) {
+    missing = missing.filter((flag) => !/--experimental[-_]worker/.test(flag));
+  }
+
+  if (missing.length > 0) {
+    const args = [ ...missing, ...process.execArgv, ...process.argv.slice(1) ];
+    const options = { encoding: 'utf8', stdio: 'inherit' };
+    const result = spawnSync(process.execPath, args, options);
+    assert.ifError(result.error);
+    process.exit(result.status);
+  }
+}
+
 module.exports = {
   allowGlobals,
   buildType,
@@ -763,6 +742,7 @@ module.exports = {
   platformTimeout,
   printSkipMessage,
   pwdCommand,
+  requireFlags,
   rootDir,
   runWithInvalidFD,
   skip,

test/common/index.mjs

@@ -1,4 +1,3 @@
-// Flags: --experimental-modules
 /* eslint-disable node-core/required-modules */
 import common from './index.js';
 
@@ -42,10 +41,11 @@ const {
   expectsError,
   skipIfInspectorDisabled,
   skipIf32Bits,
+  disableCrashOnUnhandledRejection,
   getArrayBufferViews,
   getBufferSources,
-  disableCrashOnUnhandledRejection,
   getTTYfd,
+  requireFlags,
   runWithInvalidFD
 } = common;
 
@@ -89,9 +89,10 @@ export {
   expectsError,
   skipIfInspectorDisabled,
   skipIf32Bits,
+  disableCrashOnUnhandledRejection,
   getArrayBufferViews,
   getBufferSources,
-  disableCrashOnUnhandledRejection,
   getTTYfd,
+  requireFlags,
   runWithInvalidFD
 };

test/es-module/test-esm-basic-imports.mjs

@@ -1,4 +1,3 @@
-// Flags: --experimental-modules
 import '../common';
 import assert from 'assert';
 import ok from '../fixtures/es-modules/test-esm-ok.mjs';

test/es-module/test-esm-cyclic-dynamic-import.mjs

@@ -1,3 +1,2 @@
-// Flags: --experimental-modules
 import '../common';
 import('./test-esm-cyclic-dynamic-import');

test/es-module/test-esm-double-encoding.mjs

@@ -1,4 +1,3 @@
-// Flags: --experimental-modules
 import '../common';
 
 // Assert we can import files with `%` in their pathname.

test/es-module/test-esm-dynamic-import.js

@@ -1,4 +1,3 @@
-// Flags: --experimental-modules
 'use strict';
 const common = require('../common');
 const assert = require('assert');

test/es-module/test-esm-encoded-path.mjs

@@ -1,4 +1,3 @@
-// Flags: --experimental-modules
 import '../common';
 import assert from 'assert';
 // ./test-esm-ok.mjs

test/es-module/test-esm-error-cache.js

@@ -1,7 +1,5 @@
 'use strict';
 
-// Flags: --experimental-modules
-
 require('../common');
 const assert = require('assert');
 

test/es-module/test-esm-example-loader.js

@@ -1,6 +1,16 @@
-// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/example-loader.mjs
 /* eslint-disable node-core/required-modules */
-import assert from 'assert';
-import ok from '../fixtures/es-modules/test-esm-ok.mjs';
+if (typeof require === 'function') {
+  const common = require('../common');
+  common.requireFlags(
+    '--experimental-modules',
+    '--loader=./test/fixtures/es-module-loaders/example-loader.mjs'
+  );
+} else {
+  async function test() {
+    const { default: assert } = await import('assert');
+    const ok = await import('../fixtures/es-modules/test-esm-ok.mjs');
+    assert(ok);
+  }
 
-assert(ok);
+  test().catch((e) => { console.error(e); process.exit(1); });
+}

test/es-module/test-esm-forbidden-globals.mjs

@@ -1,4 +1,3 @@
-// Flags: --experimental-modules
 import '../common';
 
 // eslint-disable-next-line no-undef

test/es-module/test-esm-import-meta.mjs

@@ -1,5 +1,3 @@
-// Flags: --experimental-modules
-
 import '../common';
 import assert from 'assert';