Skip to content

Commit 23a0c83

Browse files
kriszypclaude
andcommitted
test: make OpenDBIObject config test immune to a leaked envMngr.get stub
Another test in the full unit suite leaves envMngr.get wrapped by sinon and never restores it, which broke both prior versions of this test in CI: the setProperty version read the leaked stub's value instead of the set value, and the sinon.stub version threw "already wrapped". Override the getter via a defineProperty save/replace/restore that delegates to whatever get currently is (real or another test's stub) and restores the exact prior descriptor — proven against a simulated leak locally. Adds a non-boolean ("true" string) case to guard the strict === true comparison. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a41b47e commit 23a0c83

1 file changed

Lines changed: 34 additions & 29 deletions

File tree

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
require('../../testUtils');
22
const assert = require('assert');
3-
const sinon = require('sinon');
43
const { OpenDBIObject } = require('#src/utility/lmdb/OpenDBIObject');
54
const envMngr = require('#src/utility/environment/environmentManager');
65
const { CONFIG_PARAMS } = require('#src/utility/hdbTerms');
@@ -10,48 +9,54 @@ const { CONFIG_PARAMS } = require('#src/utility/hdbTerms');
109
// in databases.ts before the store opens, bypassing this constructor branch — so a wrong config key,
1110
// a non-boolean value, or a hdbTerms/YAML name mismatch would go uncaught.
1211
//
13-
// The constructor reads the value via envMngr.get(CONFIG_PARAMS.STORAGE_RANDOMACCESSFIELDS), so we
14-
// stub that getter per-test (restored in afterEach) rather than mutating shared config state — the
15-
// latter is order-dependent across the full unit suite and isn't reliably isolated. callThrough()
16-
// keeps every other config read (e.g. STORAGE_CACHING) on the real value; only the random-access
17-
// key is forced, which also asserts the constructor reads the *correct* key (a wrong key would fall
18-
// through to the real value and fail these assertions).
19-
describe('OpenDBIObject storage.randomAccessFields global config', () => {
20-
let getStub;
21-
afterEach(() => {
22-
if (getStub) {
23-
getStub.restore();
24-
getStub = undefined;
25-
}
12+
// The constructor reads the value via envMngr.get(CONFIG_PARAMS.STORAGE_RANDOMACCESSFIELDS). We
13+
// temporarily override that single getter via defineProperty (restored in a finally) rather than
14+
// using sinon or envMngr.setProperty: another test in the full unit suite leaves envMngr.get wrapped
15+
// by sinon, which (a) makes setProperty's value invisible behind that stub and (b) makes a second
16+
// sinon.stub throw "already wrapped". Saving/replacing/restoring the property descriptor is immune to
17+
// that — the replacement delegates to whatever get currently is (real or another test's stub) for
18+
// every other key, and restores the exact prior descriptor afterward.
19+
function withRandomAccessFields(value, fn) {
20+
const previousDescriptor = Object.getOwnPropertyDescriptor(envMngr, 'get');
21+
const currentGet = envMngr.get;
22+
Object.defineProperty(envMngr, 'get', {
23+
configurable: true,
24+
writable: true,
25+
value: (key) => (key === CONFIG_PARAMS.STORAGE_RANDOMACCESSFIELDS ? value : currentGet(key)),
2626
});
27-
28-
function stubRandomAccessFields(value) {
29-
getStub = sinon.stub(envMngr, 'get');
30-
getStub.callThrough();
31-
getStub.withArgs(CONFIG_PARAMS.STORAGE_RANDOMACCESSFIELDS).returns(value);
27+
try {
28+
fn();
29+
} finally {
30+
Object.defineProperty(envMngr, 'get', previousDescriptor);
3231
}
32+
}
3333

34+
describe('OpenDBIObject storage.randomAccessFields global config', () => {
3435
it('enables randomAccessStructure on a primary DBI when the global config is true', () => {
35-
stubRandomAccessFields(true);
36-
assert.strictEqual(new OpenDBIObject(false, true).randomAccessStructure, true);
36+
withRandomAccessFields(true, () => {
37+
assert.strictEqual(new OpenDBIObject(false, true).randomAccessStructure, true);
38+
});
3739
});
3840

3941
it('leaves randomAccessStructure off on a primary DBI when the global config is false (default)', () => {
40-
stubRandomAccessFields(false);
41-
assert.strictEqual(new OpenDBIObject(false, true).randomAccessStructure, false);
42+
withRandomAccessFields(false, () => {
43+
assert.strictEqual(new OpenDBIObject(false, true).randomAccessStructure, false);
44+
});
4245
});
4346

4447
it('treats a non-boolean truthy config value as off (strict === true)', () => {
45-
// envMngr should hand back a real boolean; guard the strict comparison so a stray truthy
46-
// (e.g. the string "true") can't silently flip encoding on.
47-
stubRandomAccessFields('true');
48-
assert.strictEqual(new OpenDBIObject(false, true).randomAccessStructure, false);
48+
// envMngr should hand back a real boolean; the strict === true guards against a stray truthy
49+
// (e.g. the string "true") silently flipping encoding on.
50+
withRandomAccessFields('true', () => {
51+
assert.strictEqual(new OpenDBIObject(false, true).randomAccessStructure, false);
52+
});
4953
});
5054

5155
it('keeps randomAccessStructure off on non-primary DBIs even when the global config is true', () => {
5256
// Non-primary stores (e.g. the __dbis__ metadata DBI) must stay in records mode for
5357
// v4-downgrade decodability, regardless of the global setting.
54-
stubRandomAccessFields(true);
55-
assert.strictEqual(new OpenDBIObject(false, false).randomAccessStructure, false);
58+
withRandomAccessFields(true, () => {
59+
assert.strictEqual(new OpenDBIObject(false, false).randomAccessStructure, false);
60+
});
5661
});
5762
});

0 commit comments

Comments
 (0)