Skip to content

Commit 4fdf480

Browse files
os-zhuangclaude
andauthored
test(driver-mongodb): gate the mongod-backed suites behind an opt-in env var (#5517) (#5538)
The seven suites that need a real MongoDB now skip by default and print why. `mongodb-memory-server` downloads a ~123 MB binary on first use, and on a cold cache two vitest workers raced that download: the winner renamed `<archive>.tgz.downloading`, the loser's `rename` threw ENOENT inside an unawaited `async` listener (`MongoBinaryDownload.js:413`), so the enclosing promise never settled AND the ENOENT surfaced as a process-level unhandled rejection. That is one event, not the two the issue describes: the never-settling promise is the "timed out after 120s" skip, and the unhandled rejection is what turned all-green runs into `exit 1` and ejected unrelated PRs from the merge queue. Per the maintainer's decision, the download is retired rather than made single-flight (#5499 freezes investment in this family): no globalSetup pre-download, no cross-worker lock, no workflow cache warming. - `OS_TEST_MONGODB_MEMORY_SERVER_ENABLED=1` opts in (`OS_TEST_*` + `_ENABLED`, the `OS_TEST_MULTI_ORG_ENABLED` shape, Prime Directive #9). - The gate is checked BEFORE `mongodb-memory-server` is imported at all, so a default run starts zero downloads; the new gate suite pins that by counting library evaluations. - Notices go to `process.stderr`, not `console.warn`: measured on vitest 4.1.10, the default reporter renders nothing a passing/skipped file logs through `console`, which would have made the retirement a silent skip. - `installAbandonedDownloadGuard` swallows the abandoned-download ENOENT — and only it — on the opt-in path, re-raising everything else because vitest steps aside once another `unhandledRejection` listener exists. - `mongodb-findone-query` / `mongodb-pagination-conformance` called `MongoMemoryServer.create()` directly (bypassing the gate and the deadline); both now go through `createTestMongod`. - Coverage consequence recorded in `scripts/check-driver-conformance.mjs`: the mongo cells stay CONSUMED on their imports, but the temporal and pagination case-sets now run only opt-in. Claude-Session: https://claude.ai/code/session_01BWS4heBoAitLmzCLhcYdbK Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 5acb93a commit 4fdf480

8 files changed

Lines changed: 571 additions & 81 deletions

packages/plugins/driver-mongodb/README.md

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,13 +199,32 @@ kernel.use(mongodbPlugin, {
199199
## Development
200200

201201
```bash
202-
# Run tests
202+
# Run tests (the suites that need a real mongod SKIP — see below)
203203
pnpm test
204204

205+
# Run every suite, including the ones that need a real mongod
206+
OS_TEST_MONGODB_MEMORY_SERVER_ENABLED=1 pnpm test
207+
205208
# Build
206209
pnpm build
207210
```
208211

212+
### The mongod-backed suites are opt-in (#5517)
213+
214+
Seven suites here need a real MongoDB, which `mongodb-memory-server` provides by
215+
downloading a ~123 MB binary on first use. With a cold cache, two vitest workers
216+
downloaded it at the same time and the loser's `rename` failed as an unhandled
217+
rejection — turning an all-green run into `exit 1` and ejecting unrelated PRs
218+
from the merge queue. Those suites are therefore gated behind
219+
`OS_TEST_MONGODB_MEMORY_SERVER_ENABLED=1`: without it they skip, each printing
220+
one line that names this issue and the switch, and **no download starts**.
221+
222+
The rest of the package's tests — filter translation, the shared filter-logic
223+
conformance case-set over the emitted documents, sort specs, tenancy guard,
224+
temporal helpers — run by default and need no binary. The gate lives in
225+
`src/test-mongod.ts`, which documents the mechanism and what a default run gives
226+
up.
227+
209228
## License
210229

211230
Apache-2.0. See [LICENSING.md](../../../LICENSING.md).

packages/plugins/driver-mongodb/src/mongodb-driver.test.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,14 @@ import { MongoDBDriver } from './mongodb-driver.js';
77
import { createTestMongod } from './test-mongod.js';
88

99
// `mongodb-memory-server` downloads a real MongoDB binary from
10-
// fastdl.mongodb.org on first use. In sandboxed / offline CI that download can
11-
// fail — or HANG, which this suite could not express until `createTestMongod`
12-
// put a deadline on the wait (see that module); both now land on the same skip.
13-
// When it does we **skip** this suite rather than failing the whole package's
14-
// test run (and, with it, the monorepo `Test Core` job). The startup is
15-
// attempted once here so availability is known at collection time — a throwing
16-
// `beforeAll` would *fail* every test instead of skipping it.
10+
// fastdl.mongodb.org on first use, which is why this suite is OPT-IN since
11+
// #5517: two workers downloading it at once made an all-green run `exit 1` and
12+
// ejected unrelated PRs from the merge queue. `createTestMongod` skips this
13+
// suite — printing why — unless `OS_TEST_MONGODB_MEMORY_SERVER_ENABLED=1`, and
14+
// an opted-in download that fails or HANGS lands on the same skip rather than
15+
// stalling the job. The acquisition happens once here so availability is known
16+
// at collection time — a throwing `beforeAll` would *fail* every test instead
17+
// of skipping it.
1718
const sharedMongod: MongoMemoryServer | undefined = await createTestMongod('MongoDBDriver');
1819

1920
describe.skipIf(!sharedMongod)('MongoDBDriver', () => {

packages/plugins/driver-mongodb/src/mongodb-filter-logic-conformance.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@
1616
*
1717
* The same table is driven server-free by
1818
* `mongodb-filter-logic-translation.test.ts`, which is the half that always
19-
* runs. This one skips when the mongod binary cannot be fetched (the
20-
* `createTestMongod` convention every suite in this package uses — a blocked or
21-
* hanging download costs a skipped suite, not a stalled test job). **A skip is
22-
* not a pass**: on a machine without the binary, the translation suite is the
23-
* whole proof, which is exactly why it carries the priority half.
19+
* runs. This one is OPT-IN since #5517: it needs the mongod binary, whose
20+
* concurrent download turned green runs into `exit 1`, so `createTestMongod`
21+
* skips it unless `OS_TEST_MONGODB_MEMORY_SERVER_ENABLED=1` (and still skips,
22+
* rather than stalling, when an opted-in download is blocked or hanging).
23+
* **A skip is not a pass**: by default the translation suite is the whole proof,
24+
* which is exactly why it carries the priority half.
2425
*/
2526

2627
import { describe, it, expect, beforeAll, afterAll } from 'vitest';

packages/plugins/driver-mongodb/src/mongodb-findone-query.test.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,22 @@
2424
* because an untouched collection comes back in insertion order whether or not
2525
* a sort went out.
2626
*
27-
* Skips itself when the mongod binary cannot be fetched — the convention the
28-
* other suites in this package already use. A skip is not a pass.
27+
* Needs a real mongod, so it is OPT-IN since #5517 — `createTestMongod` prints
28+
* why and this suite skips unless `OS_TEST_MONGODB_MEMORY_SERVER_ENABLED=1`. It
29+
* used to call `MongoMemoryServer.create()` itself, which both started a
30+
* download in every default run and bypassed the package's shared deadline. A
31+
* skip is not a pass.
2932
*/
3033

3134
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
32-
import { MongoMemoryServer } from 'mongodb-memory-server';
35+
import type { MongoMemoryServer } from 'mongodb-memory-server';
3336
import { MongoDBDriver } from './mongodb-driver.js';
3437
import { FINDONE_CASES, FINDONE_ROWS } from './mongodb-findone-cases.js';
38+
import { createTestMongod } from './test-mongod.js';
3539

36-
let sharedMongod: MongoMemoryServer | undefined;
37-
try {
38-
sharedMongod = await MongoMemoryServer.create({ instance: { launchTimeout: 60_000 } });
39-
} catch (err) {
40-
console.warn(
41-
'[driver-mongodb] Skipping findOne query-execution suite — mongodb-memory-server could not '
42-
+ `start: ${(err as Error)?.message ?? String(err)}`,
43-
);
44-
}
40+
const sharedMongod: MongoMemoryServer | undefined = await createTestMongod(
41+
'findOne query-execution',
42+
);
4543

4644
describe.skipIf(!sharedMongod)('driver-mongodb — findOne executes the whole query', () => {
4745
const mongod = sharedMongod as MongoMemoryServer;
Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#5517] The binary-dependent suites are OPT-IN, and the gate is checked before
5+
* anything can start a download.
6+
*
7+
* This is the suite that survives the retirement, so it has to pin the two
8+
* properties the retirement is worth anything for:
9+
*
10+
* 1. **Nothing is even imported when the gate is closed.** A `describe.skipIf`
11+
* alone would not have been enough: the download starts at MODULE LOAD, so
12+
* a gate evaluated after `import { MongoMemoryServer } from …` would skip
13+
* the tests and still fetch 123 MB. The mock factory below counts library
14+
* evaluations, so re-adding a static value import to `test-mongod.ts` turns
15+
* the first test red.
16+
* 2. **The abandoned-download rejection is swallowed, and only it.** The
17+
* guard's listener suppresses vitest's own unhandled-rejection reporting for
18+
* the whole worker (vitest steps aside when a second listener exists), so a
19+
* guard that swallowed indiscriminately would hide real failures. The
20+
* re-raise path is asserted, not assumed.
21+
*
22+
* Test ORDER matters in the first three cases: case 1 asserts the library has
23+
* never been evaluated, and case 3 is what evaluates it.
24+
*/
25+
26+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
27+
28+
/** Counts evaluations of `mongodb-memory-server` and lets a case choose what `create()` does. */
29+
const lib = vi.hoisted(() => ({
30+
imported: 0,
31+
create: undefined as undefined | (() => Promise<unknown>),
32+
}));
33+
34+
vi.mock('mongodb-memory-server', () => {
35+
lib.imported++;
36+
return {
37+
MongoMemoryServer: {
38+
create: () => {
39+
if (!lib.create) throw new Error('no create() behaviour was configured by this test');
40+
return lib.create();
41+
},
42+
},
43+
};
44+
});
45+
46+
import {
47+
MONGOD_TESTS_ENV,
48+
createTestMongod,
49+
disposeAbandonedDownloadGuard,
50+
installAbandonedDownloadGuard,
51+
isAbandonedDownloadRejection,
52+
mongodSkipReason,
53+
mongodTestsEnabled,
54+
printMongodNotice,
55+
} from './test-mongod.js';
56+
57+
/** The exact shape `fs.promises.rename` throws at MongoBinaryDownload.js:413. */
58+
function renameEnoent(
59+
path = '/home/runner/.cache/mongodb-binaries/mongodb-linux-x86_64-ubuntu2404-8.2.6.tgz.downloading',
60+
): Error {
61+
return Object.assign(
62+
new Error(`ENOENT: no such file or directory, rename '${path}' -> '${path.slice(0, -12)}'`),
63+
{ code: 'ENOENT', syscall: 'rename', errno: -2, path, dest: path.slice(0, -12) },
64+
);
65+
}
66+
67+
describe('[#5517] the mongod opt-in gate', () => {
68+
let warnings: string[];
69+
70+
beforeEach(() => {
71+
warnings = [];
72+
// The notices go to stderr, NOT through `console` — vitest's default reporter
73+
// renders nothing a passing/skipped file logs via `console`, which would make
74+
// the skip silent in exactly the runs that matter. `printMongodNotice`
75+
// documents the measurement.
76+
vi.spyOn(process.stderr, 'write').mockImplementation((chunk: unknown) => {
77+
warnings.push(String(chunk).trimEnd());
78+
return true;
79+
});
80+
delete process.env[MONGOD_TESTS_ENV];
81+
lib.create = undefined;
82+
});
83+
84+
afterEach(() => {
85+
vi.restoreAllMocks();
86+
delete process.env[MONGOD_TESTS_ENV];
87+
// Cases that open the gate install the guard on the REAL process. Leaving it
88+
// behind would make vitest defer its own unhandled-rejection reporting for
89+
// every later file in this worker.
90+
disposeAbandonedDownloadGuard();
91+
});
92+
93+
it('skips without touching the library at all when the switch is unset', async () => {
94+
expect(mongodTestsEnabled()).toBe(false);
95+
96+
const mongod = await createTestMongod('gate probe');
97+
98+
expect(mongod).toBeUndefined();
99+
expect(warnings).toHaveLength(1);
100+
expect(warnings[0]).toContain('#5517');
101+
expect(warnings[0]).toContain(MONGOD_TESTS_ENV);
102+
expect(warnings[0]).toContain('SKIP gate probe');
103+
// The property that makes the skip cost nothing: the module that would
104+
// download the binary was never evaluated.
105+
expect(lib.imported).toBe(0);
106+
});
107+
108+
it('treats a set-but-not-"1" value as OFF and says so instead of skipping quietly', async () => {
109+
process.env[MONGOD_TESTS_ENV] = 'true';
110+
111+
const mongod = await createTestMongod('gate probe');
112+
113+
expect(mongod).toBeUndefined();
114+
expect(lib.imported).toBe(0);
115+
expect(warnings[0]).toContain('is set to "true", which does NOT enable it');
116+
expect(mongodSkipReason('x', { [MONGOD_TESTS_ENV]: '' })).not.toContain('does NOT enable it');
117+
});
118+
119+
it('imports the library and returns a server once the switch is on', async () => {
120+
process.env[MONGOD_TESTS_ENV] = '1';
121+
const stub = { getUri: () => 'mongodb://stub', stop: async () => {} };
122+
lib.create = async () => stub;
123+
124+
const mongod = await createTestMongod('gate probe');
125+
126+
expect(mongod).toBe(stub);
127+
expect(lib.imported).toBe(1);
128+
expect(warnings).toEqual([]);
129+
});
130+
131+
it('still degrades to a named skip when an opted-in acquisition fails', async () => {
132+
process.env[MONGOD_TESTS_ENV] = '1';
133+
lib.create = async () => {
134+
throw renameEnoent();
135+
};
136+
137+
const mongod = await createTestMongod('gate probe');
138+
139+
expect(mongod).toBeUndefined();
140+
expect(warnings[0]).toContain('SKIP gate probe');
141+
expect(warnings[0]).toContain('could not start');
142+
});
143+
144+
it('prints on stderr, not through the console vitest silences on a green run', () => {
145+
const sink: string[] = [];
146+
printMongodNotice('injected', (chunk) => sink.push(chunk));
147+
expect(sink).toEqual(['injected\n']);
148+
149+
const consoleWarn = vi.spyOn(console, 'warn').mockImplementation(() => {});
150+
printMongodNotice('default sink');
151+
expect(warnings).toContain('default sink');
152+
expect(consoleWarn).not.toHaveBeenCalled();
153+
});
154+
});
155+
156+
describe('[#5517] the abandoned-download rejection guard', () => {
157+
it('recognises the rename ENOENT of a lost download race', () => {
158+
expect(isAbandonedDownloadRejection(renameEnoent())).toBe(true);
159+
});
160+
161+
it('does not recognise anything else', () => {
162+
expect(isAbandonedDownloadRejection(undefined)).toBe(false);
163+
expect(isAbandonedDownloadRejection(null)).toBe(false);
164+
expect(isAbandonedDownloadRejection('ENOENT: rename')).toBe(false);
165+
expect(isAbandonedDownloadRejection(new Error('boom'))).toBe(false);
166+
// Right code and syscall, but a real file rather than a download temp file.
167+
expect(
168+
isAbandonedDownloadRejection(
169+
Object.assign(new Error('x'), { code: 'ENOENT', syscall: 'rename', path: '/tmp/a.json' }),
170+
),
171+
).toBe(false);
172+
// A different syscall on the download temp file is somebody else's bug.
173+
expect(
174+
isAbandonedDownloadRejection(
175+
Object.assign(new Error('x'), { code: 'ENOENT', syscall: 'unlink', path: '/c/x.tgz.downloading' }),
176+
),
177+
).toBe(false);
178+
});
179+
180+
it('swallows the abandoned download and re-raises every other rejection', () => {
181+
const listeners: ((reason: unknown) => void)[] = [];
182+
const host = {
183+
on: (_event: 'unhandledRejection', listener: (reason: unknown) => void) => {
184+
listeners.push(listener);
185+
},
186+
off: (_event: 'unhandledRejection', listener: (reason: unknown) => void) => {
187+
listeners.splice(listeners.indexOf(listener), 1);
188+
},
189+
};
190+
const reraised: unknown[] = [];
191+
const warned: string[] = [];
192+
193+
installAbandonedDownloadGuard({
194+
host,
195+
reraise: (reason) => reraised.push(reason),
196+
warn: (message) => warned.push(message),
197+
});
198+
// One suite per worker installs it; the rest must not stack listeners.
199+
installAbandonedDownloadGuard({ host, reraise: () => {}, warn: () => {} });
200+
expect(listeners).toHaveLength(1);
201+
202+
listeners[0](renameEnoent());
203+
expect(reraised).toEqual([]);
204+
expect(warned).toHaveLength(1);
205+
expect(warned[0]).toContain('#5517');
206+
207+
const real = new Error('a genuine unhandled rejection');
208+
listeners[0](real);
209+
// Vitest has stepped aside because this listener exists, so the guard owes
210+
// the run a failure for anything it does not recognise.
211+
expect(reraised).toEqual([real]);
212+
expect(warned).toHaveLength(1);
213+
214+
disposeAbandonedDownloadGuard();
215+
expect(listeners).toHaveLength(0);
216+
});
217+
});

packages/plugins/driver-mongodb/src/mongodb-pagination-conformance.test.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,23 @@
2525
*/
2626

2727
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
28-
import { MongoMemoryServer } from 'mongodb-memory-server';
28+
import type { MongoMemoryServer } from 'mongodb-memory-server';
2929
import {
3030
PAGINATION_ALL_IDS,
3131
PAGINATION_CASES,
3232
PAGINATION_ROWS,
3333
PAGINATION_UNORDERED_CASES,
3434
} from '@objectstack/spec/data';
3535
import { MongoDBDriver } from './mongodb-driver.js';
36-
37-
let sharedMongod: MongoMemoryServer | undefined;
38-
try {
39-
sharedMongod = await MongoMemoryServer.create({ instance: { launchTimeout: 60_000 } });
40-
} catch (err) {
41-
console.warn(
42-
'[driver-mongodb] Skipping pagination conformance — mongodb-memory-server could not start: ' +
43-
`${(err as Error)?.message ?? String(err)}`,
44-
);
45-
}
36+
import { createTestMongod } from './test-mongod.js';
37+
38+
// The live half needs a real mongod, so it is OPT-IN since #5517 (see
39+
// `test-mongod.ts`). It used to call `MongoMemoryServer.create()` itself, which
40+
// both started a download in every default run and bypassed the package's shared
41+
// deadline. The sort-spec half below runs regardless — it needs no server.
42+
const sharedMongod: MongoMemoryServer | undefined = await createTestMongod(
43+
'pagination conformance',
44+
);
4645

4746
describe.skipIf(!sharedMongod)('driver-mongodb — paged reads are a partition of the result set', () => {
4847
const mongod = sharedMongod as MongoMemoryServer;

0 commit comments

Comments
 (0)