test: remove Flags: functionality from Python test runner

Remove the processing of `Flags:` comments and associated documentation. This is now handled by `common.relaunchWithFlags()`.
nodejs · Trott · Dec 19, 2018 · Dec 19, 2018 · Dec 19, 2018 · Dec 19, 2018
commit 97b210183b8b35c53591970ee346b17b21ce2bac
diff --git a/doc/guides/writing-tests.md b/doc/guides/writing-tests.md
@@ -248,18 +248,15 @@ 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.relaunchWithFlags()`][] or the more specific
+helper functions [`common.exposeInternals()`][] or
+[`common.experimentalWorker()`][]:
 
 ```javascript
 'use strict';
 
-// Flags: --expose-internals
-
-require('../common');
-const assert = require('assert');
+const common = require('../common');
+common.exposeInternals();
 const freelist = require('internal/freelist');
 ```
 
@@ -399,6 +396,9 @@ 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.experimentalWorker()`]: https://github.com/nodejs/node/blob/master/test/common/README.md#experimentalworker
+[`common.exposeInternals()`]: https://github.com/nodejs/node/blob/master/test/common/README.md#exposeinternals
+[`common.relaunchWithFlags()`]: https://github.com/nodejs/node/blob/master/test/common/README.md#relaunchwithflagsflags
 [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

diff --git a/test/common/README.md b/test/common/README.md
@@ -319,6 +319,12 @@ spawn(...common.pwdCommand, { stdio: ['pipe'] });
 
 Relaunches the test adding the specified command-line flags.
 
+```js
+const common = require('../common');
+if (process.argv.includes('--preserve-symlinks'))
+  common.relaunchWithFlags(['--preserve-symlinks']);
+```
+
 ### rootDir
 * [&lt;string>]
 

diff --git a/test/message/testcfg.py b/test/message/testcfg.py
@@ -41,8 +41,6 @@
 except NameError:
     xrange = range  # Python 3
 
-FLAGS_PATTERN = re.compile(r"//\s+Flags:(.*)")
-
 class MessageTestCase(test.TestCase):
 
   def __init__(self, path, file, expected, arch, mode, context, config):
@@ -105,10 +103,6 @@ def GetName(self):
 
   def GetCommand(self):
     result = [self.config.context.GetVm(self.arch, self.mode)]
-    source = open(self.file).read()
-    flags_match = FLAGS_PATTERN.search(source)
-    if flags_match:
-      result += flags_match.group(1).strip().split()
     if self.file.endswith('.mjs'):
       result += ['--experimental-modules']
     result.append(self.file)

diff --git a/test/pseudo-tty/testcfg.py b/test/pseudo-tty/testcfg.py
@@ -43,8 +43,6 @@
 except NameError:
   xrange = range  # Python 3
 
-FLAGS_PATTERN = re.compile(r"//\s+Flags:(.*)")
-
 class TTYTestCase(test.TestCase):
 
   def __init__(self, path, file, expected, input, arch, mode, context, config):
@@ -104,10 +102,6 @@ def GetName(self):
 
   def GetCommand(self):
     result = [self.config.context.GetVm(self.arch, self.mode)]
-    source = open(self.file).read()
-    flags_match = FLAGS_PATTERN.search(source)
-    if flags_match:
-      result += flags_match.group(1).strip().split()
     result.append(self.file)
     return result
 

diff --git a/test/testpy/__init__.py b/test/testpy/__init__.py
@@ -37,9 +37,6 @@
   from functools import reduce
 
 
-FLAGS_PATTERN = re.compile(r"//\s+Flags:(.*)")
-
-
 class SimpleTestCase(test.TestCase):
 
   def __init__(self, path, file, arch, mode, context, config, additional=None):
@@ -62,30 +59,6 @@ def GetName(self):
 
   def GetCommand(self):
     result = [self.config.context.GetVm(self.arch, self.mode)]
-    source = open(self.file).read()
-    flags_match = FLAGS_PATTERN.search(source)
-    if flags_match:
-      flags = flags_match.group(1).strip().split()
-      # The following block reads config.gypi to extract the v8_enable_inspector
-      # value. This is done to check if the inspector is disabled in which case
-      # the '--inspect' flag cannot be passed to the node process as it will
-      # cause node to exit and report the test as failed. The use case
-      # is currently when Node is configured --without-ssl and the tests should
-      # still be runnable but skip any tests that require ssl (which includes the
-      # inspector related tests). Also, if there is no ssl support the options
-      # '--use-bundled-ca' and '--use-openssl-ca' will also cause a similar
-      # failure so such tests are also skipped.
-      if (any(flag.startswith('--inspect') for flag in flags) and
-          not self.context.v8_enable_inspector):
-        print(': Skipping as node was compiled without inspector support')
-      elif (('--use-bundled-ca' in flags or
-          '--use-openssl-ca' in flags or
-          '--tls-v1.0' in flags or
-          '--tls-v1.1' in flags) and
-          not self.context.node_has_crypto):
-        print(': Skipping as node was compiled without crypto support')
-      else:
-        result += flags
 
     if self.additional_flags:
       result += self.additional_flags