Skip to content

Commit

Permalink
Merge pull request #589 from jan-molak/fix/node-20.12.2-windows
Browse files Browse the repository at this point in the history
fix(node): support for Node 20.12.2 on Windows
  • Loading branch information
jan-molak authored Apr 18, 2024
2 parents 8cb971b + 7cf4c26 commit 83fc05b
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 36 deletions.
1 change: 0 additions & 1 deletion .c8rc.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
".ts"
],
"reporter": [
"lcov",
"text-summary"
]
}
47 changes: 28 additions & 19 deletions .github/workflows/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@ permissions:
contents: read

jobs:
test-linux:
test:
if: "!contains(github.event.head_commit.message, 'ci skip')"
runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
node-version: [ 16.x, 18.x, 20.x ]
os: [ ubuntu-latest, windows-latest ]

runs-on: ${{ matrix.os }}

steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4
Expand All @@ -34,32 +37,39 @@ jobs:
- run: npm test
env:
CI: true
- name: Coveralls
uses: coverallsapp/github-action@3dfc5567390f6fa9267c0ee9c251e4c8c3f18949 # v2.2.3
if: ${{ matrix.node-version == '18.x' }}
- uses: actions/upload-artifact@v4
with:
github-token: ${{ github.token }}
path-to-lcov: 'coverage/lcov.info'

test-windows:
if: "!contains(github.event.head_commit.message, 'ci skip')"
runs-on: windows-latest
name: ${{ matrix.os }}-${{ matrix.node-version }}-coverage
if-no-files-found: warn
path: coverage
retention-days: 1

coverage:
needs:
- test
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4
- name: Setup Node.js
- name: Setup Node
uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4
with:
node-version: 18.x
- name: Install Node Modules
uses: bahmutov/npm-install@v1
with:
install-command: npm ci
- run: npm run clean
- run: npm run lint
- run: npm test
env:
CI: true
- name: Aggregate coverage reports
uses: actions/download-artifact@v4
with:
path: coverage
merge-multiple: true
- name: Merge coverage reports
run: npx c8 report --merge-async --reporter=lcov --reporter=text-summary
- name: Coveralls
uses: coverallsapp/github-action@3dfc5567390f6fa9267c0ee9c251e4c8c3f18949 # v2.2.3
with:
github-token: ${{ github.token }}
path-to-lcov: 'coverage/lcov.info'

build:
runs-on: ubuntu-latest
Expand All @@ -84,8 +94,7 @@ jobs:
release:
needs:
- build
- test-linux
- test-windows
- test
runs-on: ubuntu-latest
if: github.ref == 'refs/heads/main'
permissions:
Expand Down
6 changes: 3 additions & 3 deletions spec/Failsafe.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ describe(`Failsafe`, function() {
`[print-args] " foo "`,
`[print-args] " bar "`,
`[failsafe] Script 'print-args' exited with code 0`,
].join('\n'));
].join('\n'), logger.stdout());
});

it(`passes all arguments after separator`, async () => {
Expand All @@ -365,10 +365,10 @@ describe(`Failsafe`, function() {
`[print-args] "foo bar"`,
`[print-args] "baz"`,
`[failsafe] Script 'print-args' exited with code 0`,
].join('\n'));
].join('\n'), logger.stdout());
});

it(`passes different arguments to specifc scripts`, async () => {
it(`passes different arguments to specific scripts`, async () => {
const { run, logger } = failsafe();

const exitCode = await run([`print-args`, '[--foo]', 'also-print-args', '[--bar]', '--foo=bar', '--bar=foo']);
Expand Down
59 changes: 46 additions & 13 deletions src/Failsafe.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { spawn } from 'child_process';
import { ChildProcessWithoutNullStreams, spawn } from 'child_process';
import * as fs from 'fs';
import readline = require('readline');
import path = require('path');

import { ArgumentParser, ParseError, Script, UnrecognisedArgumentsError } from './ArgumentParser';
import { Logger, trimmed } from './logger';
import path = require('path');

const packageJson = JSON.parse(fs.readFileSync(path.join(__dirname, '..', 'package.json'), 'utf8'))

Expand Down Expand Up @@ -112,7 +112,6 @@ export class Failsafe {
|`);
}
else {
// istanbul ignore next
this.logger.error('failsafe', `Error: ${ error.message }`);
}

Expand Down Expand Up @@ -142,21 +141,16 @@ export class Failsafe {

private runScript(scriptName: string, arguments_: string[] = []): Promise<ExitCode> {
return new Promise((resolve, reject) => {
const npm = process.platform.startsWith('win32')
? `npm.cmd`
: (process.env.npm_execpath ?? `npm`);
const isWindows = process.platform.startsWith('win32');

const npmArguments = [`run`, scriptName];
if (arguments_.length > 0) {
npmArguments.push('--', ...arguments_);
}
const script = spawn(npm, npmArguments, {
cwd: this.config.cwd,
env: {
'FORCE_COLOR': this.config.isTTY ? '1' : undefined,
...this.env
}
});

const script = isWindows
? this.windowsSpawn(npmArguments)
: this.linuxAndMacSpawn(npmArguments);

const
stdout = readline.createInterface({ input: script.stdout }),
Expand All @@ -175,4 +169,43 @@ export class Failsafe {
});
});
}

private linuxAndMacSpawn(npmArguments: string[]): ChildProcessWithoutNullStreams {
const npm = process.env.npm_execpath ?? `npm`;

return spawn(npm, npmArguments, {
cwd: this.config.cwd,
env: {
...this.forceColorEnv(),
...this.env
},
});
}

private windowsSpawn(npmArguments: string[]): ChildProcessWithoutNullStreams {
/* c8 ignore start */
return spawn(`npm.cmd`, npmArguments.map(npmArgument => this.quoteArgumentsWithSpaces(npmArgument)), {
cwd: this.config.cwd,
env: {
...this.forceColorEnv(),
...this.env
},
shell: true,
});
/* c8 ignore stop */
}

private quoteArgumentsWithSpaces(npmArgument: string): string {
/* c8 ignore start */
return npmArgument.includes(' ')
? `"${ npmArgument }"`
: npmArgument;
/* c8 ignore stop */
}

private forceColorEnv() {
return {
'FORCE_COLOR': this.config.isTTY ? '1' : undefined,
}
}
}

0 comments on commit 83fc05b

Please sign in to comment.