From 8c14f61f24208a4a18ece198bdcade399fd976f8 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Tue, 13 Oct 2020 14:21:33 -0500 Subject: [PATCH 1/6] test: improve error message for policy failures --- test/common/index.js | 17 +++++++++++++++++ test/parallel/test-policy-dependencies.js | 1 + .../test-policy-dependency-conditions.js | 1 + test/parallel/test-policy-integrity-flag.js | 1 + test/parallel/test-policy-parse-integrity.js | 1 + test/parallel/test-policy-scopes-integrity.js | 1 + test/parallel/test-policy-scopes.js | 1 + test/pummel/test-policy-integrity.js | 1 + 8 files changed, 24 insertions(+) diff --git a/test/common/index.js b/test/common/index.js index b75c5441d2e3d5..736a2a0fc76372 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -697,6 +697,22 @@ function gcUntil(name, condition) { }); } +function requireNoPackageJSONAbove() { + let possiblePackage = path.join(__dirname, '..', 'package.json'); + let lastPackage = null; + while (possiblePackage !== lastPackage) { + if (fs.existsSync(possiblePackage)) { + throw new Error( + 'This test shouldn\'t load properties from a package.json above ' + + `it's file location. Found package.json at ${possiblePackage}, ` + + 'in order to run this test properly ensure no package.json is above ' + + 'the tests.'); + } + lastPackage = possiblePackage; + possiblePackage = path.join(possiblePackage, '..', '..', 'package.json'); + } +} + const common = { allowGlobals, buildType, @@ -736,6 +752,7 @@ const common = { platformTimeout, printSkipMessage, pwdCommand, + requireNoPackageJSONAbove, runWithInvalidFD, skip, skipIf32Bits, diff --git a/test/parallel/test-policy-dependencies.js b/test/parallel/test-policy-dependencies.js index e7b2289714dff4..4486e0f8aa08c0 100644 --- a/test/parallel/test-policy-dependencies.js +++ b/test/parallel/test-policy-dependencies.js @@ -3,6 +3,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +common.requireNoPackageJSONAbove(); const fixtures = require('../common/fixtures'); diff --git a/test/parallel/test-policy-dependency-conditions.js b/test/parallel/test-policy-dependency-conditions.js index c9db95929daeec..dec17aa22984b0 100644 --- a/test/parallel/test-policy-dependency-conditions.js +++ b/test/parallel/test-policy-dependency-conditions.js @@ -4,6 +4,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +common.requireNoPackageJSONAbove(); const Manifest = require('internal/policy/manifest').Manifest; diff --git a/test/parallel/test-policy-integrity-flag.js b/test/parallel/test-policy-integrity-flag.js index d97ef86cbe9d0f..ddcd02236d27c0 100644 --- a/test/parallel/test-policy-integrity-flag.js +++ b/test/parallel/test-policy-integrity-flag.js @@ -3,6 +3,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +common.requireNoPackageJSONAbove(); const fixtures = require('../common/fixtures'); diff --git a/test/parallel/test-policy-parse-integrity.js b/test/parallel/test-policy-parse-integrity.js index 9241f1e7843b64..cbf08c4d2ccc58 100644 --- a/test/parallel/test-policy-parse-integrity.js +++ b/test/parallel/test-policy-parse-integrity.js @@ -2,6 +2,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +common.requireNoPackageJSONAbove(); const tmpdir = require('../common/tmpdir'); const assert = require('assert'); diff --git a/test/parallel/test-policy-scopes-integrity.js b/test/parallel/test-policy-scopes-integrity.js index b506c92c41f625..923657d5daab3e 100644 --- a/test/parallel/test-policy-scopes-integrity.js +++ b/test/parallel/test-policy-scopes-integrity.js @@ -4,6 +4,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +common.requireNoPackageJSONAbove(); const Manifest = require('internal/policy/manifest').Manifest; const assert = require('assert'); diff --git a/test/parallel/test-policy-scopes.js b/test/parallel/test-policy-scopes.js index b1ba160bdde3f1..129287c73c7659 100644 --- a/test/parallel/test-policy-scopes.js +++ b/test/parallel/test-policy-scopes.js @@ -3,6 +3,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +common.requireNoPackageJSONAbove(); const fixtures = require('../common/fixtures'); diff --git a/test/pummel/test-policy-integrity.js b/test/pummel/test-policy-integrity.js index 57625268fae4d0..15124aefd46904 100644 --- a/test/pummel/test-policy-integrity.js +++ b/test/pummel/test-policy-integrity.js @@ -2,6 +2,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +common.requireNoPackageJSONAbove(); const { debuglog } = require('util'); const debug = debuglog('test'); From 50a08410af478b52c35b1ce525740a4cd57fae6c Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Tue, 13 Oct 2020 15:57:59 -0500 Subject: [PATCH 2/6] missed one --- test/parallel/test-policy-scopes-dependencies.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-policy-scopes-dependencies.js b/test/parallel/test-policy-scopes-dependencies.js index 202e6459a6543e..a5a9302ac69629 100644 --- a/test/parallel/test-policy-scopes-dependencies.js +++ b/test/parallel/test-policy-scopes-dependencies.js @@ -4,6 +4,7 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +common.requireNoPackageJSONAbove(); const Manifest = require('internal/policy/manifest').Manifest; const assert = require('assert'); From 9203dd0f02e7528dcda3c1ddbd59403cad87ff97 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Thu, 15 Oct 2020 08:06:51 -0500 Subject: [PATCH 3/6] Update test/common/index.js Co-authored-by: Rich Trott --- test/common/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/common/index.js b/test/common/index.js index 736a2a0fc76372..7999b593dccc09 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -704,9 +704,7 @@ function requireNoPackageJSONAbove() { if (fs.existsSync(possiblePackage)) { throw new Error( 'This test shouldn\'t load properties from a package.json above ' + - `it's file location. Found package.json at ${possiblePackage}, ` + - 'in order to run this test properly ensure no package.json is above ' + - 'the tests.'); + `its file location. Found package.json at ${possiblePackage}.`); } lastPackage = possiblePackage; possiblePackage = path.join(possiblePackage, '..', '..', 'package.json'); From d313252cacfc8a5a6b18d780ab1d49887056a804 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Thu, 15 Oct 2020 08:06:58 -0500 Subject: [PATCH 4/6] Update test/common/index.js Co-authored-by: Rich Trott --- test/common/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/index.js b/test/common/index.js index 7999b593dccc09..4ec68cf5857a9f 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -702,7 +702,7 @@ function requireNoPackageJSONAbove() { let lastPackage = null; while (possiblePackage !== lastPackage) { if (fs.existsSync(possiblePackage)) { - throw new Error( + assert.fail( 'This test shouldn\'t load properties from a package.json above ' + `its file location. Found package.json at ${possiblePackage}.`); } From 62c32dc3e422c7f6d22761d3dba6176efb3e115c Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Thu, 15 Oct 2020 13:55:23 -0500 Subject: [PATCH 5/6] Add readme Co-authored-by: Rich Trott --- lib/internal/url.js | 7 ++++--- test/common/README.md | 5 +++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index 3c464ffbd638f6..7f2277c474e235 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -1295,7 +1295,7 @@ function urlToOptions(url) { return options; } -const forwardSlashRegEx = /\//g; +const forwardSlashesRegEx = /\/+/g; function getPathFromURLWin32(url) { const hostname = url.hostname; @@ -1311,7 +1311,7 @@ function getPathFromURLWin32(url) { } } } - pathname = pathname.replace(forwardSlashRegEx, '\\'); + pathname = pathname.replace(forwardSlashesRegEx, '\\'); pathname = decodeURIComponent(pathname); if (hostname !== '') { // If hostname is set, then we have a UNC path @@ -1336,7 +1336,7 @@ function getPathFromURLPosix(url) { if (url.hostname !== '') { throw new ERR_INVALID_FILE_URL_HOST(platform); } - const pathname = url.pathname; + let pathname = url.pathname; for (let n = 0; n < pathname.length; n++) { if (pathname[n] === '%') { const third = pathname.codePointAt(n + 2) | 0x20; @@ -1347,6 +1347,7 @@ function getPathFromURLPosix(url) { } } } + pathname = pathname.replace(forwardSlashesRegEx, '/'); return decodeURIComponent(pathname); } diff --git a/test/common/README.md b/test/common/README.md index 31d37d7abd2b4a..5ce7473cb519fc 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -378,6 +378,11 @@ const { spawn } = require('child_process'); spawn(...common.pwdCommand, { stdio: ['pipe'] }); ``` +### `requireNoPackageJSONAbove()` + +Throws an `AssertionError` if a `package.json` file is in any ancestor +directory. Such files may interfere with proper test functionality. + ### `runWithInvalidFD(func)` * `func` [<Function>][] From e6be5c7f15089765f011c5f327e2bc4a82ffd0d9 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 18 Oct 2020 06:13:35 -0700 Subject: [PATCH 6/6] fixup! Add readme --- lib/internal/url.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/internal/url.js b/lib/internal/url.js index 7f2277c474e235..3c464ffbd638f6 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -1295,7 +1295,7 @@ function urlToOptions(url) { return options; } -const forwardSlashesRegEx = /\/+/g; +const forwardSlashRegEx = /\//g; function getPathFromURLWin32(url) { const hostname = url.hostname; @@ -1311,7 +1311,7 @@ function getPathFromURLWin32(url) { } } } - pathname = pathname.replace(forwardSlashesRegEx, '\\'); + pathname = pathname.replace(forwardSlashRegEx, '\\'); pathname = decodeURIComponent(pathname); if (hostname !== '') { // If hostname is set, then we have a UNC path @@ -1336,7 +1336,7 @@ function getPathFromURLPosix(url) { if (url.hostname !== '') { throw new ERR_INVALID_FILE_URL_HOST(platform); } - let pathname = url.pathname; + const pathname = url.pathname; for (let n = 0; n < pathname.length; n++) { if (pathname[n] === '%') { const third = pathname.codePointAt(n + 2) | 0x20; @@ -1347,7 +1347,6 @@ function getPathFromURLPosix(url) { } } } - pathname = pathname.replace(forwardSlashesRegEx, '/'); return decodeURIComponent(pathname); }