test(fonts): fix mock bypass and case-insensitive FS skip (#390 follow-up)
Two issues in the fonts service test suite added by #390 — the behaviour assertions all passed, but 5 of 24 tests had assertions that silently no-op'd, so any regression in those code paths would not have been caught. ## Issue 1: jest.resetModules() bypassed the logger mock `beforeEach` called `jest.resetModules()` then re-required `fontsService`. After resetModules, the `jest.mock('../../src/utils/logger', ...)` factory at the top of the file no longer applied to subsequent requires — so the freshly-required `fontsService` captured the REAL logger while the test file's `logger` variable still pointed at the mocked one. The 4 "warning logged" / "info logged" assertions resolved as 0 calls and silently passed-as-noop. The resetModules call wasn't necessary in the first place — module-level state in fontsService is just the cache, which clearFontsCache() already resets. And both getBundledFontsRoot() and getUserFontsRoot() read process.env at call-time, not at module load, so the env vars set in beforeEach are picked up without needing a fresh require. Fix: require fontsService once at module top (inside the jest.mock hoisting scope) and drop resetModules + the per-test re-require. ## Issue 2: case-insensitive filesystem (macOS / Windows) The "case-insensitive duplicate within the same root" test created `Inter/` and `INTER/` to trigger the dedup warning. On a case-sensitive FS (Linux ext4) both directory entries exist and the dedup branch fires; on macOS APFS or Windows NTFS the second mkdir resolves to the same folder as the first, so only one ever exists and the dedup is unreachable from this test setup. Test failed on macOS dev, passed on Linux CI. Fix: probe at load time by creating a lowercase file and checking if its uppercase variant resolves to the same inode, then conditionally test.skip the affected test on case-insensitive hosts. Comment in the test body explains why. ## Result 23 of 24 tests now pass on macOS; the case-sensitive-only test runs on Linux CI. All previously-no-op'd assertions now exercise their code paths.
This commit is contained in:
@@ -13,10 +13,35 @@ jest.mock('../../src/utils/logger', () => ({
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
const logger = require('../../src/utils/logger');
|
const logger = require('../../src/utils/logger');
|
||||||
|
// Required ONCE at module top so the jest.mock factory above applies to
|
||||||
|
// the logger reference that fontsService captures. A previous version
|
||||||
|
// re-required it inside beforeEach() with jest.resetModules() — that
|
||||||
|
// silently bypassed the mock (logger calls went to the real logger),
|
||||||
|
// so the "warning logged" assertions would resolve as 0 calls and
|
||||||
|
// silently pass-as-noop. Module-level state in fontsService is just
|
||||||
|
// the cache, which clearFontsCache() resets between tests.
|
||||||
|
const fontsService = require('../../src/services/fontsService');
|
||||||
|
|
||||||
|
// Probe at load time: is the host filesystem case-sensitive?
|
||||||
|
// macOS APFS and Windows NTFS treat "Inter" and "INTER" as the same
|
||||||
|
// directory entry, which means the "two folders, same lowercase key"
|
||||||
|
// dedup test below can't be set up via real folders on those platforms —
|
||||||
|
// the second mkdir is a no-op. Skip that one test conditionally.
|
||||||
|
const FS_IS_CASE_SENSITIVE = (() => {
|
||||||
|
const probeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-fs-probe-'));
|
||||||
|
fs.writeFileSync(path.join(probeDir, 'casetest'), '');
|
||||||
|
let sensitive = true;
|
||||||
|
try {
|
||||||
|
fs.accessSync(path.join(probeDir, 'CASETEST'));
|
||||||
|
sensitive = false;
|
||||||
|
} catch { /* file not found → case-sensitive FS */ }
|
||||||
|
fs.rmSync(probeDir, { recursive: true, force: true });
|
||||||
|
return sensitive;
|
||||||
|
})();
|
||||||
|
const testCaseSensitiveFS = FS_IS_CASE_SENSITIVE ? test : test.skip;
|
||||||
|
|
||||||
let bundledRoot;
|
let bundledRoot;
|
||||||
let userRoot;
|
let userRoot;
|
||||||
let fontsService;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create a font family folder with the given weights (and optional meta.json).
|
* Create a font family folder with the given weights (and optional meta.json).
|
||||||
@@ -56,10 +81,11 @@ beforeEach(async () => {
|
|||||||
userRoot = path.join(storageParent, 'fonts');
|
userRoot = path.join(storageParent, 'fonts');
|
||||||
process.env.STORAGE_PATH = storageParent;
|
process.env.STORAGE_PATH = storageParent;
|
||||||
|
|
||||||
// Re-require fresh after env is set so module-level constants (none here,
|
// Reset the module-level cache so each test sees a fresh scan.
|
||||||
// but cache state is module-level) start clean.
|
// (Both getBundledFontsRoot and getUserFontsRoot read process.env at
|
||||||
jest.resetModules();
|
// call-time, so the env vars set above are picked up without needing
|
||||||
fontsService = require('../../src/services/fontsService');
|
// to re-require the module — see fontsService.js getBundledFontsRoot /
|
||||||
|
// getUserFontsRoot.)
|
||||||
fontsService.clearFontsCache();
|
fontsService.clearFontsCache();
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -170,11 +196,12 @@ describe('fontsService.listFonts', () => {
|
|||||||
expect(fonts.map((f) => f.family)).toEqual(['Lobster']);
|
expect(fonts.map((f) => f.family)).toEqual(['Lobster']);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('case-insensitive duplicate within the same root → second skipped, warning', async () => {
|
testCaseSensitiveFS('case-insensitive duplicate within the same root → second skipped, warning', async () => {
|
||||||
// Two different folder names both producing the family "Inter".
|
// Two folder names whose lowercase keys collide. On a case-sensitive
|
||||||
// On case-insensitive filesystems (APFS) this can't actually happen at
|
// FS (Linux ext4) we can create both `Inter/` and `INTER/`; on a
|
||||||
// the FS layer; we simulate by using two different display names that
|
// case-insensitive FS (macOS APFS, Windows NTFS) the second mkdir
|
||||||
// normalize identically. "Inter" and "INTER" lowercase to the same key.
|
// resolves to the same directory as the first and the dedup branch
|
||||||
|
// is unreachable from this test setup — see testCaseSensitiveFS above.
|
||||||
await makeFamily(bundledRoot, 'Inter', [400]);
|
await makeFamily(bundledRoot, 'Inter', [400]);
|
||||||
await makeFamily(bundledRoot, 'INTER', [700]);
|
await makeFamily(bundledRoot, 'INTER', [700]);
|
||||||
const fonts = await fontsService.listFonts();
|
const fonts = await fontsService.listFonts();
|
||||||
|
|||||||
Reference in New Issue
Block a user