Skip to content

chore(ci): try to add Node.js v20 to build matrix #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
boneskull merged 3 commits into from
Jan 9, 2026
Merged

chore(ci): try to add Node.js v20 to build matrix #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e0e24e
chore(ci): try to add Node.js v20 to build matrix
boneskull Jan 9, 2026
31a0bdc
chore(ci): add windows to build matrix
boneskull Jan 9, 2026
df96781
chore(test): skip VTE tests on win
boneskull Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions .github/workflows/nodejs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,25 @@ jobs:
test:
env:
FORCE_COLOR: '1'
name: Build & Test
runs-on: ubuntu-latest
name: Build & Test (Node ${{ matrix.node-version }}, ${{ matrix.os }})
runs-on: ${{ matrix.os }}
strategy:
matrix:
include:
- node-version: '22'
- node-version: '24'
os: [ubuntu-latest, windows-latest]
node-version: ['20', '22', '24']
exclude:
- os: windows-latest
node-version: '20'
steps:
- name: Checkout Repository
uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
- uses: ./.github/actions/prepare
with:
node-version: ${{ matrix.node-version }}
- name: Test (Node v20.x)
if: matrix.node-version == '20'
# CAUTION: this only works in POSIX environments
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "this only works in POSIX environments" but the actual issue is not POSIX-specific. The problem is that npm doesn't perform shell glob expansion on arguments passed to scripts. The test:node20 script passes an unquoted wildcard pattern which may not be expanded correctly regardless of the operating system. Consider updating the comment to accurately reflect the actual limitation, or better yet, fix the underlying issue with how the wildcard is handled.

Suggested change
# CAUTION: this only works in POSIX environments
# NOTE: npm does not expand shell globs in script arguments; ensure test:node20 handles wildcards portably

Copilot uses AI. Check for mistakes.
run: npm run test:node20
- name: Test
if: matrix.node-version != '20'
run: npm test
6 changes: 4 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@
"lint:types": "tsc --noEmit",
"prepare": "husky; run-s -s build",
"test": "run-s test:runtime",
"test:runtime": "node --import tsx --test --test-reporter=spec \"test/**/*.test.ts\"",
"test:watch": "node --import tsx --test --test-reporter=spec --watch \"test/**/*.test.ts\""
"test:base": "node --import tsx --test --test-reporter=spec",
"test:node20": "node --import tsx --test --test-reporter=spec test/*.test.ts",
"test:runtime": "npm run test:base -- \"test/*.test.ts\"",
"test:watch": "npm run test:base -- --watch \"test/*.test.ts\""
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test:watch script forwards a quoted glob pattern to test:base using --, which may result in nested or escaped quotes being passed to the Node.js command. This could cause the glob pattern to be treated as a literal string rather than being expanded. Consider removing the quotes around the pattern in the forwarded argument, or test thoroughly to ensure the pattern is correctly interpreted by Node.js across different shells and platforms.

Suggested change
"test:watch": "npm run test:base -- --watch \"test/*.test.ts\""
"test:watch": "npm run test:base -- --watch test/*.test.ts"

Copilot uses AI. Check for mistakes.
Comment on lines +64 to +65
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test:runtime script forwards a quoted glob pattern to test:base using --, which may result in nested or escaped quotes being passed to the Node.js command. This could cause the glob pattern to be treated as a literal string rather than being expanded. Consider removing the quotes around the pattern in the forwarded argument, or test thoroughly to ensure the pattern is correctly interpreted by Node.js across different shells and platforms.

Suggested change
"test:runtime": "npm run test:base -- \"test/*.test.ts\"",
"test:watch": "npm run test:base -- --watch \"test/*.test.ts\""
"test:runtime": "npm run test:base -- test/*.test.ts",
"test:watch": "npm run test:base -- --watch test/*.test.ts"

Copilot uses AI. Check for mistakes.
},
"devDependencies": {
"@commitlint/cli": "20.2.0",
Expand Down
66 changes: 39 additions & 27 deletions test/osc.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,35 +80,47 @@ describe('osc', () => {
expect(supportsHyperlinks(), 'to be', false);
});

it('should return false for VTE 0.50.0 (dotted format) due to segfault bug', () => {
delete process.env.CI;
delete process.env.FORCE_HYPERLINK;
delete process.env.TERM_PROGRAM;
process.env.VTE_VERSION = '0.50.0';

const mockStream = { isTTY: true } as NodeJS.WriteStream;
expect(supportsHyperlinks(mockStream), 'to be', false);
});

it('should return false for VTE 0.50.0 (compact format "5000") due to segfault bug', () => {
delete process.env.CI;
delete process.env.FORCE_HYPERLINK;
delete process.env.TERM_PROGRAM;
process.env.VTE_VERSION = '5000';

const mockStream = { isTTY: true } as NodeJS.WriteStream;
expect(supportsHyperlinks(mockStream), 'to be', false);
});
it(
'should return false for VTE 0.50.0 (dotted format) due to segfault bug',
{ skip: process.platform === 'win32' },
() => {
delete process.env.CI;
delete process.env.FORCE_HYPERLINK;
delete process.env.TERM_PROGRAM;
process.env.VTE_VERSION = '0.50.0';

const mockStream = { isTTY: true } as NodeJS.WriteStream;
expect(supportsHyperlinks(mockStream), 'to be', false);
},
);

it('should return true for VTE 0.50.1 (compact format "5001")', () => {
delete process.env.CI;
delete process.env.FORCE_HYPERLINK;
delete process.env.TERM_PROGRAM;
process.env.VTE_VERSION = '5001';
it(
'should return false for VTE 0.50.0 (compact format "5000") due to segfault bug',
{ skip: process.platform === 'win32' },
() => {
delete process.env.CI;
delete process.env.FORCE_HYPERLINK;
delete process.env.TERM_PROGRAM;
process.env.VTE_VERSION = '5000';

const mockStream = { isTTY: true } as NodeJS.WriteStream;
expect(supportsHyperlinks(mockStream), 'to be', false);
},
);

const mockStream = { isTTY: true } as NodeJS.WriteStream;
expect(supportsHyperlinks(mockStream), 'to be', true);
});
it(
'should return true for VTE 0.50.1 (compact format "5001")',
{ skip: process.platform === 'win32' },
() => {
delete process.env.CI;
delete process.env.FORCE_HYPERLINK;
delete process.env.TERM_PROGRAM;
process.env.VTE_VERSION = '5001';

const mockStream = { isTTY: true } as NodeJS.WriteStream;
expect(supportsHyperlinks(mockStream), 'to be', true);
},
);
});

describe('linkifyUrls()', () => {
Expand Down
Loading