tools: fix skip detection of test runner output

[richardlau]

Fix the Python test harness so that it no longer treats the # skipped part of the summary at the end of the built-in test runner output as marking the test as skipped.

Fixes: #50346

---

So it turns out that the Python test runner scans the output of the tests to look for SKIP directives:

node/tools/test.py

Line 86 in d335487

skip_regex = re.compile(r'# SKIP\S*\s+(.*)', re.IGNORECASE)

node/tools/test.py

Lines 388 to 391 in d335487

	skip = skip_regex.search(output.output.stdout)
	if skip:
	logger.info(
	'ok %i %s # skip %s' % (self._done, command, skip.group(1)))

(this regex is based on http://testanything.org/tap-specification.html#skipping-tests).

For the tests that are using the built-in test runner, this is matching the # skipped line in the summary at the end of the output,
e.g.

$ ./node --test --test-reporter=tap test/parallel/test-dotenv-node-options.js
TAP version 13
# Subtest: .env supports NODE_OPTIONS
    # Subtest: should have access to permission scope
    ok 1 - should have access to permission scope
      ---
      duration_ms: 26.0744
      ...
    # Subtest: validate only read permissions are enabled
    ok 2 - validate only read permissions are enabled
      ---
      duration_ms: 22.168635
      ...
    # Subtest: TZ environment variable
    ok 3 - TZ environment variable
      ---
      duration_ms: 19.970299
      ...
    1..3
ok 1 - .env supports NODE_OPTIONS
  ---
  duration_ms: 69.387679
  type: 'suite'
  ...
1..1
# tests 3
# suites 1
# pass 3
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 144.850498
$

which causes the Python test harness to insert a SKIP directive with the number of skipped tests (e.g. 0 in this case) as the reason for skipping:

$ ./tools/test.py --progress=tap parallel/test-dotenv-node-options
TAP version 13
1..1
ok 1 parallel/test-dotenv-node-options # skip 0
  ---
  duration_ms: 213.30700
  ...

All tests passed.
$

With the updated regex:

$ ./tools/test.py --progress=tap parallel/test-dotenv-node-options
TAP version 13
1..1
ok 1 parallel/test-dotenv-node-options
  ---
  duration_ms: 212.83200
  ...

All tests passed.
$

cc @nodejs/test_runner

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59924/

[aduh95]

Wouldn't we want to not match line returns?

Suggested change

	skip_regex = re.compile(r'(?:\d+\.\.\d+|ok|not ok).*# SKIP\S*\s+(.*)', re.IGNORECASE)
	skip_regex = re.compile(r'(?:\d+\.\.\d+|ok|not ok)[^\n]*# SKIP\S*\s+(.*)', re.IGNORECASE)

[richardlau]

The . character already excludes newlines.

https://docs.python.org/3/library/re.html

(Dot.) In the default mode, this matches any character except a newline. If the DOTALL flag has been specified, this matches any character including a newline.

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/53545
✔  Done loading data for nodejs/node/pull/53545
----------------------------------- PR info ------------------------------------
Title      tools: fix skip detection of test runner output (#53545)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     richardlau:testharnessskip -> nodejs:main
Labels     test, tools
Commits    1
 - tools: fix skip detection of test runner output
Committers 1
 - Richard Lau 
PR-URL: https://github.com/nodejs/node/pull/53545
Fixes: https://github.com/nodejs/node/issues/50346
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Luigi Pinca 
Reviewed-By: Chemi Atlow 
Reviewed-By: Moshe Atlow 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53545
Fixes: https://github.com/nodejs/node/issues/50346
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Luigi Pinca 
Reviewed-By: Chemi Atlow 
Reviewed-By: Moshe Atlow 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 22 Jun 2024 03:56:35 GMT
   ✔  Approvals: 4
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/53545#pullrequestreview-2133822428
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/53545#pullrequestreview-2133854918
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/53545#pullrequestreview-2133920028
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/53545#pullrequestreview-2133923840
   ✘  Last GitHub CI failed
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/9645012534

[nodejs-github-bot]

Landed in 2068c40