test: skip failing tests and set the default value of strip-types to false when compiled without amaro

[y-okt]

Summary

This PR is to address #60640 . When compiled without amaro, some tests fail. This is because --strip-types option should default to false when compiled without amaro, but it is set to true now. As suggested by @marco-ippolito , this PR addresses this issue by

• skipping failing tests
• set the default value of --strip-types to false if HAVE_AMARO is not true

Background

Amaro is a wrapper around @swc/wasm-typescript, and is used internally in Node.js for Type Stripping. When compiled without amaro, Type Stripping functionalities are not available internally. However, because --strip-types default value is true, some tests fail under that situation.
The suggested fix is as aforementioned, 1) skip the failing tests when the option is false, and set its default value to false when compiled without amaro.

Regarding the failing tests, the list below is the full list of the failing tests due to Amaro. When running with amaro, all the tests passed, and when running without amaro, the tests below failed, meaning that test failures are attributed to the existence of amaro.
Also, I confirmed that these tests are failing not due to flakiness, but amaro. Most of the failures were with the Error [ERR_NO_TYPESCRIPT]: Node.js is not compiled with TypeScript support error message. For the files which don't have this error message, the reason it's relevant to amaro is written below respectively.

List of failing tests when compiled without amaro:

1. test/parallel/test-compile-cache-typescript-commonjs.js
2. test/parallel/test-compile-cache-typescript-esm.js
3. test/parallel/test-compile-cache-typescript-strip-miss.js
4. test/parallel/test-compile-cache-typescript-transform.js
5. test/parallel/test-compile-cache-typescript-strip-sourcemaps.js
6. test/parallel/test-config-file.js
7. test/parallel/test-node-output-eval.mjs
8. test/parallel/test-node-output-sourcemaps.mjs
9. test/parallel/test-runner-coverage-default-exclusion.mjs <- Failed because Node tries to strip types from file.test.ts
10. test/parallel/test-runner-run-global-hooks.mjs
11. test/parallel/test-runner-global-setup-teardown.mjs <- Failed because typescript file is read (code)
12. test/parallel/test-util-getcallsites.js
13. test/parallel/test-worker-cli-options.js
14. test/parallel/test-worker-eval-typescript.js
15. test/parallel/test-worker-load-file-with-extension-other-than-js.js
16. test/parallel/test-worker-syntax-error.js <- Failed because type stripping is tried (code, code)
17. test/es-module/test-esm-import-meta-main-eval.mjs
18. test/es-module/test-esm-detect-ambiguous.mjs
19. test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs
20. test/test-runner/test-output-typescript-coverage.mjs

Note that the environment in which this was confirmed is M4 Mac and OS is Sequoia 15.3.2.

[nodejs-github-bot]

Review requested:

• @nodejs/config
• @nodejs/test_runner

[marco-ippolito]

Good job 👍🏼
I dont think we should skip the whole file for:

