Skip to content

Commit 174b087

Browse files
fix: run npm through a shell on Windows, and assert timing-free event shapes
The packaging proof spawned `npm`, which on Windows is npm.cmd -- a file Node declines to spawn without a shell -- so the proof never ran there. Arguments are quoted now that they travel as one command line. Two shared engine assertions pinned a race instead of a contract. A change made before unwatch() may still be draining when it returns, so the baseline is now taken once the stream settles; and how many corrections close a debounce window depends on when the backend delivered the second write, so the create-then- correct test asserts the shape it means -- a leading created, corrections after it -- while the bound on coalescing stays with the burst test that owns it.
1 parent a7c9396 commit 174b087

2 files changed

Lines changed: 40 additions & 8 deletions

File tree

src/bindings/nodejs/scripts/verify-package.js

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,25 @@ function check(name, fn) {
4040
const NPM_TIMEOUT_MS = 180_000;
4141
const NODE_TIMEOUT_MS = 30_000;
4242

43+
/**
44+
* npm is `npm.cmd` on Windows, and Node refuses to spawn a `.cmd` without a shell, so there the
45+
* command has to go through one. Its arguments then travel as a single command line rather than as
46+
* a vector, which is why they are quoted: every path passed below sits under the temp directory,
47+
* and on a machine whose user name contains a space one argument would otherwise arrive as two.
48+
*/
4349
function npm(args, cwd) {
44-
const result = spawnSync('npm', args, {
45-
cwd,
46-
encoding: 'utf8',
47-
timeout: NPM_TIMEOUT_MS,
48-
env: { ...process.env, npm_config_audit: 'false', npm_config_fund: 'false' },
49-
});
50+
const windows = process.platform === 'win32';
51+
const result = spawnSync(
52+
windows ? 'npm.cmd' : 'npm',
53+
windows ? args.map((arg) => (/\s/.test(arg) ? `"${arg}"` : arg)) : args,
54+
{
55+
cwd,
56+
encoding: 'utf8',
57+
timeout: NPM_TIMEOUT_MS,
58+
shell: windows,
59+
env: { ...process.env, npm_config_audit: 'false', npm_config_fund: 'false' },
60+
}
61+
);
5062
if (result.error) throw new Error(`npm ${args.join(' ')}: ${result.error.message}`);
5163
if (result.status !== 0) {
5264
throw new Error(`npm ${args.join(' ')} failed (${result.status})\n${result.stderr}`);

src/bindings/nodejs/test/shared/engine-suite.mjs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,15 @@ export function runEngineSuite(engineName, makeRetrigger) {
216216
await waitFor(() => kindsFor(events, target).length > 0, { message: 'first write missed' });
217217

218218
watcher.unwatch(dir);
219+
// Settled before the baseline is taken. A correction owed for the write above, or a
220+
// notification the kernel had already queued, both belong to the watched period and arrive
221+
// whenever the platform gets to them; counting them as breaches would make this assert how
222+
// promptly a backend drains, not whether the unwatch took.
223+
await waitForQuiet(() => events.length);
219224
const before = events.length;
220225
writeFile(target, 'two-longer');
221226
await waitForQuiet(() => events.length);
222-
expect(events.length).toBe(before);
227+
expect(events.length, 'a change made after unwatch() must not be reported').toBe(before);
223228
});
224229

225230
// -------------------------------------------------------------- debouncing
@@ -276,7 +281,22 @@ export function runEngineSuite(engineName, makeRetrigger) {
276281
message: 'the write absorbed by the window never arrived as a correction',
277282
});
278283
await waitForQuiet(() => events.length, { quietMs: 250 });
279-
expect(kindsFor(events, target)).toEqual(['created', 'modified']);
284+
285+
// Asserted as a shape rather than an exact list. How many corrections close the window is a
286+
// property of when the backend delivered the second write -- a slow machine can report it
287+
// late enough to open a window of its own -- and the bound on coalescing is the burst test's
288+
// subject, just above. What belongs to this test is that the leading event kept its own
289+
// identity and that the absorbed write was not swallowed.
290+
const kinds = kindsFor(events, target);
291+
expect(kinds[0], 'a new file is announced as created, not as a modification').toBe('created');
292+
expect(
293+
kinds.length,
294+
'the write the window absorbed must still reach the consumer'
295+
).toBeGreaterThanOrEqual(2);
296+
expect(
297+
kinds.slice(1).every((kind) => kind === 'modified'),
298+
`everything after the leading event is a correction, got ${kinds}`
299+
).toBe(true);
280300
});
281301

282302
// -------------------------------------------------------- content changes

0 commit comments

Comments
 (0)