Skip to content

Commit 03da012

Browse files
committed
refactor: improve test
1 parent c2223c9 commit 03da012

File tree

1 file changed

+238
-102
lines changed

1 file changed

+238
-102
lines changed
Lines changed: 238 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1,136 +1,272 @@
11
// Copyright (c) Deepnote
22
// Distributed under the terms of the Modified BSD License.
33

4-
import type { NotebookPanel } from '@jupyterlab/notebook';
4+
import type { INotebookModel, NotebookPanel } from '@jupyterlab/notebook';
55
import { framePromise } from '@jupyterlab/testing';
6+
import type { PartialJSONObject } from '@lumino/coreutils';
67
import { Widget } from '@lumino/widgets';
7-
import { simulate } from 'simulate-event';
8-
import { NotebookPicker } from '../../src/components/NotebookPicker';
9-
10-
// Mock types for testing
11-
interface MockNotebookModel {
12-
fromJSON: jest.Mock;
13-
cells: unknown[];
14-
dirty: boolean;
15-
}
16-
17-
interface MockNotebookPanel {
18-
context: {
19-
ready: Promise<void>;
20-
model: {
21-
getMetadata: jest.Mock;
22-
};
23-
};
24-
model: MockNotebookModel | null;
25-
}
26-
27-
// Type for widget with overridden protected method
28-
type WidgetWithMockOnAfterAttach = NotebookPicker & {
29-
onAfterAttach: jest.Mock;
30-
};
8+
import { NotebookPicker } from '../components/NotebookPicker';
319

