Skip to content

Commit 32a2f05

Browse files
kriszypclaude
andcommitted
fix(test): address quota-mode test blockers from PR review
- Add setQuotaSizeBytes() exported setter to remove rewire.__set__ calls - Add tests for getQuotaStatus() (valid file, absent, malformed JSON) - Add tests for getDirectoryUsageBytes() against a real directory - Add tests for defaultGetAvailableSpaceRatio branches: fresh file triggers/ does not trigger reclamation, over-quota clamp (ratio → 0), stale and absent file fall back to du Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ff316af commit 32a2f05

2 files changed

Lines changed: 148 additions & 9 deletions

File tree

server/storageReclamation.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ const reclamationHandlers = new Map<
1919

2020
const RECLAMATION_THRESHOLD = envMgr.get(CONFIG_PARAMS.STORAGE_RECLAMATION_THRESHOLD) ?? 0.4; // 40% remaining free space is the default
2121
const RECLAMATION_INTERVAL = convertToMS(envMgr.get(CONFIG_PARAMS.STORAGE_RECLAMATION_INTERVAL)) || 3600000; // 1 hour is the default
22-
// let so tests can override via rewire; set once from env at startup
2322
let QUOTA_SIZE_BYTES: number | undefined = convertToBytes(envMgr.get(CONFIG_PARAMS.STORAGE_QUOTASIZE));
2423

2524
// Written by host-manager every ~90s alongside the instance's hdb root
@@ -87,7 +86,7 @@ export function onStorageReclamation(
8786
}
8887
let reclamationTimer: NodeJS.Timeout;
8988

90-
// Checked at call time so that tests can override QUOTA_SIZE_BYTES via rewire.
89+
// Checked at call time so QUOTA_SIZE_BYTES changes (via setQuotaSizeBytes) take effect immediately.
9190
// In quota mode: prefer the host-manager-written quota-status file (O(1)); fall back to
9291
// `du` on the rootPath (O(inodes)) when the file is absent or stale.
9392
// The rootPath `du` covers ALL Harper files (logs, blobs, databases), matching how XFS
@@ -142,6 +141,13 @@ export function setAvailableSpaceRatioGetter(newGetter?: (path: string) => Promi
142141
getAvailableSpaceRatio = newGetter ?? defaultGetAvailableSpaceRatio;
143142
}
144143

