Skip to content

Commit ea69b53

Browse files
Run unit tests under Windows (#106)
* Run unit tests under Windows to get some coverage on cross-platform issues * fix a few win32 incompatibilities Co-authored-by: Kræn Hansen <mail@kraenhansen.dk> * Update packages/react-native-node-api-modules/src/node/path-utils.ts Co-authored-by: Kræn Hansen <mail@kraenhansen.dk> * Update packages/react-native-node-api-modules/src/node/path-utils.ts Co-authored-by: Kræn Hansen <mail@kraenhansen.dk> --------- Co-authored-by: Kræn Hansen <mail@kraenhansen.dk>
1 parent 53173c3 commit ea69b53

File tree

5 files changed

+109
-14
lines changed

5 files changed

+109
-14
lines changed

.github/workflows/check.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@ jobs:
2929
- run: npm ci
3030
- run: npm run build
3131
- run: npm test --workspace gyp-to-cmake --workspace react-native-node-api-cmake --workspace react-native-node-api-modules
32+
test-windows:
33+
name: Run tests on Windows
34+
runs-on: windows-latest
35+
steps:
36+
- uses: actions/checkout@v4
37+
- uses: actions/setup-node@v4
38+
with:
39+
node-version: lts/jod
40+
- run: npm ci
41+
- run: npm run build
42+
- run: npm test --workspace gyp-to-cmake --workspace react-native-node-api-cmake --workspace react-native-node-api-modules
43+
3244
test-macos:
3345
name: Run tests which requires MacOS
3446
runs-on: macos-latest

package-lock.json

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/react-native-node-api-modules/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
"devDependencies": {
8585
"@babel/core": "^7.26.10",
8686
"@babel/types": "^7.27.0",
87+
"fswin": "^3.24.829",
8788
"metro-config": "0.81.1",
8889
"node-api-headers": "^1.5.0",
8990
"zod": "^3.24.3"

packages/react-native-node-api-modules/src/node/path-utils.test.ts

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import assert from "node:assert/strict";
22
import { describe, it } from "node:test";
33
import path from "node:path";
44
import fs from "node:fs";
5+
import fswin from "fswin";
56

67
import {
78
determineModuleContext,
@@ -13,6 +14,41 @@ import {
1314
} from "./path-utils.js";
1415
import { setupTempDirectory } from "./test-utils.js";
1516

17+
function removeReadPermissions(p: string) {
18+
if (process.platform !== "win32") {
19+
// Unix-like: clear all perms
20+
fs.chmodSync(p, 0);
21+
return;
22+
}
23+
24+
// Windows: simulate unreadable by setting file to offline
25+
const attributes = {
26+
IS_READ_ONLY: true,
27+
IS_OFFLINE: true,
28+
IS_TEMPORARY: true,
29+
};
30+
31+
const result = fswin.setAttributesSync(p, attributes);
32+
if (!result) throw new Error("can not set attributes to remove read permissions");
33+
}
34+
35+
function restoreReadPermissions(p: string) {
36+
if (process.platform !== "win32") {
37+
// Unix-like: clear all perms
38+
fs.chmodSync(p, 0o700);
39+
return;
40+
}
41+
42+
const attributes = {
43+
IS_READ_ONLY: false,
44+
IS_OFFLINE: false,
45+
IS_TEMPORARY: false,
46+
};
47+
48+
const result = fswin.setAttributesSync(p, attributes);
49+
if (!result) throw new Error("can not set attributes to restore read permissions");
50+
}
51+
1652
describe("isNodeApiModule", () => {
1753
it("returns true for .node", (context) => {
1854
const tempDirectoryPath = setupTempDirectory(context, {
@@ -23,19 +59,19 @@ describe("isNodeApiModule", () => {
2359
assert(isNodeApiModule(path.join(tempDirectoryPath, "addon.node")));
2460
});
2561

26-
it("returns false when directory cannot be read due to permissions", (context) => {
62+
// there is no way to set ACLs on directories in Node.js on Windows with brittle powershell commands
63+
it("returns false when directory cannot be read due to permissions", { skip: process.platform === "win32" }, (context) => {
2764
const tempDirectoryPath = setupTempDirectory(context, {
2865
"addon.android.node": "",
2966
});
30-
// remove read permissions on directory
31-
fs.chmodSync(tempDirectoryPath, 0);
67+
removeReadPermissions(tempDirectoryPath);
3268
try {
3369
assert.equal(
3470
isNodeApiModule(path.join(tempDirectoryPath, "addon")),
3571
false
3672
);
3773
} finally {
38-
fs.chmodSync(tempDirectoryPath, 0o700);
74+
restoreReadPermissions(tempDirectoryPath);
3975
}
4076
});
4177

@@ -44,15 +80,14 @@ describe("isNodeApiModule", () => {
4480
"addon.android.node": "",
4581
});
4682
const candidate = path.join(tempDirectoryPath, "addon.android.node");
47-
// remove read permission on file
48-
fs.chmodSync(candidate, 0);
83+
removeReadPermissions(candidate);
4984
try {
5085
assert.throws(
5186
() => isNodeApiModule(path.join(tempDirectoryPath, "addon")),
5287
/Found an unreadable module addon\.android\.node/
5388
);
5489
} finally {
55-
fs.chmodSync(candidate, 0o600);
90+
restoreReadPermissions(candidate);
5691
}
5792
});
5893

@@ -81,12 +116,12 @@ describe("isNodeApiModule", () => {
81116
});
82117
const unreadable = path.join(tempDirectoryPath, "addon.android.node");
83118
// only android module is unreadable
84-
fs.chmodSync(unreadable, 0);
119+
removeReadPermissions(unreadable);
85120
assert.throws(
86121
() => isNodeApiModule(path.join(tempDirectoryPath, "addon")),
87122
/Found an unreadable module addon\.android\.node/
88123
);
89-
fs.chmodSync(unreadable, 0o600);
124+
restoreReadPermissions(unreadable);
90125
});
91126
});
92127

packages/react-native-node-api-modules/src/node/path-utils.ts

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,43 @@ export function isNodeApiModule(modulePath: string): boolean {
4545
if (!entries.includes(fileName)) {
4646
return false;
4747
}
48+
49+
const filePath = path.join(dir, fileName);
50+
4851
try {
49-
fs.accessSync(path.join(dir, fileName), fs.constants.R_OK);
50-
return true;
51-
} catch (cause) {
52-
throw new Error(`Found an unreadable module ${fileName}: ${cause}`);
52+
// First, check if file exists (works the same on all platforms)
53+
fs.accessSync(filePath, fs.constants.F_OK);
54+
55+
// Then check if it's readable (behavior differs by platform)
56+
if (!isReadableSync(filePath)) {
57+
throw new Error(`Found an unreadable module ${fileName}`);
58+
}
59+
} catch (err) {
60+
throw new Error(`Found an unreadable module ${fileName}`, { cause: err });
5361
}
62+
return true;
5463
});
5564
}
5665

66+
/**
67+
* Check if a path is readable according to permission bits.
68+
* On Windows, tests store POSIX S_IWUSR bit in stats.mode.
69+
* On Unix-like, uses fs.accessSync for R_OK.
70+
*/
71+
function isReadableSync(p: string): boolean {
72+
try {
73+
if (process.platform === "win32") {
74+
const stats = fs.statSync(p);
75+
return !!(stats.mode & fs.constants.S_IWUSR);
76+
} else {
77+
fs.accessSync(p, fs.constants.R_OK);
78+
return true;
79+
}
80+
} catch {
81+
return false;
82+
}
83+
}
84+
5785
/**
5886
* Strip of any platform specific extensions from a module path.
5987
*/
@@ -107,7 +135,8 @@ export function normalizeModulePath(modulePath: string) {
107135
const dirname = path.normalize(path.dirname(modulePath));
108136
const basename = path.basename(modulePath);
109137
const strippedBasename = stripExtension(basename).replace(/^lib/, "");
110-
return path.join(dirname, strippedBasename);
138+
// Replace backslashes with forward slashes for cross-platform compatibility
139+
return path.join(dirname, strippedBasename).replaceAll("\\", "/");
111140
}
112141

113142
export function escapePath(modulePath: string) {
@@ -247,6 +276,13 @@ export function findNodeApiModulePaths(
247276
return [];
248277
}
249278
const candidatePath = path.join(fromPath, suffix);
279+
// Normalize path separators for consistent pattern matching on all platforms
280+
const normalizedSuffix = suffix.split(path.sep).join('/');
281+
282+
if (excludePatterns.some((pattern) => pattern.test(normalizedSuffix))) {
283+
return [];
284+
}
285+
250286
return fs
251287
.readdirSync(candidatePath, { withFileTypes: true })
252288
.flatMap((file) => {

0 commit comments

Comments
 (0)