Skip to content

Conversation

@jamesbhobbs
Copy link
Contributor

@jamesbhobbs jamesbhobbs commented Oct 10, 2025

Summary

Adds Codecov integration to vscode-deepnote repository to enable coverage reporting and test results upload, matching the setup implemented in jupyterlab-deepnote.

Changes

  • Coverage reporting: Added lcov reporter to nyc configuration in src/test/coverage.node.ts to generate codecov-compatible coverage format
  • CI integration: Added two new workflow steps in .github/workflows/ci.yml:
    • Upload coverage data to Codecov using codecov/codecov-action@v5
    • Upload test results to Codecov using codecov/test-results-action@v1
  • Both steps use fail_ci_if_error: false to avoid blocking CI if Codecov has issues

Prerequisites

Human Review Checklist

⚠️ Critical items to verify:

  1. File paths: Confirm that:

    • nyc outputs coverage to coverage/lcov.info (based on report-dir config)
    • mocha-junit-reporter outputs to test-report.xml in repository root
  2. Environment setup: Verify VSC_JUPYTER_INSTRUMENT_CODE_FOR_COVERAGE environment variable is set during CI test runs (required for nyc coverage instrumentation)

  3. CODECOV_TOKEN: Ensure the secret is configured in repository settings

  4. Integration: Check that adding lcov reporter doesn't conflict with existing test infrastructure

Notes

  • Could not run local verification (lint/format/tests) due to authentication issues with private @deepnote/blocks package
  • File paths are based on code analysis of existing nyc and mocha-junit-reporter configurations
  • Existing .github/codecov.yml already configures project and patch coverage checks, so no additional codecov configuration needed

Link to Devin run: https://app.devin.ai/sessions/469dd0ba825c4715b7080a86ea978fcb
Requested by: @jamesbhobbs

Summary by CodeRabbit

  • Chores

    • CI now uploads code coverage and test results to Codecov after tests, improving visibility of quality metrics in pull requests.
    • Uploads run after tests and are skipped when a workflow is canceled; upload failures won’t break the CI run.
  • Tests

    • Coverage reporting now includes LCOV format for better integration with external tools.
    • Test runs are instrumented to collect richer coverage data.

- Add lcov reporter to nyc configuration for codecov compatibility
- Add codecov upload steps to CI workflow for coverage and test results
- Configure fail_ci_if_error: false to not block CI on codecov issues

Project already has .github/codecov.yml with project and patch coverage checks configured.
@devin-ai-integration
Copy link

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 10, 2025

📝 Walkthrough

Walkthrough

CI sets VSC_JUPYTER_INSTRUMENT_CODE_FOR_COVERAGE=true for the test run, then runs tests. After tests, the workflow uploads coverage to Codecov via codecov/codecov-action@v5 (uploads coverage/lcov.info, uses secrets.CODECOV_TOKEN, fail_ci_if_error: true). If the run is not cancelled, it uploads test results via codecov/test-results-action@v1 (uploads test-report.xml, uses secrets.CODECOV_TOKEN, fail_ci_if_error: true). Node test coverage reporters now include lcov alongside existing reporters (html when enabled, text, text-summary). No exported/public API changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant GH as GitHub Actions (CI)
  participant Tests as Test Runner + NYC
  participant Codecov as Codecov

  Dev->>GH: Push / PR triggers CI
  GH->>Tests: Set env VSC_JUPYTER_INSTRUMENT_CODE_FOR_COVERAGE=true
  GH->>Tests: Run tests
  Tests->>Tests: Produce coverage (text, text-summary, lcov, html*)
  note right of Tests #D3E4CD: lcov.info emitted

  GH->>Codecov: Upload coverage (codecov-action@v5)<br/>inputs: coverage/lcov.info, token<br/>fail_ci_if_error: true
  alt Not cancelled
    GH->>Codecov: Upload test results (test-results-action@v1)<br/>inputs: test-report.xml, token<br/>fail_ci_if_error: true
  else Cancelled
    GH--xCodecov: Skip test-results upload
  end
Loading

Possibly related PRs

  • deepnote/deepnote-internal#18480: Adds LCOV generation and CI coverage steps (V8→LCOV conversion and c8) — strongly related to LCOV and Codecov changes.
  • deepnote/deepnote-internal#18479: Instruments Node tests to produce coverage and test-result artifacts — directly complements uploading those artifacts.
  • deepnote/deepnote-internal#18074: Modifies CI coverage handling and Codecov upload steps (LCOV-related) — overlaps CI workflow changes.

Suggested reviewers

  • saltenasl
  • Artmann
  • andyjakubowski

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely describes the main change—adding Codecov integration with test results upload—and reflects the key CI workflow updates introduced in the PR.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 574e270 and 36a4ab4.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml (1 hunks)
  • src/test/coverage.node.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.node.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use *.node.ts for Desktop-specific implementations that require full file system access and Python environments

Files:

  • src/test/coverage.node.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Use l10n.t() for user-facing strings
Use typed error classes from src/platform/errors/ when throwing or handling errors
Use the ILogger service instead of console.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 with CancellationToken

**/*.{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/coverage.node.ts
🔇 Additional comments (3)
src/test/coverage.node.ts (1)

17-17: LGTM.

Adds LCOV reporter for Codecov integration. Matches the CI workflow's expected coverage/lcov.info path.

.github/workflows/ci.yml (2)

95-100: LGTM.

Coverage upload correctly uses fail_ci_if_error: false and references the LCOV file generated by NYC.


102-108: test-report.xml generation validated. Unittests use mocha-multi-reporters with mocha-junit-reporter, which by default emits test-report.xml at the repo root.

…t results upload

- Add VSC_JUPYTER_INSTRUMENT_CODE_FOR_COVERAGE env var to test step to enable coverage generation
- Add fail_ci_if_error: false to test results upload for consistency with coverage upload

Addresses CodeRabbit automated review feedback on PR #35
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

📥 Commits

Reviewing files that changed from the base of the PR and between 36a4ab4 and 5cb5722.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (1 hunks)

Change fail_ci_if_error from false to true for both coverage and test results uploads to ensure CI fails when codecov upload fails, as requested by user.
@codecov
Copy link

codecov bot commented Oct 10, 2025

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

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

♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)

104-110: Match error handling on test-results upload.

Same problem here—Codecov hiccups shouldn’t fail CI. Align this step with the coverage one.

           token: ${{ secrets.CODECOV_TOKEN }}
           files: test-report.xml
-          fail_ci_if_error: true
+          fail_ci_if_error: false
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5cb5722 and 0384df0.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (1 hunks)
⏰ 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

@jamesbhobbs jamesbhobbs marked this pull request as ready for review October 10, 2025 12:11
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

📥 Commits

Reviewing files that changed from the base of the PR and between 0384df0 and 57ae66d.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (1 hunks)

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