Add integration tests for .deepnote file handling (GRN-4766) #103
Conversation
- Create test fixture test.deepnote with init and main notebooks - Add deepnote.vscode.test.ts with tests for: - Loading/opening .deepnote files - Kernel startup for .deepnote files - Code block execution with output validation - Multiple code block execution - Cell execution order verification - Add integration-tests CI job for running Deepnote tests on ubicloud - Configure CI to use Python 3.12 and xvfb for headless testing Co-Authored-By: Filip Pyrek <[email protected]>
FilipPyrek
requested review from
Artmann,
jamesbhobbs and
saltenasl
as code owners
GRN-4766 Add basic integration tests for a .deepnote file
Just run a block after kernel is instantiated. We need to be able to run some integrations tests with the Your task is
Make sure that the newly created integration tests are passing in GitHub actions. We can add more tests later. Here is a sample .deepnote file |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
|
📝 WalkthroughWalkthroughAdds integration tests for Deepnote notebook execution. A new CI job runs the test suite on a dedicated runner with 30-minute timeout. The test file loads a .deepnote fixture, initializes a Jupyter kernel, executes notebook cells, and validates outputs and execution order. Includes setup/teardown with screenshot capture on failures. Sequence DiagramsequenceDiagram
participant Test
participant TestEnv as Test Environment
participant Kernel
participant Notebook
Test->>TestEnv: Initialize test environment
TestEnv->>TestEnv: Start Jupyter server
TestEnv->>Notebook: Load test.deepnote fixture
Test->>Kernel: Create kernel instance
Kernel->>Kernel: Execute init notebook
Kernel-->>Test: Kernel ready
Test->>Notebook: Execute code cell 1
Notebook->>Kernel: Run code
Kernel-->>Notebook: Output result
Notebook-->>Test: Verify output
Test->>Notebook: Execute code cell 2
Notebook->>Kernel: Run code
Kernel-->>Notebook: Output result
Notebook-->>Test: Verify output & execution order
Test->>TestEnv: Cleanup & capture logs
TestEnv-->>Test: Test complete
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #103 +/- ##
=====================================
Coverage 71% 71%
=====================================
Files 527 527
Lines 39474 39474
Branches 4933 4933
=====================================
Hits 28277 28277
Misses 9563 9563
Partials 1634 1634 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.yml(1 hunks)src/test/datascience/notebook/deepnote.vscode.test.ts(1 hunks)src/test/datascience/notebook/test.deepnote(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/!(*.node|*.web).ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place shared cross-platform logic in common
.tsfiles (not suffixed with.nodeor.web)
Files:
src/test/datascience/notebook/deepnote.vscode.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports
Files:
src/test/datascience/notebook/deepnote.vscode.test.ts
**/!(*.unit).test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place integration tests in
*.test.tsfiles that are not*.unit.test.ts
Files:
src/test/datascience/notebook/deepnote.vscode.test.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined
thenproperty to avoid hanging tests
Files:
src/test/datascience/notebook/deepnote.vscode.test.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: deepnote/vscode-extension#0
File: .github/instructions/kernel-jupyter.instructions.md:0-0
Timestamp: 2025-09-03T12:56:39.535Z
Learning: Use real Jupyter servers for integration tests and mock jupyterlab/services for unit tests; cover both success and failure scenarios and verify cleanup
Learnt from: m1so
PR: deepnote/deepnote-toolkit#262
File: tests/unit/test_jinjasql.py:78-87
Timestamp: 2025-10-21T10:22:53.711Z
Learning: In the deepnote-toolkit repository, unit tests with 50,000 parameters in test_large_inclause (tests/unit/test_jinjasql.py) are acceptable; the full unit test suite runs in ~25 seconds.
🧬 Code graph analysis (1)
src/test/datascience/notebook/deepnote.vscode.test.ts (6)
src/platform/common/types.ts (1)
IDisposable(211-213)src/test/constants.node.ts (1)
EXTENSION_ROOT_DIR_FOR_TESTS(11-11)src/test/datascience/notebook/executionHelper.ts (2)
TestNotebookDocument(99-187)createKernelController(25-31)src/kernels/types.ts (2)
IKernel(439-446)INotebookKernelExecution(459-495)src/platform/logging/index.ts (1)
logger(35-48)src/test/datascience/notebook/helper.ts (1)
getDefaultKernelConnection(612-616)
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
148-148: label "ubicloud-standard-2" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build & Package Extension
- GitHub Check: Integration Tests - Deepnote
- GitHub Check: Build & Test
- GitHub Check: Lint & Format
🔇 Additional comments (2)
.github/workflows/ci.yml (1)
148-148: Ignore this review comment;ubicloud-standard-2is a valid runner label in this codebase.Per prior verification in this repository,
ubicloud-*labels are recognized and accepted. The label is already in use without issues. No actionlint configuration is needed.Likely an incorrect or invalid review comment.
src/test/datascience/notebook/deepnote.vscode.test.ts (1)
102-106: Remove the suggested test—it's incompatible with the test infrastructure.The suggested diff calls
notebook.insertCodeCell(), butnotebookis aTestNotebookDocumentmock that providesappendCodeCell(), notinsertCodeCell(). The helper functioninsertCodeCell()requires real VS Code editor context (window.activeNotebookEditor), which this test doesn't have. Additionally,init_variableis undefined in the codebase—the init notebook mechanism doesn't expose such a variable.If testing init notebook auto-execution is needed, use
notebook.appendCodeCell()(line 53 in this file shows the pattern) and verify expected state after kernel startup instead.Likely an incorrect or invalid review comment.
| - name: Setup Python | ||
| uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5 | ||
| with: | ||
| python-version: '3.12' | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Ensure Jupyter is available on the runner.
Kernel startup will fail if Jupyter/ipykernel aren’t present. Install minimal deps before tests.
- name: Setup Python
uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5
with:
python-version: '3.12'
+ - name: Install Python Jupyter deps
+ run: |
+ python -m pip install --upgrade pip
+ python -m pip install jupyter ipykernel
+
- name: Install dependencies
run: npm ci --prefer-offline --no-audit
env:
NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}Also applies to: 171-175
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the full CI workflow to see what happens after Python setup
cat .github/workflows/ci.yml | grep -A 50 "Setup Python"
Length of output: 1472
🏁 Script executed:
#!/bin/bash
# Search for jupyter/ipykernel references in codebase
rg -i "jupyter|ipykernel" --type-list | head -20Length of output: 389
🏁 Script executed:
#!/bin/bash
# Check package files for Jupyter dependencies
find . -name "package.json" -o -name "pyproject.toml" -o -name "requirements*.txt" -o -name "setup.py" | head -20Length of output: 457
🏁 Script executed:
#!/bin/bash
# Search for kernel or Jupyter in test files
rg -i "kernel" -C3 --type ts --type js --type py | head -30Length of output: 2003
🏁 Script executed:
#!/bin/bash
# Check pythonFiles/pyproject.toml to see what Python deps are specified
cat ./pythonFiles/pyproject.tomlLength of output: 79
🏁 Script executed:
#!/bin/bash
# Check package.json test script
rg '"test"' ./package.json -A 2 -B 2Length of output: 388
🏁 Script executed:
#!/bin/bash
# Search for Jupyter/ipykernel without type restriction
rg "jupyter|ipykernel" -iLength of output: 262776
🏁 Script executed:
#!/bin/bash
# Look for Python kernel startup in test code
rg "startKernel|kernelspec|launch.*kernel" -i -C 3Length of output: 280309
Install Jupyter + ipykernel on the runner before running tests.
Many tests start a Jupyter server or assert ipykernel is installed (startJupyterServer, multiple datascience tests). The workflow sets up Python but does not install Jupyter/ipykernel — kernel startup will fail during xvfb-run npm test. Add a step right after "Setup Python" to install minimal Python Jupyter deps (add jupyterlab if you need jupyter lab):
- File: .github/workflows/ci.yml (after Setup Python step)
- Suggested step:
python -m pip install --upgrade pip
python -m pip install jupyter ipykernel
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 166-170: the workflow sets up Python but
does not install Jupyter/ipykernel, causing tests that start a Jupyter server or
require ipykernel to fail; add a new step immediately after the "Setup Python"
step that upgrades pip and installs the minimal Jupyter dependencies (at least
jupyter and ipykernel, optionally jupyterlab if needed) using python -m pip
install --upgrade pip and python -m pip install jupyter ipykernel so kernel
startup succeeds during tests.
| - name: Run Deepnote integration tests | ||
| run: xvfb-run -a npm test | ||
| env: | ||
| VSC_JUPYTER_CI_TEST_GREP: 'Deepnote Integration Tests' | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Capture diagnostics on failures (screenshots/logs).
Tests take screenshots on failure; upload them as artifacts to debug CI.
- name: Run Deepnote integration tests
run: xvfb-run -a npm test
env:
VSC_JUPYTER_CI_TEST_GREP: 'Deepnote Integration Tests'
+
+ - name: Upload screenshots (on failure)
+ if: failure()
+ uses: actions/upload-artifact@694cdabd8bdb0f10b2cea11669e1bf5453eed0a6 # v4
+ with:
+ name: deepnote-integration-screenshots
+ path: |
+ **/*screenshot*.png
+ **/screenshots/**📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Run Deepnote integration tests | |
| run: xvfb-run -a npm test | |
| env: | |
| VSC_JUPYTER_CI_TEST_GREP: 'Deepnote Integration Tests' | |
| - name: Run Deepnote integration tests | |
| run: xvfb-run -a npm test | |
| env: | |
| VSC_JUPYTER_CI_TEST_GREP: 'Deepnote Integration Tests' | |
| - name: Upload screenshots (on failure) | |
| if: failure() | |
| uses: actions/upload-artifact@694cdabd8bdb0f10b2cea11669e1bf5453eed0a6 # v4 | |
| with: | |
| name: deepnote-integration-screenshots | |
| path: | | |
| **/*screenshot*.png | |
| **/screenshots/** |
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 179 to 183: the Deepnote integration test
step runs xvfb-run npm test but does not upload failure artifacts
(screenshots/logs); update the workflow to add a subsequent step that uses
actions/upload-artifact to upload the test failure artifacts (e.g., the
screenshots directory and any log files) with a clear artifact name, run it
conditioned to always() or at least on failure() so artifacts are preserved when
tests fail, and ensure paths match where tests write screenshots and logs and
that the step runs in the same job/workspace so the files are available.
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove header from new test file (guidelines).
New files shouldn’t include the Microsoft copyright header.
As per coding guidelines.
-// Copyright (c) Microsoft Corporation.
-// Licensed under the MIT License.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Copyright (c) Microsoft Corporation. | |
| // Licensed under the MIT License. |
🤖 Prompt for AI Agents
In src/test/datascience/notebook/deepnote.vscode.test.ts around lines 1 to 2,
remove the Microsoft copyright header present at the top of this new test file;
simply delete those header lines so the file follows project guidelines that new
files must not include the Microsoft copyright header.
| /* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires */ | ||
| import { assert } from 'chai'; | ||
| import * as path from '../../../platform/vscode-path/path'; | ||
| import { Uri, workspace } from 'vscode'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Import NotebookCellKind and use constants, not magic numbers.
Avoid brittle numeric enums in tests.
-import { Uri, workspace } from 'vscode';
+import { NotebookCellKind, Uri, workspace } from 'vscode';Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/test/datascience/notebook/deepnote.vscode.test.ts around line 7, the test
currently imports Uri and workspace and uses magic numeric values for notebook
cell kinds; update the import to also include NotebookCellKind from 'vscode' and
replace any numeric literals (e.g. 0/1 or other numbers used to denote cell
kinds) with the appropriate NotebookCellKind constants (NotebookCellKind.Markup,
NotebookCellKind.Code) so the test uses named enum values instead of brittle
magic numbers.
| const deepnoteFileUri = Uri.file( | ||
| path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'datascience', 'notebook', 'test.deepnote') | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use Uri.joinPath per guidelines.
Prefer Uri.joinPath over path.join + Uri.file.
As per coding guidelines.
- const deepnoteFileUri = Uri.file(
- path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'datascience', 'notebook', 'test.deepnote')
- );
+ const deepnoteFileUri = Uri.joinPath(
+ Uri.file(EXTENSION_ROOT_DIR_FOR_TESTS),
+ 'src',
+ 'test',
+ 'datascience',
+ 'notebook',
+ 'test.deepnote'
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const deepnoteFileUri = Uri.file( | |
| path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'datascience', 'notebook', 'test.deepnote') | |
| ); | |
| const deepnoteFileUri = Uri.joinPath( | |
| Uri.file(EXTENSION_ROOT_DIR_FOR_TESTS), | |
| 'src', | |
| 'test', | |
| 'datascience', | |
| 'notebook', | |
| 'test.deepnote' | |
| ); |
🤖 Prompt for AI Agents
In src/test/datascience/notebook/deepnote.vscode.test.ts around lines 27 to 29,
replace the Uri.file(path.join(...)) usage with Uri.joinPath to construct the
file URI; specifically, call Uri.joinPath starting from
Uri.file(EXTENSION_ROOT_DIR_FOR_TESTS) and pass the path segments
('src','test','datascience','notebook','test.deepnote') as separate arguments so
the URI is built using VS Code's recommended API.
| if (cell.kind === 1) { | ||
| return notebook.appendCodeCell( | ||
| cell.document.getText(), | ||
| cell.document.languageId, | ||
| cell.metadata | ||
| ); | ||
| } else { | ||
| return notebook.appendMarkdownCell(cell.document.getText(), cell.metadata); | ||
| } |
There was a problem hiding this comment.
Replace magic numbers with NotebookCellKind.
1 = Code, 2 = Markup today, but using constants is safer and clearer.
- if (cell.kind === 1) {
+ if (cell.kind === NotebookCellKind.Code) {
return notebook.appendCodeCell(
cell.document.getText(),
cell.document.languageId,
cell.metadata
);
} else {
return notebook.appendMarkdownCell(cell.document.getText(), cell.metadata);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (cell.kind === 1) { | |
| return notebook.appendCodeCell( | |
| cell.document.getText(), | |
| cell.document.languageId, | |
| cell.metadata | |
| ); | |
| } else { | |
| return notebook.appendMarkdownCell(cell.document.getText(), cell.metadata); | |
| } | |
| if (cell.kind === NotebookCellKind.Code) { | |
| return notebook.appendCodeCell( | |
| cell.document.getText(), | |
| cell.document.languageId, | |
| cell.metadata | |
| ); | |
| } else { | |
| return notebook.appendMarkdownCell(cell.document.getText(), cell.metadata); | |
| } |
🤖 Prompt for AI Agents
In src/test/datascience/notebook/deepnote.vscode.test.ts around lines 52 to 60,
replace the magic numeric cell.kind checks with the NotebookCellKind enum to
make intent explicit and robust: use NotebookCellKind.Code instead of 1 and
NotebookCellKind.Markup (or NotebookCellKind.Markdown if your VSCode version
uses that name) instead of 2; ensure NotebookCellKind is imported from 'vscode'
at the top of the file if not already, and update the conditional branches to
compare against those enum members.
| await waitForExecutionCompletedSuccessfully(cell!); | ||
|
|
||
| assert.isTrue(cell!.executionSummary?.success, 'Cell execution should succeed'); | ||
| assert.isAtLeast(cell!.outputs.length, 0, 'Cell should have outputs'); |
There was a problem hiding this comment.
Vacuous assertion; require at least one output.
Length >= 0 is always true.
- assert.isAtLeast(cell!.outputs.length, 0, 'Cell should have outputs');
+ assert.isAtLeast(cell!.outputs.length, 1, 'Cell should have outputs');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert.isAtLeast(cell!.outputs.length, 0, 'Cell should have outputs'); | |
| assert.isAtLeast(cell!.outputs.length, 1, 'Cell should have outputs'); |
🤖 Prompt for AI Agents
In src/test/datascience/notebook/deepnote.vscode.test.ts around line 118, the
assertion assert.isAtLeast(cell!.outputs.length, 0, 'Cell should have outputs')
is vacuous (always true); change it to require at least one output by asserting
length >= 1 (e.g., use assert.isAtLeast(cell!.outputs.length, 1, 'Cell should
have at least one output') or assert.isAbove(cell!.outputs.length, 0, 'Cell
should have at least one output')).
| const codeCells = notebook.cells.filter((c) => c.kind === 2); // Code cells | ||
| assert.isAtLeast(codeCells.length, 1, 'Should have at least one code cell'); | ||
|
|
There was a problem hiding this comment.
Wrong kind constant; this filters markup cells, not code.
Use NotebookCellKind.Code.
- const codeCells = notebook.cells.filter((c) => c.kind === 2); // Code cells
+ const codeCells = notebook.cells.filter((c) => c.kind === NotebookCellKind.Code); // Code cells📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const codeCells = notebook.cells.filter((c) => c.kind === 2); // Code cells | |
| assert.isAtLeast(codeCells.length, 1, 'Should have at least one code cell'); | |
| const codeCells = notebook.cells.filter((c) => c.kind === NotebookCellKind.Code); // Code cells | |
| assert.isAtLeast(codeCells.length, 1, 'Should have at least one code cell'); |
🤖 Prompt for AI Agents
In src/test/datascience/notebook/deepnote.vscode.test.ts around lines 127 to
129, the test filters cells using the numeric constant 2 which corresponds to
markup cells; change the filter to use NotebookCellKind.Code instead (import
NotebookCellKind from 'vscode' if not already imported) so the test selects
actual code cells, and keep the assertion as-is.
| const codeCells = notebook.cells.filter((c) => c.kind === 2 && c.document.getText().trim().length > 0); | ||
| if (codeCells.length < 2) { | ||
| this.skip(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Same issue here; correct the code-cell filter.
Use NotebookCellKind.Code and keep non-empty check.
- const codeCells = notebook.cells.filter((c) => c.kind === 2 && c.document.getText().trim().length > 0);
+ const codeCells = notebook.cells.filter(
+ (c) => c.kind === NotebookCellKind.Code && c.document.getText().trim().length > 0
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const codeCells = notebook.cells.filter((c) => c.kind === 2 && c.document.getText().trim().length > 0); | |
| if (codeCells.length < 2) { | |
| this.skip(); | |
| } | |
| const codeCells = notebook.cells.filter( | |
| (c) => c.kind === NotebookCellKind.Code && c.document.getText().trim().length > 0 | |
| ); |
🤖 Prompt for AI Agents
In src/test/datascience/notebook/deepnote.vscode.test.ts around lines 141 to
145, the notebook cell filter uses a magic number (2) for the cell kind; replace
that with NotebookCellKind.Code and keep the non-empty document text check. If
NotebookCellKind is not already imported from 'vscode', add the import. Ensure
the filter reads something like: notebook.cells.filter(c => c.kind ===
NotebookCellKind.Code && c.document.getText().trim().length > 0) and run tests.
| blocks: | ||
| - id: init-block-1 | ||
| type: code | ||
| sortingKey: '0001' | ||
| content: | | ||
| # This is the init notebook | ||
| init_variable = "initialized" | ||
| print("Init notebook executed") | ||
| - id: main-notebook-id |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Validate fixture schema against the deserializer; consider adding language for code blocks.
If the deserializer keys differ (e.g., expects language), parsing may degrade. Add language to be explicit.
blocks:
- id: init-block-1
type: code
+ language: python
sortingKey: '0001'
content: |
# This is the init notebook
init_variable = "initialized"
print("Init notebook executed")
@@
- id: block-1
type: code
+ language: python
sortingKey: '0001'
content: |
print("Hello World")
- id: block-2
type: code
+ language: python
sortingKey: '0002'
content: |
x = 42
print(f"The answer is {x}")Also applies to: 22-33
🤖 Prompt for AI Agents
In src/test/datascience/notebook/test.deepnote around lines 11 to 19 (and also
apply the same change to 22-33), the fixture's code block objects lack an
explicit language key which can break the deserializer that expects a language
field; update each code block object to include a language property (e.g.,
language: "python") and ensure the fixture schema matches the deserializer's
expected keys, then run the deserializer unit tests to verify parsing succeeds.
|
@FilipPyrek rabbit has some valid feedback please address that first before opening your PRs |
@saltenasl probably some missclick, I'm not aware of marking this PR as ready for review |
Fixes GRN-4766
Summary
Adds integration tests for .deepnote file handling in the VS Code extension. This PR introduces a test suite that verifies loading, kernel startup, and code execution for .deepnote files.
Changes
Test Files
src/test/datascience/notebook/deepnote.vscode.test.ts: New integration test suite with 5 tests:src/test/datascience/notebook/test.deepnote: Test fixture with minimal .deepnote structure including:initNotebookIdreferenceCI Configuration
.github/workflows/ci.yml: Newintegration-testsjob that:VSC_JUPYTER_CI_TEST_GREP: 'Deepnote Integration Tests'Review Focus
Init notebook auto-execution: The requirements mentioned testing "Making sure the init notebook runs automatically" but there's no dedicated test for this feature. The fixture includes an
initNotebookId, but we should verify if a test is needed to validate automatic execution.Test fixture validity: The
.deepnotefile structure was created from type definitions without real-world examples. Verify the YAML structure correctly matches what the deserializer expects.Test execution pattern: Tests use
TestNotebookDocumentand manually copy cells from the deserialized notebook. Confirm this accurately represents real .deepnote file usage in VS Code.Tests not run locally: These integration tests were compiled but not executed. First CI run will be the initial validation.
Testing
Link to Devin run: https://app.devin.ai/sessions/e14ceb76944741e79823eb71a2088bd8
Requested by: Filip Pyrek ([email protected]) (@FilipPyrek)
Summary by CodeRabbit
Tests
Chores