test/es-module/test-esm-import-meta-main-eval.mjs
test/es-module/test-esm-detect-ambiguous.mjs
test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs
test/parallel/test-node-output-eval.mjs
test/parallel/test-node-output-sourcemaps.mjs
test/parallel/test-runner-coverage-default-exclusion.mjs <- Failed because Node tries to strip types from file.test.ts
test/parallel/test-runner-run-global-hooks.mjs
test/parallel/test-runner-global-setup-teardown.mjs <- Failed because typescript file is read ([code](https://github.com/nodejs/node/blob/fb6b83c9eface51acad916cff365b98aa826f56a/test/parallel/test-runner-global-setup-teardown.mjs#L265))
test/parallel/test-util-getcallsites.js
test/parallel/test-worker-cli-options.js

We can skip just the failing subtest, you can pass an argument { skip: process.config.variables.node_use_amaro } to the test function if the test is using the node.js test runner

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.54%. Comparing base (08d966c) to head (c334402).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60815      +/-   ##
==========================================
- Coverage   88.54%   88.54%   -0.01%     
==========================================
  Files         703      703              
  Lines      208291   208291              
  Branches    40170    40169       -1     
==========================================
- Hits       184430   184426       -4     
- Misses      15865    15868       +3     
- Partials     7996     7997       +1     

Files with missing lines	Coverage Δ
src/node_options.cc	77.85% <ø> (ø)
src/node_options.h	97.89% <100.00%> (ø)

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kapouer]

That's great, I'll be glad to test when it's ready and before it's merged.

[y-okt]

Thank you very much for your reviews. I addressed the points. Could you please take a look again? Thank you in advance!

[marco-ippolito]

Once we fix #60815 (comment)
it will be no longer necessary to skip these test files:

test/es-module/test-esm-detect-ambiguous.mjs
test/es-module/test-esm-tla-syntax-errors-not-recognized-as-tla-error.mjs
test/parallel/test-worker-cli-options.js
test/parallel/test-worker-load-file-with-extension-other-than-js.js
test/parallel/test-worker-syntax-error.js

[y-okt]

@marco-ippolito @aduh95 Thank you for your reviews! Could you please review again?

[nodejs-github-bot]

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

[aduh95]

Tested with nix-shell --arg benchmarkTools '[]' --arg devTools '[]' --arg extraConfigFlags '["--without-amaro"]' --arg loadJSBuiltinsDynamically false --run 'make run-ci -j12', got the following:

Failed tests:
out/Release/node test/parallel/test-compile-cache-typescript-commonjs.js
out/Release/node test/parallel/test-compile-cache-typescript-esm.js

$  out/Release/node test/parallel/test-compile-cache-typescript-commonjs.js
[process 81863]: --- stderr ---
[compile cache] resolved path …/test/.tmp.0/.compile_cache_dir + v26.0.0-pre-arm64-2cc2566e-501 -> …/test/.tmp.0/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501
[compile cache] creating cache directory …/test/.tmp.0/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501...success
[compile cache] reading cache from …/test/.tmp.0/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501/493f28c6 for CommonJS …/test/fixtures/typescript/cts/test-require-commonjs.cts... no such file or directory
[compile cache] reading cache from …/test/.tmp.0/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501/5118beb5 for ESM file://…/test/fixtures/typescript/cts/test-require-commonjs.cts... no such file or directory
…/test/fixtures/typescript/cts/test-require-commonjs.cts:3
interface Foo {};
          ^^^

SyntaxError: Unexpected identifier 'Foo'
    at wrapSafe (node:internal/modules/cjs/loader:1691:18)
    at Module._compile (node:internal/modules/cjs/loader:1734:20)
    at Object..js (node:internal/modules/cjs/loader:1891:10)
    at Module.load (node:internal/modules/cjs/loader:1480:32)
    at Module._load (node:internal/modules/cjs/loader:1299:12)
    at TracingChannel.traceSync (node:diagnostics_channel:328:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:245:24)
    at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5)
    at node:internal/main/run_main_module:33:47

Node.js v26.0.0-pre
[compile cache] skip persisting ESM file://…/test/fixtures/typescript/cts/test-require-commonjs.cts because the cache was not initialized
[compile cache] skip persisting CommonJS …/test/fixtures/typescript/cts/test-require-commonjs.cts because the cache was not initialized
[compile cache] Clear deserialized cache.

[process 81863]: --- stdout ---

[process 81863]: status = 1, signal = null
…/test/common/child_process.js:112
    throw error;
    ^

Error: - process terminated with status 1, expected 0
    at Object.<anonymous> (…/test/parallel/test-compile-cache-typescript-commonjs.js:66:3)
    at Module._compile (node:internal/modules/cjs/loader:1760:14)
    at Object..js (node:internal/modules/cjs/loader:1891:10)
    at Module.load (node:internal/modules/cjs/loader:1480:32)
    at Module._load (node:internal/modules/cjs/loader:1299:12)
    at TracingChannel.traceSync (node:diagnostics_channel:328:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:245:24)
    at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5)
    at node:internal/main/run_main_module:33:47 {
  options: {
    env: {
      NODE_DEBUG_NATIVE: 'COMPILE_CACHE',
      NODE_COMPILE_CACHE: '…/test/.tmp.0/.compile_cache_dir'
    },
    cwd: '…/test/.tmp.0'
  },
  command: '…/out/Release/node …/test/fixtures/typescript/cts/test-require-commonjs.cts'
}

Node.js v26.0.0-pre
$ out/Release/node test/parallel/test-compile-cache-typescript-esm.js
[process 82022]: --- stderr ---
[compile cache] resolved path …/test/.tmp.0/.compile_cache_dir + v26.0.0-pre-arm64-2cc2566e-501 -> …/test/.tmp.0/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501
[compile cache] creating cache directory …/test/.tmp.0/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501...success
[compile cache] reading cache from …/test/.tmp.0/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501/97aac843 for CommonJS …/test/fixtures/typescript/mts/test-import-module.mts... no such file or directory
node:internal/modules/esm/get_format:185
  throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath);
        ^

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".mts" for …/test/fixtures/typescript/mts/test-import-module.mts
    at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:185:9)
    at defaultGetFormat (node:internal/modules/esm/get_format:211:36)
    at defaultLoadSync (node:internal/modules/esm/load:158:16)
    at #loadAndMaybeBlockOnLoaderThread (node:internal/modules/esm/loader:789:12)
    at #loadSync (node:internal/modules/esm/loader:809:49)
    at ModuleLoader.load (node:internal/modules/esm/loader:774:26)
    at ModuleLoader.loadAndTranslate (node:internal/modules/esm/loader:520:31)
    at #getOrCreateModuleJobAfterResolve (node:internal/modules/esm/loader:565:36)
    at afterResolve (node:internal/modules/esm/loader:618:52)
    at ModuleLoader.getOrCreateModuleJob (node:internal/modules/esm/loader:624:12) {
  code: 'ERR_UNKNOWN_FILE_EXTENSION'
}

