Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions lib/internal/test_runner/reporter/dot.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module.exports = async function* dot(source) {
let count = 0;
let columns = getLineLength();
const failedTests = [];
const coverageErrors = [];
for await (const { type, data } of source) {
Comment on lines 12 to 14
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

The PR description is about fixing os.machine() on Windows ARM64, but this change modifies the test runner dot reporter output. If this is intentional, the PR description (or PR scope) should be updated; otherwise this looks like an unrelated change that should be moved to a separate PR.

Copilot uses AI. Check for mistakes.
if (type === 'test:pass') {
yield `${colors.green}.${colors.reset}`;
Expand All @@ -18,6 +19,10 @@ module.exports = async function* dot(source) {
yield `${colors.red}X${colors.reset}`;
ArrayPrototypePush(failedTests, data);
}
if (type === 'test:diagnostic' && data.level === 'error') {
// Collect coverage errors (coverage threshold failures)
ArrayPrototypePush(coverageErrors, data.message);
}
Comment on lines +22 to +25
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

coverageErrors collects all test:diagnostic events with data.level === 'error', but the output label hard-codes "Coverage errors". Either filter specifically for coverage-threshold diagnostics (e.g., by message pattern), or rename the variable/heading to something accurate like "Diagnostic errors".

Copilot uses AI. Check for mistakes.
if ((type === 'test:fail' || type === 'test:pass') && ++count === columns) {
yield '\n';

Expand All @@ -33,6 +38,12 @@ module.exports = async function* dot(source) {
yield formatTestReport('test:fail', test);
}
}
if (coverageErrors.length > 0) {
yield `\n${colors.red}Coverage errors:${colors.white}\n\n`;
for (const error of coverageErrors) {
yield `${colors.red}${error}${colors.white}\n`;
}
}
Comment on lines +42 to +46
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

This introduces new observable output for the dot reporter (printing diagnostic errors at the end), but there is no test/snapshot coverage for the new behavior. Please add a test that runs --test-reporter=dot with failing coverage thresholds and asserts the emitted diagnostics are included in the output.

Copilot uses AI. Check for mistakes.
};

function getLineLength() {
Expand Down
15 changes: 15 additions & 0 deletions src/node_os.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,21 @@ static void GetOSInformation(const FunctionCallbackInfo<Value>& args) {
return;
}

#ifdef _WIN32
// On Windows, uv_os_uname may return "unknown" for the machine field on ARM64.
// Try to detect the processor architecture via Windows APIs.
if (strcmp(info.machine, "unknown") == 0) {
SYSTEM_INFO sys_info;
GetSystemInfo(&sys_info);

Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

There is trailing whitespace on this blank line; please remove it to keep diffs clean and avoid lint/style checks failing on whitespace-only changes.

Suggested change

Copilot uses AI. Check for mistakes.
// Map Windows processor architecture to machine designations
// PROCESSOR_ARCHITECTURE_ARM64 = 12
if (sys_info.wProcessorArchitecture == 12) {
Comment on lines +100 to +103
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Avoid using the raw numeric value 12 for the Windows processor architecture. Prefer PROCESSOR_ARCHITECTURE_ARM64 when available, and (if necessary for older SDKs) add a small fallback #ifndef PROCESSOR_ARCHITECTURE_ARM64 definition so the intent is explicit and the code is easier to maintain.

Suggested change
// Map Windows processor architecture to machine designations
// PROCESSOR_ARCHITECTURE_ARM64 = 12
if (sys_info.wProcessorArchitecture == 12) {
// Map Windows processor architecture to machine designations.
// Prefer the SDK constant when available; fall back to the known value
// for older SDKs that may not define PROCESSOR_ARCHITECTURE_ARM64.
#ifndef PROCESSOR_ARCHITECTURE_ARM64
# define PROCESSOR_ARCHITECTURE_ARM64 12
#endif
if (sys_info.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_ARM64) {

Copilot uses AI. Check for mistakes.
snprintf(info.machine, sizeof(info.machine), "arm64");
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

This change fixes os.machine() for Windows ARM64, but there is no regression test guarding against returning "unknown" on that platform. Consider adding a conditional test that asserts os.machine() === 'arm64' when running on Windows ARM64 (e.g., common.isWindows && process.arch === 'arm64').

Copilot uses AI. Check for mistakes.
}
}
#endif

// [sysname, version, release, machine]
Local<Value> osInformation[4];
if (String::NewFromUtf8(env->isolate(), info.sysname)
Expand Down
45 changes: 45 additions & 0 deletions test/parallel/test-os-machine-arm64.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
'use strict';

// Test for Windows ARM64 os.machine() detection fix
// When on Windows ARM64, os.machine() should return 'arm64' instead of 'unknown'

const common = require('../common');
const assert = require('assert');
const os = require('os');

// os.machine() should return a string
const machine = os.machine();
assert.strictEqual(typeof machine, 'string');
assert.ok(machine.length > 0, 'os.machine() should not be empty');

// Verify it returns a recognized architecture string
// Valid values include: x64, x32, ia32, arm, arm64, ppc64, ppc64le, s390, s390x, mips, mips64el, riscv64, loong64, unknown

Check failure on line 16 in test/parallel/test-os-machine-arm64.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

This line has a length of 123. Maximum allowed is 120
const validArchitectures = [
'unknown',
'x64',
'x32',
'ia32',
'arm',
'arm64',
'ppc64',
'ppc64le',
's390',
's390x',
'mips',
'mips64el',
'riscv64',
'loong64',
];

assert.ok(

Check failure on line 34 in test/parallel/test-os-machine-arm64.js

View workflow job for this annotation

GitHub Actions / test-linux (ubuntu-24.04)

--- stderr --- node:internal/assert/utils:146 throw error; ^ AssertionError [ERR_ASSERTION]: os.machine() returned "x86_64", but should be one of: unknown, x64, x32, ia32, arm, arm64, ppc64, ppc64le, s390, s390x, mips, mips64el, riscv64, loong64 at Object.<anonymous> (/home/runner/work/node/node/node/test/parallel/test-os-machine-arm64.js:34:8) at Module._compile (node:internal/modules/cjs/loader:1829:14) at Object..js (node:internal/modules/cjs/loader:1969:10) at Module.load (node:internal/modules/cjs/loader:1552:32) at Module._load (node:internal/modules/cjs/loader:1354:12) at wrapModuleLoad (node:internal/modules/cjs/loader:255:19) at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5) at node:internal/main/run_main_module:33:47 { generatedMessage: false, code: 'ERR_ASSERTION', actual: false, expected: true, operator: '==', diff: 'simple' } Node.js v26.0.0-pre Command: out/Release/node /home/runner/work/node/node/node/test/parallel/test-os-machine-arm64.js

Check failure on line 34 in test/parallel/test-os-machine-arm64.js

View workflow job for this annotation

GitHub Actions / test-linux (ubuntu-24.04-arm)

--- stderr --- node:internal/assert/utils:146 throw error; ^ AssertionError [ERR_ASSERTION]: os.machine() returned "aarch64", but should be one of: unknown, x64, x32, ia32, arm, arm64, ppc64, ppc64le, s390, s390x, mips, mips64el, riscv64, loong64 at Object.<anonymous> (/home/runner/work/node/node/node/test/parallel/test-os-machine-arm64.js:34:8) at Module._compile (node:internal/modules/cjs/loader:1829:14) at Object..js (node:internal/modules/cjs/loader:1969:10) at Module.load (node:internal/modules/cjs/loader:1552:32) at Module._load (node:internal/modules/cjs/loader:1354:12) at wrapModuleLoad (node:internal/modules/cjs/loader:255:19) at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5) at node:internal/main/run_main_module:33:47 { generatedMessage: false, code: 'ERR_ASSERTION', actual: false, expected: true, operator: '==', diff: 'simple' } Node.js v26.0.0-pre Command: out/Release/node /home/runner/work/node/node/node/test/parallel/test-os-machine-arm64.js

Check failure on line 34 in test/parallel/test-os-machine-arm64.js

View workflow job for this annotation

GitHub Actions / x86_64-darwin: with shared libraries

--- stderr --- node:internal/assert/utils:146 throw error; ^ AssertionError [ERR_ASSERTION]: os.machine() returned "x86_64", but should be one of: unknown, x64, x32, ia32, arm, arm64, ppc64, ppc64le, s390, s390x, mips, mips64el, riscv64, loong64 at Object.<anonymous> (/Users/runner/work/_temp/node-v26.0.0-nightly2026-03-12a73b7f8ae5-slim/test/parallel/test-os-machine-arm64.js:34:8) at Module._compile (node:internal/modules/cjs/loader:1829:14) at Object..js (node:internal/modules/cjs/loader:1969:10) at Module.load (node:internal/modules/cjs/loader:1552:32) at Module._load (node:internal/modules/cjs/loader:1354:12) at wrapModuleLoad (node:internal/modules/cjs/loader:255:19) at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5) at node:internal/main/run_main_module:33:47 { generatedMessage: false, code: 'ERR_ASSERTION', actual: false, expected: true, operator: '==', diff: 'simple' } Node.js v26.0.0-pre Command: out/Release/node /Users/runner/work/_temp/node-v26.0.0-nightly2026-03-12a73b7f8ae5-slim/test/parallel/test-os-machine-arm64.js

Check failure on line 34 in test/parallel/test-os-machine-arm64.js

View workflow job for this annotation

GitHub Actions / aarch64-linux: with shared libraries

--- stderr --- node:internal/assert/utils:146 throw error; ^ AssertionError [ERR_ASSERTION]: os.machine() returned "aarch64", but should be one of: unknown, x64, x32, ia32, arm, arm64, ppc64, ppc64le, s390, s390x, mips, mips64el, riscv64, loong64 at Object.<anonymous> (/home/runner/work/_temp/node-v26.0.0-nightly2026-03-12a73b7f8ae5-slim/test/parallel/test-os-machine-arm64.js:34:8) at Module._compile (node:internal/modules/cjs/loader:1829:14) at Object..js (node:internal/modules/cjs/loader:1969:10) at Module.load (node:internal/modules/cjs/loader:1552:32) at Module._load (node:internal/modules/cjs/loader:1354:12) at wrapModuleLoad (node:internal/modules/cjs/loader:255:19) at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5) at node:internal/main/run_main_module:33:47 { generatedMessage: false, code: 'ERR_ASSERTION', actual: false, expected: true, operator: '==', diff: 'simple' } Node.js v26.0.0-pre Command: out/Release/node /home/runner/work/_temp/node-v26.0.0-nightly2026-03-12a73b7f8ae5-slim/test/parallel/test-os-machine-arm64.js

Check failure on line 34 in test/parallel/test-os-machine-arm64.js

View workflow job for this annotation

GitHub Actions / x86_64-linux: with shared libraries

--- stderr --- node:internal/assert/utils:146 throw error; ^ AssertionError [ERR_ASSERTION]: os.machine() returned "x86_64", but should be one of: unknown, x64, x32, ia32, arm, arm64, ppc64, ppc64le, s390, s390x, mips, mips64el, riscv64, loong64 at Object.<anonymous> (/home/runner/work/_temp/node-v26.0.0-nightly2026-03-12a73b7f8ae5-slim/test/parallel/test-os-machine-arm64.js:34:8) at Module._compile (node:internal/modules/cjs/loader:1829:14) at Object..js (node:internal/modules/cjs/loader:1969:10) at Module.load (node:internal/modules/cjs/loader:1552:32) at Module._load (node:internal/modules/cjs/loader:1354:12) at wrapModuleLoad (node:internal/modules/cjs/loader:255:19) at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5) at node:internal/main/run_main_module:33:47 { generatedMessage: false, code: 'ERR_ASSERTION', actual: false, expected: true, operator: '==', diff: 'simple' } Node.js v26.0.0-pre Command: out/Release/node /home/runner/work/_temp/node-v26.0.0-nightly2026-03-12a73b7f8ae5-slim/test/parallel/test-os-machine-arm64.js
validArchitectures.includes(machine),
`os.machine() returned "${machine}", but should be one of: ${validArchitectures.join(', ')}`
);

// On Windows systems, specifically verify that os.machine() doesn't return 'unknown'
// if the system is actually ARM64 capable (this fix ensures GetSystemInfo() is used)
if (common.isWindows) {
// This test will pass on all Windows systems. The fix ensures that on ARM64 systems,
// os.machine() no longer returns 'unknown', but instead returns 'arm64'
assert.ok(machine.length > 0);
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this test matches the intent in the comments

Loading