From a23bba0f5ca93a58bf385c009c40797f28d1a761 Mon Sep 17 00:00:00 2001 From: Luc Patiny Date: Mon, 29 Jun 2026 11:00:24 +0200 Subject: [PATCH 1/3] test: simplify the canonized-prochirality cache transfer test Assert the transferred cache entry directly (reference equality on `prochiralityByEnantioID`) instead of monkey-patching OCL's `ensureHelperArrays` to count CIP recomputations. The shared reference is the optimisation, so this tests intent directly and drops the spy, the try/finally restore, and coupling to OCL internals. --- ...olecule.setProchiralHydrogenLabels.test.js | 30 ++++++------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/src/topic/__tests__/TopicMolecule.setProchiralHydrogenLabels.test.js b/src/topic/__tests__/TopicMolecule.setProchiralHydrogenLabels.test.js index e346f40..2ab4c11 100644 --- a/src/topic/__tests__/TopicMolecule.setProchiralHydrogenLabels.test.js +++ b/src/topic/__tests__/TopicMolecule.setProchiralHydrogenLabels.test.js @@ -204,30 +204,18 @@ test('canonizedProchiralities is transferred across TopicMolecule instances with // (e.g. after expanding hydrogens) should not recompute CIP. const molecule = Molecule.fromSmiles('CC(Cl)CC'); const original = new TopicMolecule(molecule); - const labels = original.prochiralities; - const labelledCount = labels.filter((l) => l !== undefined).length; - expect(labelledCount).toBe(2); + const reused = original.fromMolecule(molecule); - const oclModule = molecule.getOCL().Molecule; - let cipCalls = 0; - const originalEnsure = oclModule.prototype.ensureHelperArrays; - oclModule.prototype.ensureHelperArrays = function spy(bits) { - if (bits === oclModule.cHelperCIP) cipCalls++; - return originalEnsure.call(this, bits); - }; - - try { - const reused = original.fromMolecule(molecule); - const reusedLabels = reused.prochiralities; + // The transferred entry is the very same object: that shared reference is the + // whole optimisation — there is nothing to recompute. + expect(reused.cache.prochiralityByEnantioID).toBe( + original.cache.prochiralityByEnantioID, + ); - expect( - reusedLabels.filter((l) => l !== undefined).toSorted(), - ).toStrictEqual(['r', 's']); - expect(cipCalls).toBe(0); - } finally { - oclModule.prototype.ensureHelperArrays = originalEnsure; - } + expect( + reused.prochiralities.filter((l) => l !== undefined).toSorted(), + ).toStrictEqual(['r', 's']); }); test('CC(Cl)CC: implicit-H molecule gets no labels, only moleculeWithH does', () => { From 49732b947d2ffe73c69bf03ef51fb590f3c5ad8f Mon Sep 17 00:00:00 2001 From: Luc Patiny Date: Mon, 29 Jun 2026 12:56:43 +0200 Subject: [PATCH 2/3] test: check the number of time ensureHelper is called to confirm cache is being used. --- ...olecule.setProchiralHydrogenLabels.test.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/topic/__tests__/TopicMolecule.setProchiralHydrogenLabels.test.js b/src/topic/__tests__/TopicMolecule.setProchiralHydrogenLabels.test.js index 2ab4c11..5591b14 100644 --- a/src/topic/__tests__/TopicMolecule.setProchiralHydrogenLabels.test.js +++ b/src/topic/__tests__/TopicMolecule.setProchiralHydrogenLabels.test.js @@ -1,5 +1,5 @@ import { Molecule } from 'openchemlib'; -import { expect, test } from 'vitest'; +import { expect, onTestFinished, test, vi } from 'vitest'; import { TopicMolecule } from '../TopicMolecule'; @@ -205,6 +205,17 @@ test('canonizedProchiralities is transferred across TopicMolecule instances with const molecule = Molecule.fromSmiles('CC(Cl)CC'); const original = new TopicMolecule(molecule); + // Compute the original labels once, populating its prochirality cache. + expect( + original.prochiralities.filter((l) => l !== undefined).toSorted(), + ).toStrictEqual(['r', 's']); + + // Count only the dedicated CIP computation (the exact cHelperCIP bit); + // composite requests that merely include that bit are unrelated. + const oclMolecule = molecule.getOCL().Molecule; + const spy = vi.spyOn(oclMolecule.prototype, 'ensureHelperArrays'); + onTestFinished(() => spy.mockRestore()); + const reused = original.fromMolecule(molecule); // The transferred entry is the very same object: that shared reference is the @@ -216,6 +227,12 @@ test('canonizedProchiralities is transferred across TopicMolecule instances with expect( reused.prochiralities.filter((l) => l !== undefined).toSorted(), ).toStrictEqual(['r', 's']); + + const cipCalls = spy.mock.calls.filter( + ([bits]) => bits === oclMolecule.cHelperCIP, + ).length; + + expect(cipCalls).toBe(0); }); test('CC(Cl)CC: implicit-H molecule gets no labels, only moleculeWithH does', () => { From 7a95ca571f4d9441cfb93804a17eef2387a163ca Mon Sep 17 00:00:00 2001 From: Luc Patiny Date: Mon, 29 Jun 2026 13:52:25 +0200 Subject: [PATCH 3/3] test: use vi.spyOn instead of manual prototype spy in getCanonizedDiaIDs Assisted-By: Claude Opus 4.8 (1M context) --- .../__tests__/getCanonizedDiaIDs.test.js | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/topic/__tests__/getCanonizedDiaIDs.test.js b/src/topic/__tests__/getCanonizedDiaIDs.test.js index 5eee4de..215e641 100644 --- a/src/topic/__tests__/getCanonizedDiaIDs.test.js +++ b/src/topic/__tests__/getCanonizedDiaIDs.test.js @@ -1,5 +1,5 @@ import { Molecule } from 'openchemlib'; -import { expect, test } from 'vitest'; +import { expect, onTestFinished, test, vi } from 'vitest'; import { TopicMolecule } from '../TopicMolecule'; @@ -10,22 +10,14 @@ test('symmetric atoms with the same heterotopic rank share a cached diaID (no re const topicMolecule = new TopicMolecule(molecule); const oclModule = molecule.getOCL().Molecule; - let canonizationCalls = 0; - const originalGetCanonizedIDCode = oclModule.prototype.getCanonizedIDCode; - oclModule.prototype.getCanonizedIDCode = function spy(...args) { - canonizationCalls++; - return originalGetCanonizedIDCode.apply(this, args); - }; - - try { - const diaIDs = topicMolecule.diaIDs; - - expect(diaIDs).toHaveLength(12); - expect(canonizationCalls).toBe(2); - expect(new Set(diaIDs).size).toBe(2); - } finally { - oclModule.prototype.getCanonizedIDCode = originalGetCanonizedIDCode; - } + const spy = vi.spyOn(oclModule.prototype, 'getCanonizedIDCode'); + onTestFinished(() => spy.mockRestore()); + + const diaIDs = topicMolecule.diaIDs; + + expect(diaIDs).toHaveLength(12); + expect(spy).toHaveBeenCalledTimes(2); + expect(new Set(diaIDs).size).toBe(2); }); test('a molecule with all-distinct atoms still gets one canonization per atom', () => {