Node.js v26.0.0-pre
[compile cache] skip persisting CommonJS …/test/fixtures/typescript/mts/test-import-module.mts because the cache was not initialized
[compile cache] Clear deserialized cache.

[process 82022]: --- stdout ---

[process 82022]: status = 1, signal = null
…/test/common/child_process.js:112
    throw error;
    ^

Error: - process terminated with status 1, expected 0
    at Object.<anonymous> (…/test/parallel/test-compile-cache-typescript-esm.js:66:3)
    at Module._compile (node:internal/modules/cjs/loader:1760:14)
    at Object..js (node:internal/modules/cjs/loader:1891:10)
    at Module.load (node:internal/modules/cjs/loader:1480:32)
    at Module._load (node:internal/modules/cjs/loader:1299:12)
    at TracingChannel.traceSync (node:diagnostics_channel:328:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:245:24)
    at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5)
    at node:internal/main/run_main_module:33:47 {
  options: {
    env: {
      NODE_DEBUG_NATIVE: 'COMPILE_CACHE',
      NODE_COMPILE_CACHE: '…/test/.tmp.0/.compile_cache_dir'
    },
    cwd: '…/test/.tmp.0'
  },
  command: '…/out/Release/node …/test/fixtures/typescript/mts/test-import-module.mts'
}

Node.js v26.0.0-pre

[y-okt]

@aduh95 Hi, I tested with nix-shell --arg benchmarkTools '[]' --arg devTools '[]' --arg loadJSBuiltinsDynamically false --run 'make test -j12' (without --without amaro option), and I observed the same errors. Thus, I think these tests are failing not because of the amaro option.

Details

=== release test-compile-cache-typescript-commonjs === Path: parallel/test-compile-cache-typescript-commonjs [process 7062]: --- stderr --- [compile cache] resolved path /Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.390/.compile_cache_dir + v26.0.0-pre-arm64-2cc2566e-501 -> /Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.390/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501 [compile cache] creating cache directory /Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.390/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501...success [compile cache] reading cache from /Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.390/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501/75e66346 for CommonJS /Users/y-okt/ghq/github.com/y-okt/node/test/fixtures/typescript/cts/test-require-commonjs.cts... no such file or directory [compile cache] reading cache from /Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.390/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501/91185ffe for ESM file:///Users/y-okt/ghq/github.com/y-okt/node/test/fixtures/typescript/cts/test-require-commonjs.cts... no such file or directory /Users/y-okt/ghq/github.com/y-okt/node/test/fixtures/typescript/cts/test-require-commonjs.cts:3 interface Foo {}; ^^^

SyntaxError: Unexpected identifier 'Foo'
at wrapSafe (node:internal/modules/cjs/loader:1691:18)
at Module._compile (node:internal/modules/cjs/loader:1734:20)
at Object..js (node:internal/modules/cjs/loader:1891:10)
at Module.load (node:internal/modules/cjs/loader:1480:32)
at Module._load (node:internal/modules/cjs/loader:1299:12)
at TracingChannel.traceSync (node:diagnostics_channel:328:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:245:24)
at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5)
at node:internal/main/run_main_module:33:47

Node.js v26.0.0-pre
[compile cache] skip persisting ESM file:///Users/y-okt/ghq/github.com/y-okt/node/test/fixtures/typescript/cts/test-require-commonjs.cts because the cache was not initialized
[compile cache] skip persisting CommonJS /Users/y-okt/ghq/github.com/y-okt/node/test/fixtures/typescript/cts/test-require-commonjs.cts because the cache was not initialized
[compile cache] Clear deserialized cache.

[process 7062]: --- stdout ---

[process 7062]: status = 1, signal = null
/Users/y-okt/ghq/github.com/y-okt/node/test/common/child_process.js:112
throw error;
^