3210
describe('NotebookPicker', () => {
33-
let panel: MockNotebookPanel;
34-
let model: MockNotebookModel;
11+
let widget: NotebookPicker;
12+
let mockNotebookModel: Partial<INotebookModel>;
13+
let deepnoteMetadata: PartialJSONObject;
14+
let consoleErrorSpy: jest.SpyInstance | null = null;
15+
16+
const createMockPanel = (metadata: PartialJSONObject): NotebookPanel => {
17+
deepnoteMetadata = metadata;
3518

36-
beforeEach(async () => {
37-
// Mock model + metadata
38-
model = {
19+
mockNotebookModel = {
3920
fromJSON: jest.fn(),
40-
get cells() {
41-
return [];
42-
},
43-
dirty: true
21+
cells: {
22+
length: 0
23+
} as never,
24+
dirty: false,
25+
getMetadata: jest.fn((key: string) => {
26+
if (key === 'deepnote') {
27+
return deepnoteMetadata;
28+
}
29+
return undefined;
30+
})
4431
};
4532

46-
panel = {
33+
return {
4734
context: {
4835
ready: Promise.resolve(),
49-
model: {
50-
getMetadata: jest.fn().mockReturnValue({
51-
notebooks: {
52-
nb1: { id: 'nb1', name: 'nb1', cells: [{ source: 'code' }] },
53-
nb2: { id: 'nb2', name: 'nb2', cells: [] }
54-
},
55-
notebook_names: ['nb1', 'nb2']
56-
})
57-
}
36+
model: mockNotebookModel as INotebookModel
5837
},
59-
model
60-
};
38+
model: mockNotebookModel as INotebookModel
39+
} as unknown as NotebookPanel;
40+
};
6141

62-
// Attach to DOM
63-
const widget = new NotebookPicker(
64-
panel as unknown as NotebookPanel
65-
) as WidgetWithMockOnAfterAttach;
66-
// Override onAfterAttach to avoid errors from this.parent being null
67-
widget.onAfterAttach = jest.fn();
42+
const attachWidget = async (panel: NotebookPanel): Promise<void> => {
43+
widget = new NotebookPicker(panel);
6844
Widget.attach(widget, document.body);
45+
// Wait for widget to attach and render
6946
await framePromise();
70-
});
47+
// Wait for constructor's async initialization to complete
48+
await new Promise(resolve => setTimeout(resolve, 0));
49+
await framePromise();
50+
};
7151

7252
afterEach(() => {
73-
document.body.innerHTML = '';
74-
jest.restoreAllMocks();
53+
if (consoleErrorSpy) {
54+
consoleErrorSpy.mockRestore();
55+
consoleErrorSpy = null;
56+
}
57+
if (widget && !widget.isDisposed) {
58+
widget.dispose();
59+
}
60+
// Clean up DOM
61+
const attached = document.querySelectorAll('.jp-ReactWidget');
62+
attached.forEach(node => {
63+
node.remove();
64+
});
7565
});
7666

77-
it('should render a select element', async () => {
78-
await framePromise(); // wait for rendering
79-
const select = document.querySelector('select') as HTMLSelectElement;
80-
expect(select).not.toBeNull();
81-
expect(select.options.length).toBe(2);
82-
expect(select.options[0]?.value).toBe('nb1');
67+
describe('rendering', () => {
68+
it('should render a select element with notebooks', async () => {
69+
const metadata = {
70+
notebooks: {
71+
'Notebook 1': { id: 'nb1', name: 'Notebook 1', cells: [] },
72+
'Notebook 2': { id: 'nb2', name: 'Notebook 2', cells: [] }
73+
}
74+
};
75+
76+
const panel = createMockPanel(metadata);
77+
await attachWidget(panel);
78+
79+
const select = widget.node.querySelector('select');
80+
expect(select).not.toBeNull();
81+
expect(select?.options.length).toBe(2);
82+
expect(select?.options[0]?.value).toBe('Notebook 1');
83+
expect(select?.options[1]?.value).toBe('Notebook 2');
84+
});
85+
86+
it('should render a placeholder when no notebooks are available', async () => {
87+
const metadata = {
88+
notebooks: {}
89+
};
90+
91+
const panel = createMockPanel(metadata);
92+
await attachWidget(panel);
93+
94+
const select = widget.node.querySelector('select');
95+
expect(select).not.toBeNull();
96+
expect(select?.options.length).toBe(1);
97+
expect(select?.options[0]?.value).toBe('-');
98+
});
99+
100+
it('should handle invalid metadata gracefully', async () => {
101+
const metadata = {
102+
notebooks: null
103+
} as PartialJSONObject;
104+
105+
const panel = createMockPanel(metadata);
106+
await attachWidget(panel);
107+
108+
const select = widget.node.querySelector('select');
109+
expect(select).not.toBeNull();
110+
expect(select?.options.length).toBe(1);
111+
expect(select?.options[0]?.value).toBe('-');
112+
});
83113
});
84114

85-
it('should call fromJSON when selecting a notebook', async () => {
86-
const select = document.querySelector('select') as HTMLSelectElement;
87-
simulate(select, 'change', { target: { value: 'nb2' } });
88-
await framePromise();
89-
expect(model.fromJSON).toHaveBeenCalledWith(
90-
expect.objectContaining({
91-
cells: expect.any(Array),
92-
metadata: expect.objectContaining({
93-
deepnote: expect.objectContaining({
94-
notebooks: expect.any(Object)
95-
})
115+
describe('notebook selection', () => {
116+
let panel: NotebookPanel;
117+
118+
beforeEach(async () => {
119+
const metadata = {
120+
notebooks: {
121+
'Notebook 1': {
122+
id: 'nb1',
123+
name: 'Notebook 1',
124+
cells: [{ cell_type: 'code', source: 'print(1)' }]
125+
},
126+
'Notebook 2': {
127+
id: 'nb2',
128+
name: 'Notebook 2',
129+
cells: [{ cell_type: 'code', source: 'print(2)' }]
130+
}
131+
}
132+
};
133+
134+
panel = createMockPanel(metadata);
135+
await attachWidget(panel);
136+
});
137+
138+
it('should call fromJSON when selecting a different notebook', async () => {
139+
const select = widget.node.querySelector('select') as HTMLSelectElement;
140+
expect(select).not.toBeNull();
141+
142+
select.value = 'Notebook 2';
143+
select.dispatchEvent(new Event('change', { bubbles: true }));
144+
await framePromise();
145+
146+
expect(mockNotebookModel.fromJSON).toHaveBeenCalledTimes(1);
147+
expect(mockNotebookModel.fromJSON).toHaveBeenCalledWith(
148+
expect.objectContaining({
149+
cells: [{ cell_type: 'code', source: 'print(2)' }],
150+
metadata: {
151+
deepnote: {
152+
notebooks: expect.objectContaining({
153+
'Notebook 1': expect.any(Object),
154+
'Notebook 2': expect.any(Object)
155+
})
156+
}
157+
},
158+
nbformat: 4,
159+
nbformat_minor: 0
96160
})
97-
})
98-
);
99-
});
161+
);
162+
});
100163

101-
it('should not call fromJSON if selected notebook is invalid', async () => {
102-
const getMetadata = panel.context.model.getMetadata as jest.Mock;
103-
getMetadata.mockReturnValue({ notebooks: {}, notebook_names: [] });
164+
it('should set model.dirty to false after switching notebooks', async () => {
165+
const select = widget.node.querySelector('select') as HTMLSelectElement;
166+
select.value = 'Notebook 2';
167+
select.dispatchEvent(new Event('change', { bubbles: true }));
168+
await framePromise();
104169

105-
const select = document.querySelector('select') as HTMLSelectElement;
106-
simulate(select, 'change', { target: { value: 'nonexistent' } });
107-
await framePromise();
108-
expect(model.fromJSON).not.toHaveBeenCalled();
170+
expect(mockNotebookModel.dirty).toBe(false);
171+
});
172+
173+
it('should not call fromJSON when selecting a non-existent notebook', async () => {
174+
const select = widget.node.querySelector('select') as HTMLSelectElement;
175+
select.value = 'NonExistent';
176+
select.dispatchEvent(new Event('change', { bubbles: true }));
177+
await framePromise();
178+
179+
expect(mockNotebookModel.fromJSON).not.toHaveBeenCalled();
180+
});
181+
182+
it('should not call fromJSON when panel.model is null', async () => {
183+
widget.dispose();
184+
185+
// Create panel with null model
186+
const nullModelPanel = {
187+
context: {
188+
ready: Promise.resolve(),
189+
model: {
190+
getMetadata: jest.fn(() => deepnoteMetadata)
191+
}
192+
},
193+
model: null
194+
} as unknown as NotebookPanel;
195+
196+
await attachWidget(nullModelPanel);
197+
198+
const select = widget.node.querySelector('select') as HTMLSelectElement;
199+
select.value = 'Notebook 2';
200+
select.dispatchEvent(new Event('change', { bubbles: true }));
201+
await framePromise();
202+
203+
expect(mockNotebookModel.fromJSON).not.toHaveBeenCalled();
204+
});
109205
});
110206

111-
it('should update UI after selection', async () => {
112-
const select = document.querySelector('select') as HTMLSelectElement;
113-
select.value = 'nb2';
114-
simulate(select, 'change');
115-
await framePromise();
116-
expect(select.value).toBe('nb2');
207+
describe('initialization', () => {
208+
it('should select first notebook by default when notebooks exist', async () => {
209+
const metadata = {
210+
notebooks: {
211+
First: { id: 'nb1', name: 'First', cells: [] },
212+
Second: { id: 'nb2', name: 'Second', cells: [] }
213+
}
214+
};
215+
216+
const panel = createMockPanel(metadata);
217+
await attachWidget(panel);
218+
219+
const select = widget.node.querySelector('select') as HTMLSelectElement;
220+
expect(select.value).toBe('First');
221+
});
222+
223+
it('should handle initialization errors gracefully', async () => {
224+
consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
225+
226+
const failingPanel = {
227+
context: {
228+
ready: Promise.reject(new Error('Initialization failed')),
229+
model: {
230+
getMetadata: jest.fn(() => ({}))
231+
}
232+
},
233+
model: mockNotebookModel as INotebookModel
234+
} as unknown as NotebookPanel;
235+
236+
await attachWidget(failingPanel);
237+
await framePromise();
238+
239+
expect(consoleErrorSpy).toHaveBeenCalledWith(
240+
'Failed to initialize NotebookPicker:',
241+
expect.any(Error)
242+
);
243+
});
117244
});
118245

119-
it('should handle empty metadata gracefully', async () => {
120-
const getMetadata = panel.context.model.getMetadata as jest.Mock;
121-
getMetadata.mockReturnValue({ notebooks: {}, notebook_names: [] });
246+
describe('metadata validation', () => {
247+
it('should handle invalid metadata when changing notebooks', async () => {
248+
consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
122249

123-
document.body.innerHTML = '';
124-
const widget = new NotebookPicker(
125-
panel as unknown as NotebookPanel
126-
) as WidgetWithMockOnAfterAttach;
127-
// Override onAfterAttach to avoid errors from this.parent being null
128-
widget.onAfterAttach = jest.fn();
129-
Widget.attach(widget, document.body);
130-
await framePromise();
250+
const metadata = {
251+
notebooks: {
252+
'Notebook 1': { id: 'nb1', name: 'Notebook 1', cells: [] }
253+
}
254+
};
255+
256+
const panel = createMockPanel(metadata);
257+
await attachWidget(panel);
258+
259+
// Change the metadata to invalid format
260+
deepnoteMetadata = { invalid: 'metadata' } as PartialJSONObject;
261+
262+
const select = widget.node.querySelector('select') as HTMLSelectElement;
263+
select.value = 'Notebook 1';
264+
select.dispatchEvent(new Event('change', { bubbles: true }));
265+
await framePromise();
131266

132-
const select = document.querySelector('select') as HTMLSelectElement;
133-
expect(select.options.length).toBeGreaterThanOrEqual(1);
134-
expect(select.options[0]?.value).toBe('-');
267+
expect(consoleErrorSpy).toHaveBeenCalled();
268+
expect(consoleErrorSpy.mock.calls[0]?.[0]).toMatch(/invalid.*metadata/i);
269+
expect(mockNotebookModel.fromJSON).not.toHaveBeenCalled();
270+
});
135271
});
136272
});

0 commit comments

Comments
 (0)