From 371de4ca8ebb49c933cd6c92a162a46845109c01 Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Wed, 27 May 2026 14:26:27 +0200 Subject: [PATCH] test(fit): assert mxGraphView calls in the fit integration tests Replace the smoke tests (which only checked that no error was thrown) with assertions on the scaleAndTranslate calls and the resulting scale, for every FitType and for the margin option. Mock getGraphBounds and force the container dimensions to make the scale and translation deterministic and decoupled from the parse/render pipeline. This prepares the migration to maxGraph (the mxGraph successor, with a close API) by mirroring its FitPlugin tests: configure the container dimensions, then verify the underlying mxGraphView method calls and their arguments. Pinning the exact scale and translation values lets us detect any numerical discrepancy introduced by the migration. The e2e visual tests also catch such discrepancies, on the whole computation against the real rendered bounds; these integration tests complement them with fast, precise, per-branch checks. --- test/integration/diagram.navigation.test.ts | 151 +++++++++++++++++++- 1 file changed, 144 insertions(+), 7 deletions(-) diff --git a/test/integration/diagram.navigation.test.ts b/test/integration/diagram.navigation.test.ts index c5d475f666..c1fd1ebd2e 100644 --- a/test/integration/diagram.navigation.test.ts +++ b/test/integration/diagram.navigation.test.ts @@ -15,26 +15,163 @@ limitations under the License. */ import { initializeBpmnVisualizationWithHtmlElement } from './helpers/bpmn-visualization-initialization'; -import { allTestedFitTypes } from './helpers/fit-utils'; -import { type FitType, ZoomType } from '@lib/component/options'; +import { mxRectangle } from '@lib/component/mxgraph/initializer'; +import { FitType, ZoomType } from '@lib/component/options'; import { readFileSync } from '@test/shared/file-helper'; const bpmnVisualization = initializeBpmnVisualizationWithHtmlElement('bpmn-container', true); +type ContainerDimensions = Partial>; + +// jsdom always reports 0 for the container dimensions, so force them to assert the scale/translation computed by 'fit'. +const setContainerDimensions = (container: HTMLElement, dimensions: ContainerDimensions): void => { + for (const [name, value] of Object.entries(dimensions)) { + Object.defineProperty(container, name, { value, configurable: true }); + } +}; + describe('diagram navigation', () => { beforeEach(() => { bpmnVisualization.load(readFileSync('../fixtures/bpmn/simple-start-task-end.bpmn')); }); - // The following tests ensure there is no error when calling the fit method describe('Fit', () => { - it('Fit no options', () => { - bpmnVisualization.navigation.fit(); + // The rendered graph bounds are not predictable, so mock them to assert the exact scale and translation computed by + // 'fit'. Combined with 'setContainerDimensions', this reproduces the approach of the maxGraph 'FitPlugin' tests: + // configure the container dimensions, then verify the calls to the underlying mxGraphView methods. + const view = bpmnVisualization.graph.view; + let getGraphBoundsSpy: jest.SpyInstance | undefined; + + const configureFitScenario = ( + dimensions: ContainerDimensions, + bounds: { x: number; y: number; width: number; height: number }, + ): { scaleAndTranslateSpy: jest.SpyInstance; setScaleSpy: jest.SpyInstance } => { + setContainerDimensions(bpmnVisualization.graph.container, dimensions); + // Mock the bounds getter (not 'setGraphBounds', which the 'zoomActual' call in 'fit' would revalidate and clobber) + // to make the scale/translation deterministic and decoupled from the parse/render pipeline. The full computation + // on real bounds is covered by the e2e visual tests. + getGraphBoundsSpy = jest.spyOn(view, 'getGraphBounds').mockReturnValue(new mxRectangle(bounds.x, bounds.y, bounds.width, bounds.height)); + return { + scaleAndTranslateSpy: jest.spyOn(view, 'scaleAndTranslate').mockClear(), + setScaleSpy: jest.spyOn(view, 'setScale').mockClear(), + }; + }; + + afterEach(() => { + // Restore only the bounds mock to avoid leaking it into other tests sharing the same instance. + getGraphBoundsSpy?.mockRestore(); + getGraphBoundsSpy = undefined; + }); + + describe('No scaling', () => { + // 'null' is loosely equal to 'undefined' in the implementation, so both trigger the early return. + it.each([undefined, null, FitType.None])('Fit with %s does not scale the diagram', (type: FitType | null | undefined) => { + const scaleAndTranslateSpy = jest.spyOn(view, 'scaleAndTranslate').mockClear(); + const setScaleSpy = jest.spyOn(view, 'setScale').mockClear(); + + bpmnVisualization.navigation.fit({ type: type as FitType }); + + expect(scaleAndTranslateSpy).not.toHaveBeenCalled(); + expect(setScaleSpy).not.toHaveBeenCalled(); + expect(view.scale).toBe(1); + }); + + it('Fit without options does not scale the diagram', () => { + const scaleAndTranslateSpy = jest.spyOn(view, 'scaleAndTranslate').mockClear(); + const setScaleSpy = jest.spyOn(view, 'setScale').mockClear(); + + bpmnVisualization.navigation.fit(); + + expect(scaleAndTranslateSpy).not.toHaveBeenCalled(); + expect(setScaleSpy).not.toHaveBeenCalled(); + expect(view.scale).toBe(1); + }); + }); + + describe('Center', () => { + // Center uses the container 'client' dimensions and computes the scale and translation itself. + it('Center the diagram', () => { + const { scaleAndTranslateSpy, setScaleSpy } = configureFitScenario({ clientWidth: 200, clientHeight: 200 }, { x: 0, y: 0, width: 100, height: 100 }); + + bpmnVisualization.navigation.fit({ type: FitType.Center }); + + expect(scaleAndTranslateSpy).toHaveBeenCalledExactlyOnceWith(2, 0, 0); + expect(setScaleSpy).not.toHaveBeenCalled(); + expect(view.scale).toBe(2); + }); + + it('Limit the scale to the maximum value (3)', () => { + const { scaleAndTranslateSpy } = configureFitScenario({ clientWidth: 2000, clientHeight: 2000 }, { x: 0, y: 0, width: 100, height: 100 }); + + bpmnVisualization.navigation.fit({ type: FitType.Center }); + + expect(scaleAndTranslateSpy).toHaveBeenCalledExactlyOnceWith(3, expect.closeTo(283.33), expect.closeTo(283.33)); + expect(view.scale).toBe(3); + }); + + it('Take the margin into account', () => { + const { scaleAndTranslateSpy } = configureFitScenario({ clientWidth: 200, clientHeight: 200 }, { x: 0, y: 0, width: 100, height: 100 }); + + bpmnVisualization.navigation.fit({ type: FitType.Center, margin: 30 }); + + expect(scaleAndTranslateSpy).toHaveBeenCalledExactlyOnceWith(1.7, expect.closeTo(8.82), expect.closeTo(8.82)); + expect(view.scale).toBe(1.7); + }); + + it('Ignore a negative margin (treated as 0)', () => { + const { scaleAndTranslateSpy } = configureFitScenario({ clientWidth: 200, clientHeight: 200 }, { x: 0, y: 0, width: 100, height: 100 }); + + bpmnVisualization.navigation.fit({ type: FitType.Center, margin: -50 }); + + expect(scaleAndTranslateSpy).toHaveBeenCalledExactlyOnceWith(2, 0, 0); + expect(view.scale).toBe(2); + }); }); - it.each(allTestedFitTypes)('Fit with %s', (fitType: string) => { - bpmnVisualization.navigation.fit({ type: fitType as FitType }); + describe('Delegating to the mxGraph fit', () => { + // These types use the container 'offset' dimensions and delegate the computation to mxGraph 'fit'. + // An unknown type behaves like 'HorizontalVertical' (no dimension ignored). + // A positive margin lowers the scale and shifts the translation by 'margin / 2'; it still shifts the translation + // even when the scale is capped to the maximum value (8). A negative margin is treated as 0 (see the 'Center' tests). + it.each` + type | dimensions | bounds | margin | expectedScale | expectedTranslateX | expectedTranslateY + ${FitType.HorizontalVertical} | ${{ offsetWidth: 480, offsetHeight: 820 }} | ${{ x: 70, y: -60, width: 100, height: 100 }} | ${undefined} | ${4.78} | ${-70} | ${60} + ${FitType.Horizontal} | ${{ offsetWidth: 860, offsetHeight: 860 }} | ${{ x: 70, y: -60, width: 150, height: 300 }} | ${undefined} | ${5.72} | ${-70} | ${60} + ${FitType.Vertical} | ${{ offsetWidth: 860, offsetHeight: 860 }} | ${{ x: 70, y: -60, width: 1000, height: 100 }} | ${undefined} | ${8} | ${-70} | ${60} + ${'invalid'} | ${{ offsetWidth: 480, offsetHeight: 820 }} | ${{ x: 70, y: -60, width: 100, height: 100 }} | ${undefined} | ${4.78} | ${-70} | ${60} + ${FitType.HorizontalVertical} | ${{ offsetWidth: 480, offsetHeight: 820 }} | ${{ x: 70, y: -60, width: 100, height: 100 }} | ${30} | ${4.48} | ${-55} | ${75} + ${FitType.Horizontal} | ${{ offsetWidth: 860, offsetHeight: 860 }} | ${{ x: 70, y: -60, width: 150, height: 300 }} | ${30} | ${5.52} | ${-55} | ${75} + ${FitType.Vertical} | ${{ offsetWidth: 860, offsetHeight: 860 }} | ${{ x: 70, y: -60, width: 1000, height: 100 }} | ${30} | ${8} | ${-55} | ${75} + ${FitType.Vertical} | ${{ offsetWidth: 860, offsetHeight: 860 }} | ${{ x: 70, y: -60, width: 1000, height: 100 }} | ${100} | ${7.58} | ${-20} | ${110} + `( + 'Fit with $type (margin: $margin)', + ({ + type, + dimensions, + bounds, + margin, + expectedScale, + expectedTranslateX, + expectedTranslateY, + }: { + type: FitType; + dimensions: ContainerDimensions; + bounds: { x: number; y: number; width: number; height: number }; + margin: number | undefined; + expectedScale: number; + expectedTranslateX: number; + expectedTranslateY: number; + }) => { + const { scaleAndTranslateSpy, setScaleSpy } = configureFitScenario(dimensions, bounds); + + bpmnVisualization.navigation.fit({ type, margin }); + + expect(scaleAndTranslateSpy).toHaveBeenCalledExactlyOnceWith(expectedScale, expectedTranslateX, expectedTranslateY); + expect(setScaleSpy).not.toHaveBeenCalled(); + expect(view.scale).toBe(expectedScale); + }, + ); }); });