-
Notifications
You must be signed in to change notification settings - Fork 4
Add basic integration tests for .deepnote file handling (GRN-4766) #99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ef1f07c
441b333
9de407e
579de3b
e898f8e
2b8ef59
925292a
bfb141d
4f48157
f5401c3
6679ce7
0fbedc8
7c637c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,6 +143,44 @@ jobs: | |
| - name: Check licenses | ||
| run: npm run check-licenses | ||
|
|
||
| integration-tests: | ||
| name: Integration Tests | ||
| runs-on: ubicloud | ||
| timeout-minutes: 30 | ||
| permissions: | ||
| id-token: write | ||
| contents: read | ||
| packages: read | ||
|
Comment on lines
+150
to
+153
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DevinAI What do you need the permissions for? I thought it's defined at the top-level |
||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5 | ||
| with: | ||
| cache: 'npm' | ||
| node-version-file: '.nvmrc' | ||
| registry-url: 'https://npm.pkg.github.com' | ||
| scope: '@deepnote' | ||
|
|
||
| - name: Setup Python | ||
| uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 | ||
| with: | ||
| python-version: '3.12' | ||
|
|
||
| - name: Install dependencies | ||
| run: npm ci --prefer-offline --no-audit | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Compile TypeScript | ||
| run: npm run compile | ||
|
|
||
| - name: Run integration tests | ||
| run: xvfb-run -a -s "-screen 0 1024x768x24" npm run test:integration | ||
| env: | ||
| VSC_JUPYTER_CI_TEST_GREP: 'Deepnote Integration Tests' | ||
FilipPyrek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| check_licenses: | ||
| name: Check Licenses | ||
| runs-on: ubuntu-latest | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ import { | |
| } from '@jupyterlab/services/lib/kernel/messages'; | ||
| import { Deferred, createDeferred } from '../../platform/common/utils/async'; | ||
| import { NotebookCellOutput, NotebookCellOutputItem } from 'vscode'; | ||
| import { JVSC_EXTENSION_ID_FOR_TESTS } from '../../test/constants'; | ||
| import { JVSC_EXTENSION_ID } from '../../platform/common/constants'; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DevinAI why did you change the import here? I think you can use |
||
|
|
||
| suite('Code Execution', () => { | ||
| let disposables: IDisposable[] = []; | ||
|
|
@@ -318,7 +318,7 @@ suite('Code Execution', () => { | |
| }); | ||
| test('Cancelling pending Internal Jupyter execution code should not interrupt the kernel', async () => { | ||
| const code = `print('Hello World')`; | ||
| const execution = createExecution(code, JVSC_EXTENSION_ID_FOR_TESTS); | ||
| const execution = createExecution(code, JVSC_EXTENSION_ID); | ||
|
|
||
| const outputs: NotebookCellOutput[] = []; | ||
| disposables.push(execution.onDidEmitOutput((output) => outputs.push(output))); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,109 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { assert } from 'chai'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Uri, workspace } from 'vscode'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IDisposable } from '../../../platform/common/types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { captureScreenShot, IExtensionTestApi } from '../../common.node'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { EXTENSION_ROOT_DIR_FOR_TESTS, initialize } from '../../initialize.node'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { closeNotebooksAndCleanUpAfterTests } from './helper.node'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { logger } from '../../../platform/logging'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IDeepnoteNotebookManager } from '../../../notebooks/types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* eslint-disable @typescript-eslint/no-explicit-any, no-invalid-this */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| suite('Deepnote Integration Tests @kernelCore', function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Misleading suite tag. Suite is tagged -suite('Deepnote Integration Tests @kernelCore', function () {
+suite('Deepnote Integration Tests', function () {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let api: IExtensionTestApi; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const disposables: IDisposable[] = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const deepnoteFilePath = Uri.joinPath( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Uri.file(EXTENSION_ROOT_DIR_FOR_TESTS), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'src', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'test', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'datascience', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'notebook', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'test.deepnote' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.timeout(240_000); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+13
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Add blank line after declarations. Per coding guidelines, add a blank line between the declaration group and const deepnoteFilePath = Uri.joinPath(
Uri.file(EXTENSION_ROOT_DIR_FOR_TESTS),
'src',
'test',
'datascience',
'notebook',
'test.deepnote'
);
+
this.timeout(240_000);As per coding guidelines. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| suiteSetup(async function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info('Suite Setup VS Code Notebook - Deepnote Integration'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.timeout(240_000); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| api = await initialize(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info('Suite Setup (completed)'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.error('Suite Setup (failed) - Deepnote Integration', e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await captureScreenShot('deepnote-suite'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw e; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setup(function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info(`Start Test (completed) ${this.currentTest?.title}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| teardown(async function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (this.currentTest?.isFailed()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await captureScreenShot(this); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info(`Ended Test (completed) ${this.currentTest?.title}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| suiteTeardown(() => closeNotebooksAndCleanUpAfterTests(disposables)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test('Load .deepnote file and verify structure', async function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Test: Load .deepnote file - starting'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const notebookManager = api.serviceContainer.get<IDeepnoteNotebookManager>(IDeepnoteNotebookManager); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.isOk(notebookManager, 'Notebook manager should be available'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| notebookManager.selectNotebookForProject('test-project-id', 'main-notebook-id'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const nbDocument = await workspace.openNotebookDocument(deepnoteFilePath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug(`Opened notebook with type: ${nbDocument.notebookType}, cells: ${nbDocument.cellCount}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.equal(nbDocument.notebookType, 'deepnote', 'Notebook type should be deepnote'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.equal(nbDocument.cellCount, 3, 'Notebook should have 3 cells'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.equal(nbDocument.metadata?.deepnoteProjectId, 'test-project-id', 'Project ID should match'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.equal(nbDocument.metadata?.deepnoteNotebookId, 'main-notebook-id', 'Notebook ID should match'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Test: Load .deepnote file - completed'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test('Verify notebook cells are correctly deserialized', async function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Test: Verify cells - starting'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const notebookManager = api.serviceContainer.get<IDeepnoteNotebookManager>(IDeepnoteNotebookManager); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| notebookManager.selectNotebookForProject('test-project-id', 'main-notebook-id'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const nbDocument = await workspace.openNotebookDocument(deepnoteFilePath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.isAtLeast(nbDocument.cellCount, 3, 'Notebook should have at least 3 cells'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let hasCodeCell = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let hasMarkdownCell = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (let i = 0; i < nbDocument.cellCount; i++) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cell = nbDocument.cellAt(i); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (cell.kind === 1) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hasCodeCell = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (cell.kind === 2) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hasMarkdownCell = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+85
to
+93
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Use NotebookCellKind enum for clarity. Magic numbers Import NotebookCellKind: -import { Uri, workspace } from 'vscode';
+import { NotebookCellKind, Uri, workspace } from 'vscode';Then use the enum: for (let i = 0; i < nbDocument.cellCount; i++) {
const cell = nbDocument.cellAt(i);
- if (cell.kind === 1) {
+ if (cell.kind === NotebookCellKind.Code) {
hasCodeCell = true;
}
- if (cell.kind === 2) {
+ if (cell.kind === NotebookCellKind.Markup) {
hasMarkdownCell = true;
}
}📝 Committable suggestion
Suggested change
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.isTrue(hasCodeCell, 'Notebook should have at least one code cell'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.isTrue(hasMarkdownCell, 'Notebook should have at least one markdown cell'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Test: Verify cells - completed'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test('Extension services are available', async function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Test: Extension services are available - starting'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const notebookManager = api.serviceContainer.get<IDeepnoteNotebookManager>(IDeepnoteNotebookManager); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.isOk(notebookManager, 'Notebook manager should be available'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug('Test: Extension services are available - completed'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| metadata: | ||
| createdAt: '2025-01-01T00:00:00.000Z' | ||
| modifiedAt: '2025-01-01T00:00:00.000Z' | ||
| project: | ||
| id: test-project-id | ||
| name: Test Project | ||
| initNotebookId: init-notebook-id | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainInit notebook execution not tested. PR objectives require verifying that the init notebook (referenced by Verify whether init notebook auto-execution is implemented and add a test that:
🏁 Script executed: #!/bin/bash
# Search for init notebook execution logic in the codebase
rg -nP --type=ts 'initNotebook' -C3Length of output: 10708 Length of output: 0 🏁 Script executed: #!/bin/bash
# Check if test.deepnote file exists and what it contains
find . -name "test.deepnote" -type f -exec cat {} \;Length of output: 990 🏁 Script executed: #!/bin/bash
# Search for any test references to deepnoteKernelAutoSelector or deepnoteInitNotebookRunner
fd -e "test.ts" -e "test.js" -e "spec.ts" | xargs rg -l "deepnoteKernelAutoSelector|deepnoteInitNotebookRunner" 2>/dev/nullLength of output: 0 Add tests for init notebook auto-execution on kernel start. Init notebook execution is implemented in
🤖 Prompt for AI Agents |
||
| notebooks: | ||
| - id: init-notebook-id | ||
| name: Init Notebook | ||
| blocks: | ||
| - id: init-block-1 | ||
| type: code | ||
| content: | | ||
| # This is the init notebook | ||
| import sys | ||
| print("Init notebook executed") | ||
| sortingKey: '0001' | ||
| - id: main-notebook-id | ||
| name: Main Notebook | ||
| blocks: | ||
| - id: block-1 | ||
| type: code | ||
| content: | | ||
| print("Hello World") | ||
| sortingKey: '0001' | ||
| - id: block-2 | ||
| type: code | ||
| content: | | ||
| x = 42 | ||
| print(f"The answer is {x}") | ||
| sortingKey: '0002' | ||
| - id: block-3 | ||
| type: markdown | ||
| content: | | ||
| # Test Markdown | ||
| This is a test markdown block. | ||
| sortingKey: '0003' | ||
| version: '1.0' | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FilipPyrek pls open a pr doing just this for the sake of integrations tests already in main and assign me as a reviewer.