-
-
Notifications
You must be signed in to change notification settings - Fork 35.1k
fix(os): return 'arm64' from os.machine() on Windows ARM64 (fix #62232) #62235
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) { | ||
| if (type === 'test:pass') { | ||
| yield `${colors.green}.${colors.reset}`; | ||
|
|
@@ -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
|
||
| if ((type === 'test:fail' || type === 'test:pass') && ++count === columns) { | ||
| yield '\n'; | ||
|
|
||
|
|
@@ -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
|
||
| }; | ||
|
|
||
| function getLineLength() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Copilot
AI
Mar 12, 2026
There was a problem hiding this comment.
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.
| // 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
AI
Mar 12, 2026
There was a problem hiding this comment.
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').
| 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 | ||
| 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
|
||
| 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); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this test matches the intent in the comments |
||
There was a problem hiding this comment.
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.