Skip to content

Commit bfba3e6

Browse files
committed
[Tests] run every test file: exec bits, fixes; add check-exec to CI
urchin only executes test files that have the executable bit, silently skipping the rest, and `npm run test:check-exec` would catch non-executable ones, but no workflow ran it - so seven test files have never actually run. Make all of them executable, and fix the ones that were broken and verifiable locally: - `nvm install --offline` (born 644 in 59bd32b): `nvm ls | tail -1` always grabbed an alias line, never a version; use `nvm_ls` and skip its trailing `system <version>` line. - `nvm exec ... help should not parse`: in a BRE, `[options]` is a bracket expression, so the grep could never match node's literal `Usage: node [options]` output. - `nvm use ... nvmrc containing not installed version`: current output once again matches its expectations verbatim; no changes needed. The io.js source-install tests and the sudo-dependent uninstall-permissions test can only be exercised in CI; if they fail there, they fail visibly, which beats silent skipping. Rewrite `test:check-exec` in POSIX sh - npm runs scripts with /bin/sh, which is dash on Ubuntu, where the previous `[[`/`$'\n'` bashisms would break - and match by extension, so that filenames containing dots (like `.nvmrc from source`) can not dodge the check. Then wire it into the lint workflow.
1 parent 3a8b51c commit bfba3e6

9 files changed

Lines changed: 27 additions & 5 deletions

‎.github/workflows/lint.yml‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,29 @@ jobs:
7171
- name: check tests filenames
7272
run: ./rename_test.sh --check
7373

74+
check_exec:
75+
runs-on: ubuntu-latest
76+
steps:
77+
- uses: step-security/harden-runner@v2
78+
with:
79+
allowed-endpoints:
80+
github.com:443
81+
raw.githubusercontent.com:443
82+
nodejs.org:443
83+
registry.npmjs.org:443
84+
- uses: actions/checkout@v6
85+
- uses: ljharb/actions/node/install@main
86+
name: 'nvm install lts/* && npm install'
87+
with:
88+
node-version: 'lts/*'
89+
- name: check that test files are executable
90+
run: npm run test:check-exec
91+
7492
all:
7593
permissions:
7694
contents: none
7795
name: 'all linting'
78-
needs: [eclint, dockerfile_lint, doctoc, test_naming]
96+
needs: [eclint, dockerfile_lint, doctoc, test_naming, check_exec]
7997
runs-on: ubuntu-latest
8098
steps:
8199
- run: true

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"test/installation/node": "shell=$(basename -- $(ps -o comm= $(ps -o ppid= -p $PPID)) | sed 's/^-//'); make TEST_SUITE=installation_node test-$shell",
1515
"test/installation/iojs": "shell=$(basename -- $(ps -o comm= $(ps -o ppid= -p $PPID)) | sed 's/^-//'); make TEST_SUITE=installation_iojs test-$shell",
1616
"test/sourcing": "shell=$(basename -- $(ps -o comm= $(ps -o ppid= -p $PPID)) | sed 's/^-//'); make TEST_SUITE=sourcing test-$shell",
17-
"test:check-exec": "(IFS=$'\\n'; for file in $(git ls-files test); do if [ ! -x \"$file\" ] && [[ \"$file\" != *.* ]] && [[ \"$file\" != test/fixtures/* ]]; then echo \"$file\"; fi; done) | tee /dev/stderr | awk 'END {if (NR > 0) exit 1}'",
17+
"test:check-exec": "git ls-files test | while IFS= read -r file; do if [ ! -x \"$file\" ]; then case \"$file\" in test/fixtures/*|*.json|*.txt|*.sh|*.js|*.log|*.tab|*.gitkeep) ;; *) echo \"$file\" ;; esac; fi; done | tee /dev/stderr | awk 'END {if (NR > 0) exit 1}'",
1818
"test:check-nonexec": "(IFS=$'\\n'; for file in $(git ls-files test); do if [ -x \"$file\" ] && [ ! -d \"$file\" ] && { [[ \"$file\" =~ '\\.(json|txt|sh|js|log)$' ]] || [[ \"$file\" =~ '^test/(mocks|fixtures)/.*' ]]; }; then echo \"$file\"; fi; done) | tee /dev/stderr | awk 'END {if (NR > 0) exit 1}'",
1919
"doctoc": "doctoc --title='## Table of Contents' --github README.md",
2020
"predoctoc:check": "cp README.md v-README.md.orig && npm run doctoc",

‎test/fast/Unit tests/nvm install --offline‎

100644100755
Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@ nvm_download() {
1212
}
1313

1414
# --offline with an already-installed version should succeed
15-
INSTALLED_VERSION="$(nvm ls | command tail -1 | command awk '{print $1}' | command sed 's/\x1b\[[0-9;]*m//g')"
16-
if [ -n "${INSTALLED_VERSION}" ] && [ "_${INSTALLED_VERSION}" != '_N/A' ] && [ "_${INSTALLED_VERSION}" != '_system' ]; then
15+
# (`nvm ls` ends with the alias listing, so use `nvm_ls`, which emits bare
16+
# versions, and skip its trailing `system <version>` line)
17+
INSTALLED_VERSION="$(nvm_ls | command grep -v '^system' | command tail -n1)"
18+
if [ -n "${INSTALLED_VERSION}" ] && [ "_${INSTALLED_VERSION}" != '_N/A' ]; then
1719
try nvm install --offline "${INSTALLED_VERSION}"
1820
[ "_$CAPTURED_EXIT_CODE" = "_0" ] \
1921
|| die "nvm install --offline with installed version '${INSTALLED_VERSION}' should succeed, got exit code $CAPTURED_EXIT_CODE"

‎test/fast/Unit tests/nvm_offline_version‎

100644100755
File mode changed.

‎test/installation_iojs/install from source‎

100644100755
File mode changed.

‎test/installation_iojs/install version specified in .nvmrc from source‎

100644100755
File mode changed.

‎test/slow/nvm exec/Running 'nvm exec' with help should not parse‎

100644100755
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,6 @@ die () { echo "$@" ; exit 1; }
66

77
nvm use 0.10
88

9-
nvm exec stable -- node --help | grep 'Usage: node [options]' || die "Help menu should have been displayed for node and not nvm"
9+
# note: in a BRE, `[options]` is a bracket expression, so the literal `[options]`
10+
# in node's help output could never match; match the unbracketed prefix instead
11+
nvm exec stable -- node --help | grep 'Usage: node' || die "Help menu should have been displayed for node and not nvm"

‎test/slow/nvm uninstall/Running 'nvm uninstall' with incorrect file permissions fails nicely‎

100644100755
File mode changed.

‎test/slow/nvm use/Running 'nvm use' with nvmrc containing not installed version‎

100644100755
File mode changed.

0 commit comments

Comments
 (0)