Error: - process terminated with status 1, expected 0
at Object. (/Users/y-okt/ghq/github.com/y-okt/node/test/parallel/test-compile-cache-typescript-commonjs.js:66:3)
at Module._compile (node:internal/modules/cjs/loader:1760:14)
at Object..js (node:internal/modules/cjs/loader:1891:10)
at Module.load (node:internal/modules/cjs/loader:1480:32)
at Module._load (node:internal/modules/cjs/loader:1299:12)
at TracingChannel.traceSync (node:diagnostics_channel:328:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:245:24)
at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5)
at node:internal/main/run_main_module:33:47 {
options: {
env: {
NODE_DEBUG_NATIVE: 'COMPILE_CACHE',
NODE_COMPILE_CACHE: '/Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.390/.compile_cache_dir'
},
cwd: '/Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.390'
},
command: '/Users/y-okt/ghq/github.com/y-okt/node/out/Release/node /Users/y-okt/ghq/github.com/y-okt/node/test/fixtures/typescript/cts/test-require-commonjs.cts'
}

Node.js v26.0.0-pre
Command: out/Release/node /Users/y-okt/ghq/github.com/y-okt/node/test/parallel/test-compile-cache-typescript-commonjs.js

=== release test-compile-cache-typescript-esm ===
Path: parallel/test-compile-cache-typescript-esm
[process 7065]: --- stderr ---
[compile cache] resolved path /Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.391/.compile_cache_dir + v26.0.0-pre-arm64-2cc2566e-501 -> /Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.391/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501
[compile cache] creating cache directory /Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.391/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501...success
[compile cache] reading cache from /Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.391/.compile_cache_dir/v26.0.0-pre-arm64-2cc2566e-501/c4cb3a75 for CommonJS /Users/y-okt/ghq/github.com/y-okt/node/test/fixtures/typescript/mts/test-import-module.mts... no such file or directory
node:internal/modules/esm/get_format:185
throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath);
^

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".mts" for /Users/y-okt/ghq/github.com/y-okt/node/test/fixtures/typescript/mts/test-import-module.mts
at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:185:9)
at defaultGetFormat (node:internal/modules/esm/get_format:211:36)
at defaultLoadSync (node:internal/modules/esm/load:158:16)
at #loadAndMaybeBlockOnLoaderThread (node:internal/modules/esm/loader:789:12)
at #loadSync (node:internal/modules/esm/loader:809:49)
at ModuleLoader.load (node:internal/modules/esm/loader:774:26)
at ModuleLoader.loadAndTranslate (node:internal/modules/esm/loader:520:31)
at #getOrCreateModuleJobAfterResolve (node:internal/modules/esm/loader:565:36)
at afterResolve (node:internal/modules/esm/loader:618:52)
at ModuleLoader.getOrCreateModuleJob (node:internal/modules/esm/loader:624:12) {
code: 'ERR_UNKNOWN_FILE_EXTENSION'
}

Node.js v26.0.0-pre
[compile cache] skip persisting CommonJS /Users/y-okt/ghq/github.com/y-okt/node/test/fixtures/typescript/mts/test-import-module.mts because the cache was not initialized
[compile cache] Clear deserialized cache.

[process 7065]: --- stdout ---

[process 7065]: status = 1, signal = null
/Users/y-okt/ghq/github.com/y-okt/node/test/common/child_process.js:112
throw error;
^

Error: - process terminated with status 1, expected 0
at Object. (/Users/y-okt/ghq/github.com/y-okt/node/test/parallel/test-compile-cache-typescript-esm.js:66:3)
at Module._compile (node:internal/modules/cjs/loader:1760:14)
at Object..js (node:internal/modules/cjs/loader:1891:10)
at Module.load (node:internal/modules/cjs/loader:1480:32)
at Module._load (node:internal/modules/cjs/loader:1299:12)
at TracingChannel.traceSync (node:diagnostics_channel:328:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:245:24)
at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5)
at node:internal/main/run_main_module:33:47 {
options: {
env: {
NODE_DEBUG_NATIVE: 'COMPILE_CACHE',
NODE_COMPILE_CACHE: '/Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.391/.compile_cache_dir'
},
cwd: '/Users/y-okt/ghq/github.com/y-okt/node/test/.tmp.391'
},
command: '/Users/y-okt/ghq/github.com/y-okt/node/out/Release/node /Users/y-okt/ghq/github.com/y-okt/node/test/fixtures/typescript/mts/test-import-module.mts'
}

Node.js v26.0.0-pre
Command: out/Release/node /Users/y-okt/ghq/github.com/y-okt/node/test/parallel/test-compile-cache-typescript-esm.js

[aduh95]

They are failing because of the lack of Amaro. My guess is that because you used make test and not make run-ci, ./configure wasn't re-run and you ended up testing the previous configuration (which I assume was the one without Amaro).

[y-okt]

@aduh95 Thank you, fixed the tests to skip when compiled without amaro. Could you please review again?

[aduh95]

All tests passed

[nodejs-github-bot]

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

[nodejs-github-bot]

Landed in d09c3ff