Skip to content

Commit a22c0af

Browse files
authored
test(superdoc): tighten assertions + adjust codecov policy (#2841)
Follow-up to the coverage push, addressing feedback from an external review of the new test suite. - Replace tautological assertions (expect(true).toBe(true)) and does-not-throw placeholders with observable-state assertions on the Delete-keydown-from-input branch and image reconciliation - Fix window keydown listener leak in WhiteboardPage-deep tests by destroying mounted pages in afterEach - Rewrite the "clicking an image selects it" test to assert the selection overlay was attached to the visible layer, not just that the Transformer constructor was invoked - Trim create-app-edges from 7 queue-mechanics tests to 4 outcome tests; the suite now survives refactors of the suppression strategy - Migrate setTimeout(r, 0) microtask flushing to await Promise.resolve() codecov.yml: set project and patch targets to 90% (from auto). Avoids the coverage-gaming incentives of a 95% project target on a UI-heavy library and sets a sustainable bar for new code.
1 parent 33377b9 commit a22c0af

4 files changed

Lines changed: 94 additions & 93 deletions

File tree

codecov.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ coverage:
22
status:
33
project:
44
default:
5-
target: auto
5+
target: 90%
66
threshold: 1%
77
patch:
88
default:
9-
target: auto
10-
threshold: 1%
9+
target: 90%
10+
threshold: 0%

packages/superdoc/src/core/create-app-edges.test.js

Lines changed: 48 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,7 @@ vi.mock('../SuperDoc.vue', () => ({ default: { name: 'SuperDocMock' } }));
1919

2020
const setupAppMocks = () => {
2121
const originalUnmount = vi.fn();
22-
const app = {
23-
use: vi.fn(),
24-
directive: vi.fn(),
25-
unmount: originalUnmount,
26-
};
22+
const app = { use: vi.fn(), directive: vi.fn(), unmount: originalUnmount };
2723
createAppMock.mockReturnValue(app);
2824
createPiniaMock.mockReturnValue({});
2925
useSuperdocStoreMock.mockReturnValue({});
@@ -34,12 +30,10 @@ const setupAppMocks = () => {
3430

3531
const safeDelete = (key) => {
3632
const desc = Object.getOwnPropertyDescriptor(globalThis, key);
37-
if (!desc || desc.configurable !== false) {
38-
delete globalThis[key];
39-
}
33+
if (!desc || desc.configurable !== false) delete globalThis[key];
4034
};
4135

42-
describe('createSuperdocVueApp edge cases', () => {
36+
describe('createSuperdocVueApp', () => {
4337
beforeEach(() => {
4438
vi.resetModules();
4539
vi.clearAllMocks();
@@ -52,49 +46,7 @@ describe('createSuperdocVueApp edge cases', () => {
5246
safeDelete('__VUE_DEVTOOLS_PLUGINS__');
5347
});
5448

55-
it('handles queue replacement via property setter', async () => {
56-
const { app } = setupAppMocks();
57-
const { createSuperdocVueApp } = await import('./create-app.js');
58-
createSuperdocVueApp({ disablePiniaDevtools: true });
59-
60-
const newQueue = [];
61-
globalThis.__VUE_DEVTOOLS_PLUGINS__ = newQueue;
62-
newQueue.push([{ id: 'dev.esm.pinia', app }, vi.fn()]);
63-
expect(newQueue).toHaveLength(0);
64-
65-
const otherApp = {};
66-
newQueue.push([{ id: 'dev.esm.pinia', app: otherApp }, vi.fn()]);
67-
expect(newQueue).toHaveLength(1);
68-
});
69-
70-
it('handles multiple unmount calls safely', async () => {
71-
const { app } = setupAppMocks();
72-
const { createSuperdocVueApp } = await import('./create-app.js');
73-
createSuperdocVueApp({ disablePiniaDevtools: true });
74-
app.unmount();
75-
app.unmount(); // second call — no-op via ref count guard
76-
expect(true).toBe(true);
77-
});
78-
79-
it('handles non-array pre-existing queue', async () => {
80-
globalThis.__VUE_DEVTOOLS_PLUGINS__ = { notAnArray: true };
81-
const { app } = setupAppMocks();
82-
const { createSuperdocVueApp } = await import('./create-app.js');
83-
expect(() => createSuperdocVueApp({ disablePiniaDevtools: true })).not.toThrow();
84-
});
85-
86-
it('emits unrelated events without suppression', async () => {
87-
const emitSpy = vi.fn(() => 'emitted');
88-
globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__ = { emit: emitSpy };
89-
const { app } = setupAppMocks();
90-
const { createSuperdocVueApp } = await import('./create-app.js');
91-
createSuperdocVueApp({ disablePiniaDevtools: true });
92-
const hook = globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__;
93-
expect(hook.emit('other-event', { id: 'other', app: {} })).toBe('emitted');
94-
expect(emitSpy).toHaveBeenCalled();
95-
});
96-
97-
it('returns app + stores from factory', async () => {
49+
it('returns the Vue app with all stores', async () => {
9850
const { app } = setupAppMocks();
9951
const { createSuperdocVueApp } = await import('./create-app.js');
10052
const result = createSuperdocVueApp({ disablePiniaDevtools: false });
@@ -105,36 +57,61 @@ describe('createSuperdocVueApp edge cases', () => {
10557
expect(result.highContrastModeStore).toBeDefined();
10658
});
10759

108-
it('replacing hook at runtime picks up new hook instance', async () => {
60+
// Outcome-focused: when suppressed, the Pinia devtools plugin for this app
61+
// is never surfaced via the hook or queue. The test does not pin the
62+
// specific suppression mechanism (hook-emit patch vs. queue-push patch),
63+
// so it survives refactors of the strategy.
64+
it('keeps the Pinia devtools plugin hidden from consumers when suppressed', async () => {
65+
const emitSpy = vi.fn(() => 'emitted');
66+
globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__ = { emit: emitSpy };
10967
const { app } = setupAppMocks();
11068
const { createSuperdocVueApp } = await import('./create-app.js');
11169
createSuperdocVueApp({ disablePiniaDevtools: true });
11270

113-
const firstEmit = vi.fn(() => 'a');
114-
globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__ = { emit: firstEmit };
115-
let hook = globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__;
71+
const hook = globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__;
72+
// Hook emit for this app is swallowed
11673
expect(hook.emit('devtools-plugin:setup', { id: 'dev.esm.pinia', app }, vi.fn())).toBeUndefined();
74+
expect(emitSpy).not.toHaveBeenCalled();
11775

118-
const secondEmit = vi.fn(() => 'b');
119-
globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__ = { emit: secondEmit };
120-
hook = globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__;
121-
expect(hook.emit('devtools-plugin:setup', { id: 'dev.esm.pinia', app }, vi.fn())).toBeUndefined();
76+
// Queue pushes for this app are dropped
77+
const queue = globalThis.__VUE_DEVTOOLS_PLUGINS__;
78+
queue.push([{ id: 'dev.esm.pinia', app }, vi.fn()]);
79+
expect(queue).toHaveLength(0);
80+
81+
// Unrelated apps are not suppressed
82+
const otherApp = {};
83+
expect(hook.emit('devtools-plugin:setup', { id: 'dev.esm.pinia', app: otherApp }, vi.fn())).toBe('emitted');
84+
queue.push([{ id: 'dev.esm.pinia', app: otherApp }, vi.fn()]);
85+
expect(queue).toHaveLength(1);
86+
87+
// Suppression lifts after unmount
88+
app.unmount();
89+
expect(hook.emit('devtools-plugin:setup', { id: 'dev.esm.pinia', app }, vi.fn())).toBe('emitted');
12290
});
12391

124-
it('replacement queue also intercepts pinia setup for suppressed app', async () => {
92+
it('does not suppress anything when disablePiniaDevtools is false', async () => {
93+
const emitSpy = vi.fn(() => 'emitted');
94+
globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__ = { emit: emitSpy };
12595
const { app } = setupAppMocks();
12696
const { createSuperdocVueApp } = await import('./create-app.js');
127-
createSuperdocVueApp({ disablePiniaDevtools: true });
97+
createSuperdocVueApp({ disablePiniaDevtools: false });
12898

129-
const replacement1 = [];
130-
globalThis.__VUE_DEVTOOLS_PLUGINS__ = replacement1;
131-
replacement1.push([{ id: 'dev.esm.pinia', app }, vi.fn()]);
132-
expect(replacement1).toHaveLength(0);
99+
const hook = globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__;
100+
expect(hook.emit('devtools-plugin:setup', { id: 'dev.esm.pinia', app }, vi.fn())).toBe('emitted');
101+
expect(emitSpy).toHaveBeenCalledTimes(1);
102+
});
133103

134-
// Replace again — new queue should also get patched
135-
const replacement2 = [];
136-
globalThis.__VUE_DEVTOOLS_PLUGINS__ = replacement2;
137-
replacement2.push([{ id: 'dev.esm.pinia', app }, vi.fn()]);
138-
expect(replacement2).toHaveLength(0);
104+
it('cleans up suppression state when app initialization throws', async () => {
105+
const emitSpy = vi.fn(() => 'emitted');
106+
globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__ = { emit: emitSpy };
107+
const { app } = setupAppMocks();
108+
app.use.mockImplementation(() => {
109+
throw new Error('init failed');
110+
});
111+
const { createSuperdocVueApp } = await import('./create-app.js');
112+
expect(() => createSuperdocVueApp({ disablePiniaDevtools: true })).toThrow('init failed');
113+
114+
const hook = globalThis.__VUE_DEVTOOLS_GLOBAL_HOOK__;
115+
expect(hook.emit('devtools-plugin:setup', { id: 'dev.esm.pinia', app }, vi.fn())).toBe('emitted');
139116
});
140117
});

packages/superdoc/src/core/whiteboard/WhiteboardPage-deep.test.js

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,14 @@ const makeLayer = () => {
5050

5151
const makeStage = () => {
5252
const stageListeners = new Map();
53+
const layers = [];
5354
const stage = makeNode();
5455
stage.on = vi.fn((events, fn) => events.split(' ').forEach((e) => stageListeners.set(e, fn)));
55-
stage.add = vi.fn();
56+
stage.add = vi.fn((l) => layers.push(l));
5657
stage.size = vi.fn();
5758
stage.getPointerPosition = vi.fn(() => ({ x: 10, y: 10 }));
5859
stage._listeners = stageListeners;
60+
stage._layers = layers;
5961
return stage;
6062
};
6163

@@ -86,6 +88,9 @@ const makeRenderer = () => {
8688
return { Stage, Layer, Line, Text, Image, Transformer };
8789
};
8890

91+
const mountedPages = [];
92+
const mountedContainers = [];
93+
8994
const mountPage = (opts = {}) => {
9095
const renderer = opts.Renderer ?? makeRenderer();
9196
const page = new WhiteboardPage({
@@ -99,6 +104,8 @@ const mountPage = (opts = {}) => {
99104
const container = document.createElement('div');
100105
document.body.appendChild(container);
101106
page.mount(container);
107+
mountedPages.push(page);
108+
mountedContainers.push(container);
102109
return { page, renderer, container };
103110
};
104111

@@ -123,6 +130,10 @@ describe('WhiteboardPage: image async + transform paths', () => {
123130
});
124131

125132
afterEach(() => {
133+
// Destroy mounted pages so the window keydown listeners they installed
134+
// are removed. Leaking them across tests causes cross-test coupling.
135+
while (mountedPages.length) mountedPages.pop().destroy();
136+
while (mountedContainers.length) mountedContainers.pop().remove();
126137
window.Image = originalImage;
127138
});
128139

@@ -133,7 +144,7 @@ describe('WhiteboardPage: image async + transform paths', () => {
133144
});
134145
page.render();
135146
// Flush microtasks so onload fires
136-
await new Promise((r) => setTimeout(r, 0));
147+
await Promise.resolve();
137148
expect(renderer.Image).toHaveBeenCalled();
138149
});
139150

@@ -151,7 +162,7 @@ describe('WhiteboardPage: image async + transform paths', () => {
151162
const { page, renderer } = mountPage();
152163
page.applyData({ images: [{ id: 'i1', xN: 0, yN: 0, src: '/x.png' }] });
153164
page.render();
154-
await new Promise((r) => setTimeout(r, 0));
165+
await Promise.resolve();
155166
expect(renderer.Image).not.toHaveBeenCalled();
156167
});
157168

@@ -161,15 +172,15 @@ describe('WhiteboardPage: image async + transform paths', () => {
161172
images: [{ id: 'i1', xN: 0.1, yN: 0.1, src: '/x.png', widthN: 0.5, heightN: 0.5 }],
162173
});
163174
page.render();
164-
await new Promise((r) => setTimeout(r, 0));
175+
await Promise.resolve();
165176
const callsBefore = renderer.Image.mock.calls.length;
166177

167178
// Re-render with same id — should update existing, not recreate
168179
page.applyData({
169180
images: [{ id: 'i1', xN: 0.2, yN: 0.2, src: '/x.png', widthN: 0.5, heightN: 0.5 }],
170181
});
171182
page.render();
172-
await new Promise((r) => setTimeout(r, 0));
183+
await Promise.resolve();
173184
expect(renderer.Image.mock.calls.length).toBe(callsBefore);
174185
});
175186

@@ -182,23 +193,25 @@ describe('WhiteboardPage: image async + transform paths', () => {
182193
],
183194
});
184195
page.render();
185-
await new Promise((r) => setTimeout(r, 0));
196+
await Promise.resolve();
197+
const createdNodes = renderer.Image.mock.results.map((r) => r.value);
198+
const i1Node = createdNodes.find((n) => n._whiteboardId === 'i1');
199+
expect(i1Node).toBeDefined();
186200

187-
// Now remove i1
201+
// Drop i1 from the model and re-render
188202
page.applyData({ images: [{ id: 'i2', xN: 0, yN: 0, src: '/b.png' }] });
189203
page.render();
190-
await new Promise((r) => setTimeout(r, 0));
191-
// i1 node should have been destroyed during renderImages cleanup; no assertion on count
192-
// (Konva mock doesn't expose destroy counts per node id here) — just ensure render does not throw.
193-
expect(true).toBe(true);
204+
await Promise.resolve();
205+
expect(i1Node.destroy).toHaveBeenCalled();
206+
expect(page.images.map((i) => i.id)).toEqual(['i2']);
194207
});
195208

196209
it('image node transformend recomputes normalized width/height/position', async () => {
197210
const onChange = vi.fn();
198211
const { page, renderer } = mountPage({ onChange });
199212
page.applyData({ images: [{ id: 'i1', xN: 0, yN: 0, src: '/a.png' }] });
200213
page.render();
201-
await new Promise((r) => setTimeout(r, 0));
214+
await Promise.resolve();
202215
const imageNode = renderer.Image.mock.results.at(-1).value;
203216
imageNode.scaleX(2);
204217
imageNode.scaleY(2);
@@ -218,7 +231,7 @@ describe('WhiteboardPage: image async + transform paths', () => {
218231
const { page, renderer } = mountPage({ onChange });
219232
page.applyData({ images: [{ id: 'i1', xN: 0, yN: 0, src: '/a.png' }] });
220233
page.render();
221-
await new Promise((r) => setTimeout(r, 0));
234+
await Promise.resolve();
222235
const imageNode = renderer.Image.mock.results.at(-1).value;
223236
imageNode.x(50);
224237
imageNode.y(50);
@@ -227,15 +240,19 @@ describe('WhiteboardPage: image async + transform paths', () => {
227240
expect(page.images[0].xN).toBeCloseTo(0.5);
228241
});
229242

230-
it('clicking an image node selects it', async () => {
243+
it('clicking an image node in select mode attaches a selection overlay', async () => {
231244
const { page, renderer } = mountPage();
232245
page.setTool('select');
233246
page.applyData({ images: [{ id: 'i1', xN: 0, yN: 0, src: '/a.png' }] });
234247
page.render();
235-
await new Promise((r) => setTimeout(r, 0));
248+
await Promise.resolve();
236249
const imageNode = renderer.Image.mock.results.at(-1).value;
250+
const stage = renderer.Stage.mock.results[0].value;
251+
const layer = stage._layers[0];
252+
const childCountBefore = layer._children.length;
237253
imageNode._listeners.get('click')({});
238-
expect(renderer.Transformer).toHaveBeenCalled();
254+
// Selection attaches a Transformer node to the visible layer
255+
expect(layer._children.length).toBe(childCountBefore + 1);
239256
});
240257

241258
it('text node transform keeps height fixed (text resize only horizontally)', () => {
@@ -326,7 +343,7 @@ describe('WhiteboardPage: image async + transform paths', () => {
326343
page.setTool('select');
327344
page.applyData({ images: [{ id: 'i1', xN: 0, yN: 0, src: '/a.png' }] });
328345
page.render();
329-
await new Promise((r) => setTimeout(r, 0));
346+
await Promise.resolve();
330347
const imageNode = renderer.Image.mock.results.at(-1).value;
331348
imageNode._whiteboardId = 'i1';
332349
imageNode._listeners.get('click')({});

packages/superdoc/src/core/whiteboard/WhiteboardPage.test.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -659,14 +659,21 @@ describe('WhiteboardPage', () => {
659659
});
660660

661661
it('ignores key presses from input/textarea target', () => {
662-
const page = makePage();
662+
const onChange = vi.fn();
663+
const page = makePage({ onChange });
664+
page.setSize({ width: 200, height: 300 });
665+
page.applyData({
666+
text: [{ id: 't1', xN: 0.1, yN: 0.2, content: 'hi', fontSizeN: 0.05 }],
667+
});
663668
mountWithSize(page);
664669
const target = document.createElement('input');
665670
document.body.appendChild(target);
666671
const e = new KeyboardEvent('keydown', { key: 'Delete' });
667672
Object.defineProperty(e, 'target', { value: target });
668673
window.dispatchEvent(e);
669-
expect(true).toBe(true); // no throw
674+
// The text item was NOT deleted because the keydown came from an input element
675+
expect(page.text.find((t) => t.id === 't1')).toBeDefined();
676+
expect(onChange).not.toHaveBeenCalled();
670677
target.remove();
671678
});
672679
});

0 commit comments

Comments
 (0)