144+
/**
145+
* Override the quota size in bytes (for testing only).
146+
*/
147+
export function setQuotaSizeBytes(n: number | undefined): void {
148+
QUOTA_SIZE_BYTES = n;
149+
}
150+
145151
/**
146152
* Returns which basis is used for free-space calculations: 'quota' when storage_quotaSize is
147153
* configured, 'filesystem' otherwise.

unitTests/server/storageReclamation.test.js

Lines changed: 140 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
'use strict';
22

33
const assert = require('node:assert/strict');
4+
const fs = require('node:fs');
5+
const os = require('node:os');
6+
const path = require('node:path');
47
const sinon = require('sinon');
58
const rewire = require('rewire');
69

@@ -35,9 +38,10 @@ describe('storageReclamation module', function () {
3538
});
3639

3740
afterEach(function () {
38-
// Reset the space ratio getter
41+
// Reset the space ratio getter and quota size
3942
if (storageReclamation) {
4043
storageReclamation.setAvailableSpaceRatioGetter(null);
44+
storageReclamation.setQuotaSizeBytes(undefined);
4145
}
4246

4347
// Clear any timers
@@ -396,26 +400,155 @@ describe('storageReclamation module', function () {
396400

397401
describe('quota mode', function () {
398402
const QUOTA_100GB = 100 * 1024 * 1024 * 1024;
403+
let tmpDir;
404+
let quotaStatusPath;
405+
406+
beforeEach(function () {
407+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'harper-quota-test-'));
408+
quotaStatusPath = path.join(tmpDir, 'quota-status.json');
409+
});
410+
411+
afterEach(function () {
412+
try {
413+
fs.rmSync(tmpDir, { recursive: true });
414+
} catch {}
415+
});
399416

400417
it('getFreeSpaceBasis returns filesystem when no quota configured', function () {
401418
assert.equal(storageReclamation.getFreeSpaceBasis(), 'filesystem');
402419
});
403420

404421
it('getFreeSpaceBasis returns quota when QUOTA_SIZE_BYTES is set', function () {
405-
storageReclamation.__set__('QUOTA_SIZE_BYTES', QUOTA_100GB);
422+
storageReclamation.setQuotaSizeBytes(QUOTA_100GB);
406423
assert.equal(storageReclamation.getFreeSpaceBasis(), 'quota');
407-
storageReclamation.__set__('QUOTA_SIZE_BYTES', undefined);
408424
});
409425

410426
it('getQuotaInfo returns undefined when no quota configured', function () {
411427
assert.equal(storageReclamation.getQuotaInfo(), undefined);
412428
});
413429

414430
it('getQuotaInfo returns quota size when QUOTA_SIZE_BYTES is set', function () {
415-
storageReclamation.__set__('QUOTA_SIZE_BYTES', QUOTA_100GB);
416-
const info = storageReclamation.getQuotaInfo();
417-
assert.deepEqual(info, { quotaSizeBytes: QUOTA_100GB });
418-
storageReclamation.__set__('QUOTA_SIZE_BYTES', undefined);
431+
storageReclamation.setQuotaSizeBytes(QUOTA_100GB);
432+
assert.deepEqual(storageReclamation.getQuotaInfo(), { quotaSizeBytes: QUOTA_100GB });
433+
});
434+
435+
describe('getQuotaStatus', function () {
436+
let originalRootPath;
437+
438+
beforeEach(function () {
439+
originalRootPath = env.get('rootPath');
440+
env.setProperty('rootPath', tmpDir);
441+
});
442+
443+
afterEach(function () {
444+
env.setProperty('rootPath', originalRootPath);
445+
});
446+
447+
it('returns parsed object when file is present and valid', async function () {
448+
const data = { usedBytes: 50_000_000_000, quotaBytes: QUOTA_100GB, updatedAt: Date.now() };
449+
fs.writeFileSync(quotaStatusPath, JSON.stringify(data));
450+
assert.deepEqual(await storageReclamation.getQuotaStatus(), data);
451+
});
452+
453+
it('returns undefined when file is absent', async function () {
454+
assert.equal(await storageReclamation.getQuotaStatus(), undefined);
455+
});
456+
457+
it('returns undefined when file contains malformed JSON', async function () {
458+
fs.writeFileSync(quotaStatusPath, 'not-valid-json{');
459+
assert.equal(await storageReclamation.getQuotaStatus(), undefined);
460+
});
461+
});
462+
463+
describe('getDirectoryUsageBytes', function () {
464+
it('returns a non-negative integer for a real directory', async function () {
465+
const bytes = await storageReclamation.getDirectoryUsageBytes(tmpDir);
466+
assert.ok(Number.isInteger(bytes));
467+
assert.ok(bytes >= 0);
468+
});
469+
});
470+
471+
describe('defaultGetAvailableSpaceRatio', function () {
472+
let originalRootPath;
473+
474+
beforeEach(function () {
475+
originalRootPath = env.get('rootPath');
476+
env.setProperty('rootPath', tmpDir);
477+
storageReclamation.setQuotaSizeBytes(QUOTA_100GB);
478+
storageReclamation.setAvailableSpaceRatioGetter(undefined); // use real default
479+
});
480+
481+
afterEach(function () {
482+
env.setProperty('rootPath', originalRootPath);
483+
});
484+
485+
it('uses fresh quota-status file and triggers reclamation when headroom is low', async function () {
486+
const usedBytes = 65 * 1024 * 1024 * 1024; // 65 GB → 35% remaining → below 40% threshold
487+
fs.writeFileSync(
488+
quotaStatusPath,
489+
JSON.stringify({ usedBytes, quotaBytes: QUOTA_100GB, updatedAt: Date.now() })
490+
);
491+
492+
const handler = sandbox.stub().returns(Promise.resolve());
493+
storageReclamation.onStorageReclamation(tmpDir, handler, true);
494+
await storageReclamation.runReclamationHandlers();
495+
496+
assert.ok(handler.calledOnce);
497+
assert.ok(handler.firstCall.args[0] > 1); // priority = 0.4 / 0.35 ≈ 1.14
498+
});
499+
500+
it('uses fresh quota-status file and does not trigger when headroom is sufficient', async function () {
501+
const usedBytes = 50 * 1024 * 1024 * 1024; // 50% used → 50% remaining → above threshold
502+
fs.writeFileSync(
503+
quotaStatusPath,
504+
JSON.stringify({ usedBytes, quotaBytes: QUOTA_100GB, updatedAt: Date.now() })
505+
);
506+
507+
const handler = sandbox.stub();
508+
storageReclamation.onStorageReclamation(tmpDir, handler, true);
509+
await storageReclamation.runReclamationHandlers();
510+
511+
assert.ok(handler.notCalled);
512+
});
513+
514+
it('clamps ratio to 0 and triggers reclamation when usage exceeds quota', async function () {
515+
const usedBytes = 110 * 1024 * 1024 * 1024; // 10 GB over quota
516+
fs.writeFileSync(
517+
quotaStatusPath,
518+
JSON.stringify({ usedBytes, quotaBytes: QUOTA_100GB, updatedAt: Date.now() })
519+
);
520+
521+
const handler = sandbox.stub().returns(Promise.resolve());
522+
storageReclamation.onStorageReclamation(tmpDir, handler, true);
523+
await storageReclamation.runReclamationHandlers();
524+
525+
// Ratio clamped to 0 → priority = Infinity → handler called
526+
assert.ok(handler.calledOnce);
527+
});
528+
529+
it('falls back to du when quota-status file is absent', async function () {
530+
// No quota-status.json; du reports actual tmpDir usage which is far below 100 GB
531+
const handler = sandbox.stub();
532+
storageReclamation.onStorageReclamation(tmpDir, handler, true);
533+
await storageReclamation.runReclamationHandlers();
534+
535+
assert.ok(handler.notCalled);
536+
});
537+
538+
it('falls back to du when quota-status file is stale', async function () {
539+
const staleTimestamp = Date.now() - 10 * 60 * 1000; // 10 minutes old
540+
fs.writeFileSync(
541+
quotaStatusPath,
542+
JSON.stringify({ usedBytes: 65 * 1024 * 1024 * 1024, quotaBytes: QUOTA_100GB, updatedAt: staleTimestamp })
543+
);
544+
545+
// Despite the stale "65 GB" reading, du reports actual tmpDir usage (far below 100 GB)
546+
const handler = sandbox.stub();
547+
storageReclamation.onStorageReclamation(tmpDir, handler, true);
548+
await storageReclamation.runReclamationHandlers();
549+
550+
assert.ok(handler.notCalled);
551+
});
419552
});
420553

421554
it('quota-aware ratio triggers reclamation when usage exceeds threshold headroom', async function () {

0 commit comments

Comments
 (0)