From 8f88cee7a6a426d65ba5819ceaa27b65ad1e9c11 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 9 Dec 2025 12:30:40 +0100 Subject: [PATCH 1/2] test: simplify `test-cli-node-options-docs` --- test/parallel/test-cli-node-options-docs.js | 62 +++++++-------------- 1 file changed, 19 insertions(+), 43 deletions(-) diff --git a/test/parallel/test-cli-node-options-docs.js b/test/parallel/test-cli-node-options-docs.js index 1be5ede13ce21b..80caa29d4263ef 100644 --- a/test/parallel/test-cli-node-options-docs.js +++ b/test/parallel/test-cli-node-options-docs.js @@ -39,34 +39,10 @@ for (const [, envVar] of manPageText.matchAll(/\.It Fl (-[a-zA-Z0-9._-]+)/g)) { } for (const [, envVar, config] of nodeOptionsCC.matchAll(addOptionRE)) { - let hasTrueAsDefaultValue = false; - let isInNodeOption = false; - let isV8Option = false; - let isNoOp = false; - - if (config.includes('NoOp{}')) { - isNoOp = true; - } - - if (config.includes('kAllowedInEnvvar')) { - isInNodeOption = true; - } - if (config.includes('kDisallowedInEnvvar')) { - isInNodeOption = false; - } - - if (config.includes('V8Option{}')) { - isV8Option = true; - } - - if (/^\s*true\s*$/.test(config.split(',').pop())) { - hasTrueAsDefaultValue = true; - } - - // Exception for HAVE_AMARO conditional default (defaults to true when Amaro is available) - if (config.includes('HAVE_AMARO')) { - hasTrueAsDefaultValue = true; - } + const hasTrueAsDefaultValue = /,\s*(?:true|HAVE_[A-Z_]+)\s*$/.test(config); + const isInNodeOption = config.includes('kAllowedInEnvvar') && !config.includes('kDisallowedInEnvvar'); + const isV8Option = config.includes('V8Option{}'); + const isNoOp = config.includes('NoOp{}'); if ( envVar.startsWith('[') || @@ -79,27 +55,27 @@ for (const [, envVar, config] of nodeOptionsCC.matchAll(addOptionRE)) { } // Internal API options are documented in /doc/contributing/internal-api.md - if (new RegExp(`####.*\`${envVar}[[=\\s\\b\`]`).test(internalApiText) === true) { + if (new RegExp(`####.*\`${RegExp.escape(envVar)}[[=\\s\\b\`]`).test(internalApiText)) { manPagesOptions.delete(envVar.slice(1)); continue; } // CLI options if (!isV8Option && !hasTrueAsDefaultValue) { - if (new RegExp(`###.*\`${envVar}[[=\\s\\b\`]`).test(cliText) === false) { - assert(false, `Should have option ${envVar} documented`); + if (!new RegExp(`###.*\`${RegExp.escape(envVar)}[[=\\s\\b\`]`).test(cliText)) { + assert.fail(`Should have option ${envVar} documented`); } else { manPagesOptions.delete(envVar.slice(1)); } } - if (!hasTrueAsDefaultValue && new RegExp(`###.*\`--no${envVar.slice(1)}[[=\\s\\b\`]`).test(cliText) === true) { - assert(false, `Should not have option --no${envVar.slice(1)} documented`); + if (!hasTrueAsDefaultValue && new RegExp(`###.*\`--no${RegExp.escape(envVar.slice(1))}[[=\\s\\b\`]`).test(cliText)) { + assert.fail(`Should not have option --no${envVar.slice(1)} documented`); } if (!isV8Option && hasTrueAsDefaultValue) { - if (new RegExp(`###.*\`--no${envVar.slice(1)}[[=\\s\\b\`]`).test(cliText) === false) { - assert(false, `Should have option --no${envVar.slice(1)} documented`); + if (!new RegExp(`###.*\`--no${RegExp.escape(envVar.slice(1))}[[=\\s\\b\`]`).test(cliText)) { + assert.fail(`Should have option --no${envVar.slice(1)} documented`); } else { manPagesOptions.delete(`-no${envVar.slice(1)}`); } @@ -107,22 +83,22 @@ for (const [, envVar, config] of nodeOptionsCC.matchAll(addOptionRE)) { // NODE_OPTIONS if (isInNodeOption && !hasTrueAsDefaultValue && - new RegExp(`\`${envVar}\``).test(nodeOptionsText) === false) { - assert(false, `Should have option ${envVar} in NODE_OPTIONS documented`); + !new RegExp(`\`${RegExp.escape(envVar)}\``).test(nodeOptionsText)) { + assert.fail(`Should have option ${envVar} in NODE_OPTIONS documented`); } - if (isInNodeOption && hasTrueAsDefaultValue && new RegExp(`\`--no${envVar.slice(1)}\``).test(cliText) === false) { - assert(false, `Should have option --no${envVar.slice(1)} in NODE_OPTIONS documented`); + if (isInNodeOption && hasTrueAsDefaultValue && !new RegExp(`\`--no${RegExp.escape(envVar.slice(1))}\``).test(cliText)) { + assert.fail(`Should have option --no${envVar.slice(1)} in NODE_OPTIONS documented`); } - if (!hasTrueAsDefaultValue && new RegExp(`\`--no${envVar.slice(1)}\``).test(cliText) === true) { - assert(false, `Should not have option --no${envVar.slice(1)} in NODE_OPTIONS documented`); + if (!hasTrueAsDefaultValue && new RegExp(`\`--no${RegExp.escape(envVar.slice(1))}\``).test(cliText)) { + assert.fail(`Should not have option --no${envVar.slice(1)} in NODE_OPTIONS documented`); } // V8 options if (isV8Option) { - if (new RegExp(`###.*\`${envVar}[[=\\s\\b\`]`).test(v8OptionsText) === false) { - assert(false, `Should have option ${envVar} in V8 options documented`); + if (!new RegExp(`###.*\`${RegExp.escape(envVar)}[[=\\s\\b\`]`).test(v8OptionsText)) { + assert.fail(`Should have option ${envVar} in V8 options documented`); } else { manPagesOptions.delete(envVar.slice(1)); } From 672f50efd3a8740067a8ae17581015c75c0868c0 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 30 Nov 2025 16:48:29 +0100 Subject: [PATCH 2/2] lib,src,test: fix tests without SQLite --- .github/workflows/test-shared.yml | 2 +- node.gyp | 2 -- node.gypi | 5 +++++ src/node_options.cc | 2 +- src/node_options.h | 2 +- test/common/index.js | 4 +++- test/module-hooks/test-module-hooks-builtin-require.js | 5 ++++- .../test-module-hooks-load-builtin-require.js | 5 ++++- test/parallel/test-global.js | 5 +++-- test/parallel/test-sqlite-config.js | 2 +- test/parallel/test-sqlite-template-tag.js | 4 +++- test/parallel/test-webstorage-without-sqlite.js | 8 ++++++++ typings/internalBinding/config.d.ts | 1 + 13 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 test/parallel/test-webstorage-without-sqlite.js diff --git a/.github/workflows/test-shared.yml b/.github/workflows/test-shared.yml index 176f28bcdbec3b..afe94d8452854a 100644 --- a/.github/workflows/test-shared.yml +++ b/.github/workflows/test-shared.yml @@ -192,7 +192,7 @@ jobs: --arg ccache '(import {}).sccache' \ --arg devTools '[]' \ --arg benchmarkTools '[]' \ - ${{ endsWith(matrix.system, '-darwin') && '--arg extraConfigFlags ''["--without-amaro" "--without-inspector" "--without-node-options"]'' \' || '\' }} + ${{ endsWith(matrix.system, '-darwin') && '--arg withAmaro false --arg withSQLite false --arg extraConfigFlags ''["--without-inspector" "--without-node-options"]'' \' || '\' }} --run ' make -C "$TAR_DIR" run-ci -j4 V=1 TEST_CI_ARGS="-p actions --measure-flakiness 9 --skip-tests=$CI_SKIP_TESTS" ' diff --git a/node.gyp b/node.gyp index c236d20e2bd37e..e1033654a672fd 100644 --- a/node.gyp +++ b/node.gyp @@ -933,7 +933,6 @@ 'sources': [ '<@(node_sqlite_sources)', ], - 'defines': [ 'HAVE_SQLITE=1' ], }], [ 'node_shared=="true" and node_module_version!="" and OS!="win"', { 'product_extension': '<(shlib_suffix)', @@ -982,7 +981,6 @@ 'sources': [ '<@(node_sqlite_sources)', ], - 'defines': [ 'HAVE_SQLITE=1' ], }], [ 'OS in "linux freebsd mac solaris openharmony" and ' 'target_arch=="x64" and ' diff --git a/node.gypi b/node.gypi index 667deb50577252..8ba67f53192159 100644 --- a/node.gypi +++ b/node.gypi @@ -436,5 +436,10 @@ }, { 'defines': [ 'HAVE_AMARO=0' ] }], + [ 'node_use_sqlite=="true"', { + 'defines': [ 'HAVE_SQLITE=1' ], + }, { + 'defines': [ 'HAVE_SQLITE=0' ] + }], ], } diff --git a/src/node_options.cc b/src/node_options.cc index 5ae511a52ad3d2..cd8387068df5ac 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -577,7 +577,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "experimental Web Storage API", &EnvironmentOptions::webstorage, kAllowedInEnvvar, - true); + HAVE_SQLITE); AddAlias("--webstorage", "--experimental-webstorage"); AddOption("--localstorage-file", "file used to persist localStorage data", diff --git a/src/node_options.h b/src/node_options.h index 2d30c115dcb27e..5aee848802558b 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -127,7 +127,7 @@ class EnvironmentOptions : public Options { bool experimental_fetch = true; bool experimental_websocket = true; bool experimental_sqlite = true; - bool webstorage = true; + bool webstorage = HAVE_SQLITE; #ifndef OPENSSL_NO_QUIC bool experimental_quic = false; #endif diff --git a/test/common/index.js b/test/common/index.js index 5feb94563543d9..8056b7d434a8ab 100755 --- a/test/common/index.js +++ b/test/common/index.js @@ -370,7 +370,6 @@ const knownGlobals = new Set([ 'CompressionStream', 'DecompressionStream', 'Storage', - 'sessionStorage', ].forEach((i) => { if (globalThis[i] !== undefined) { knownGlobals.add(globalThis[i]); @@ -387,6 +386,9 @@ if (hasCrypto) { if (hasLocalStorage) { knownGlobals.add(globalThis.localStorage); } +if (hasSQLite) { + knownGlobals.add(globalThis.sessionStorage); +} const { Worker } = require('node:worker_threads'); knownGlobals.add(Worker); diff --git a/test/module-hooks/test-module-hooks-builtin-require.js b/test/module-hooks/test-module-hooks-builtin-require.js index 0d81f8dfb277b5..2086cbe062b020 100644 --- a/test/module-hooks/test-module-hooks-builtin-require.js +++ b/test/module-hooks/test-module-hooks-builtin-require.js @@ -12,11 +12,14 @@ const { registerHooks } = require('module'); const schemelessBlockList = new Set([ 'sea', - 'sqlite', 'test', 'test/reporters', ]); +if (common.hasSQLite) { + schemelessBlockList.add('sqlite'); +} + const testModules = []; for (const mod of schemelessBlockList) { testModules.push(`node:${mod}`); diff --git a/test/module-hooks/test-module-hooks-load-builtin-require.js b/test/module-hooks/test-module-hooks-load-builtin-require.js index 023c5431efa553..962080b3c2c874 100644 --- a/test/module-hooks/test-module-hooks-load-builtin-require.js +++ b/test/module-hooks/test-module-hooks-load-builtin-require.js @@ -36,11 +36,14 @@ hook.deregister(); // stripped for internal lookups should not get passed into the hooks. const schemelessBlockList = new Set([ 'sea', - 'sqlite', 'test', 'test/reporters', ]); +if (common.hasSQLite) { + schemelessBlockList.add('sqlite'); +} + const testModules = []; for (const mod of schemelessBlockList) { testModules.push(`node:${mod}`); diff --git a/test/parallel/test-global.js b/test/parallel/test-global.js index 8aba5c333a2ac3..257f4619830c7d 100644 --- a/test/parallel/test-global.js +++ b/test/parallel/test-global.js @@ -59,9 +59,10 @@ for (const moduleName of builtinModules) { 'fetch', 'crypto', 'navigator', - 'localStorage', - 'sessionStorage', ]; + if (common.hasSQLite) { + expected.push('localStorage', 'sessionStorage'); + } assert.deepStrictEqual(new Set(Object.keys(globalThis)), new Set(expected)); expected.forEach((value) => { const desc = Object.getOwnPropertyDescriptor(globalThis, value); diff --git a/test/parallel/test-sqlite-config.js b/test/parallel/test-sqlite-config.js index 13916f7407ed48..eb8455ff45c8df 100644 --- a/test/parallel/test-sqlite-config.js +++ b/test/parallel/test-sqlite-config.js @@ -2,8 +2,8 @@ const { skipIfSQLiteMissing } = require('../common/index.mjs'); const { test } = require('node:test'); const assert = require('node:assert'); -const { DatabaseSync } = require('node:sqlite'); skipIfSQLiteMissing(); +const { DatabaseSync } = require('node:sqlite'); function checkDefensiveMode(db) { function journalMode() { diff --git a/test/parallel/test-sqlite-template-tag.js b/test/parallel/test-sqlite-template-tag.js index 8628200cf8930f..b9917695de9acc 100644 --- a/test/parallel/test-sqlite-template-tag.js +++ b/test/parallel/test-sqlite-template-tag.js @@ -1,5 +1,7 @@ 'use strict'; -require('../common'); +const { skipIfSQLiteMissing } = require('../common'); +skipIfSQLiteMissing(); + const assert = require('assert'); const { DatabaseSync } = require('node:sqlite'); const { test, beforeEach } = require('node:test'); diff --git a/test/parallel/test-webstorage-without-sqlite.js b/test/parallel/test-webstorage-without-sqlite.js new file mode 100644 index 00000000000000..fcccc342cd469a --- /dev/null +++ b/test/parallel/test-webstorage-without-sqlite.js @@ -0,0 +1,8 @@ +'use strict'; + +const { hasSQLite } = require('../common'); +const assert = require('node:assert'); + +// Ensuring that `sessionStorage` is not a getter that throws when built without SQLite. +assert.strictEqual(typeof sessionStorage, hasSQLite ? 'object' : 'undefined'); +assert.strictEqual(Object.hasOwn(globalThis, 'sessionStorage'), hasSQLite); diff --git a/typings/internalBinding/config.d.ts b/typings/internalBinding/config.d.ts index 9fa5713fb7c89c..5651b391b88e35 100644 --- a/typings/internalBinding/config.d.ts +++ b/typings/internalBinding/config.d.ts @@ -8,6 +8,7 @@ export interface ConfigBinding { hasTracing: boolean; hasNodeOptions: boolean; hasInspector: boolean; + hasSQLite: boolean; noBrowserGlobals: boolean; bits: number; getDefaultLocale(): string;