Skip to content

Conversation

@tkislan
Copy link
Member

@tkislan tkislan commented Nov 4, 2025

Summary by CodeRabbit

  • Tests
    • Added unit tests verifying generation of a requirements.txt when none exists: ensures the file is created at the expected path and contains the specified packages with exact version operators and formatting (each dependency on its own line, LF line endings, and a trailing newline).

Signed-off-by: Tomas Kislan <tomas@kislan.sk>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

📝 Walkthrough

Walkthrough

Adds a unit test suite for DeepnoteRequirementsHelper.node that verifies createRequirementsFile writes a requirements.txt when none exists. The test mocks VSCode workspace context, fs.promises.access and fs.promises.writeFile, logging, and a cancellable token. It constructs a DeepnoteProject with dependencies (numpy>=1.20.0, pandas==1.3.0, matplotlib) and asserts the written path contains requirements.txt and the file content matches exact LF-separated lines with a trailing newline.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Unit Test
    participant Helper as DeepnoteRequirementsHelper
    participant FS as fs.promises
    participant Assert as Assertions

    Note right of Test `#DDEBF7`: Setup mocks\n(workspace, token, logger)
    Test->>Helper: createRequirementsFile(project, token)
    Helper->>FS: access(path)  -- mocked to throw (file missing)
    alt file missing
        Helper->>FS: writeFile(path, content)  -- captured by mock
        FS-->>Helper: resolve
        Helper-->>Test: return success
    end
    Test->>Assert: verify path contains "requirements.txt"
    Test->>Assert: verify captured content:
    Note right of Assert `#F7F3D9`: "numpy>=1.20.0\npandas==1.3.0\nmatplotlib\n"
    Assert-->>Test: pass/fail
Loading

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title directly describes the main change: adding a unit test for DeepnoteRequirementsHelper.

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73%. Comparing base (c80091e) to head (ee6c9b7).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #178   +/-   ##
=====================================
  Coverage     73%     73%           
=====================================
  Files        574     575    +1     
  Lines      46881   46922   +41     
  Branches    5521    5521           
=====================================
+ Hits       34428   34488   +60     
+ Misses     10635   10612   -23     
- Partials    1818    1822    +4     
Files with missing lines Coverage Δ
...pnote/deepnoteRequirementsHelper.node.unit.test.ts 100% <100%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 31be353 and 507508e.

📒 Files selected for processing (1)
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.unit.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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 then property to avoid hanging tests

Files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.unit.test.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.unit.test.ts (3)
src/test/mocks/vsc/index.ts (1)
  • CancellationToken (100-112)
src/test/vscode-mock.ts (2)
  • resetVSCodeMocks (60-79)
  • mockedVSCodeNamespaces (17-17)
src/platform/deepnote/deepnoteTypes.ts (1)
  • DeepnoteProject (12-12)

coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 4, 2025
Signed-off-by: Tomas Kislan <tomas@kislan.sk>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 507508e and ecc5ec2.

📒 Files selected for processing (1)
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.unit.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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 then property to avoid hanging tests

Files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.unit.test.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.unit.test.ts (3)
src/test/mocks/vsc/index.ts (1)
  • CancellationToken (100-112)
src/test/vscode-mock.ts (2)
  • resetVSCodeMocks (60-79)
  • mockedVSCodeNamespaces (17-17)
src/platform/deepnote/deepnoteTypes.ts (1)
  • DeepnoteProject (12-12)
⏰ 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). (1)
  • GitHub Check: Build & Test

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ecc5ec2 and 39061de.

📒 Files selected for processing (1)
  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.unit.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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 then property to avoid hanging tests

Files:

  • src/notebooks/deepnote/deepnoteRequirementsHelper.node.unit.test.ts
🧬 Code graph analysis (1)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.unit.test.ts (3)
src/test/mocks/vsc/index.ts (1)
  • CancellationToken (100-112)
src/test/vscode-mock.ts (2)
  • resetVSCodeMocks (60-79)
  • mockedVSCodeNamespaces (17-17)
src/platform/deepnote/deepnoteTypes.ts (1)
  • DeepnoteProject (12-12)
⏰ 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). (1)
  • GitHub Check: Build & Test
🔇 Additional comments (5)
src/notebooks/deepnote/deepnoteRequirementsHelper.node.unit.test.ts (5)

1-11: LGTM!

Imports are clean and appropriate for the test setup.


13-19: LGTM!

Test suite structure is well-defined.


20-34: LGTM!

Setup correctly initializes mocks and creates the helper instance. The CancellationToken mock does not need a then property per previous discussion.

Based on learnings


36-39: LGTM!

Proper teardown prevents test pollution.


69-70: Good improvement with ENOENT error code.

The test now uses a realistic filesystem error with the ENOENT code, improving test fidelity over the previous generic error.

@tkislan tkislan marked this pull request as ready for review November 4, 2025 15:46
@tkislan tkislan requested a review from a team as a code owner November 4, 2025 15:46
@saltenasl saltenasl enabled auto-merge (squash) November 4, 2025 15:50
@saltenasl saltenasl merged commit 332d615 into main Nov 4, 2025
13 checks passed
@saltenasl saltenasl deleted the tk/deepnote-requirements-helper-test branch November 4, 2025 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants