feat: add integration tests to CI workflow #290
Conversation
Closes deepnote#216 - Add new integration-tests job that runs after build - Setup Python 3.11 with xvfb for headless VS Code - Use existing create-venv-for-tests composite action - Upload test artifacts (screenshots, logs) on failure - Allow skipping with [skip-integration] in commit message
📝 WalkthroughWalkthroughA new Sequence Diagram(s)sequenceDiagram
participant GitHubActions as GitHub Actions Runner
participant Checkout as Repository Checkout
participant Node as Node.js/npm
participant Python as Python/venv/pytest
participant Xvfb as Xvfb (headless X server)
participant Artifacts as Artifact Storage
GitHubActions->>Checkout: checkout code
GitHubActions->>Node: setup Node.js, restore/cache deps
Node->>Node: install npm deps (with Linux fallbacks)
Node->>GitHubActions: compilation (TypeScript)
GitHubActions->>Python: setup Python, create venv, install test deps
GitHubActions->>Xvfb: install/start Xvfb for headless tests
Python->>Xvfb: run integration tests (headless)
Python->>Artifacts: on failure upload test results/screenshots
GitHubActions->>Artifacts: upload logs/artifacts
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds integration tests to the CI workflow, enabling automated VS Code extension integration testing on every PR and push to main. The implementation follows existing CI patterns and introduces proper headless testing support using xvfb on Linux.
Changes:
- Added new
integration-testsjob that runs after successful build - Configured headless VS Code testing environment with xvfb and required dependencies
- Set up Python 3.11 with test virtual environments using composite actions
- Added artifact uploads for test results, logs, and screenshots on failure
- Included skip condition via
[skip-integration]commit message tag
.github/workflows/ci.yml
Outdated
| - name: Upload screenshots on failure | ||
| if: failure() | ||
| uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4 | ||
| with: | ||
| name: integration-test-screenshots | ||
| path: | | ||
| **/*screenshot*.png | ||
| **/*Screen*.png | ||
| retention-days: 7 | ||
| if-no-files-found: ignore | ||
|
|
There was a problem hiding this comment.
The first artifact upload already includes all PNG files with the pattern '**/*.png' (line 199). This second upload step for screenshots is redundant and will capture the same files, resulting in duplicate artifacts.
| - name: Upload screenshots on failure | |
| if: failure() | |
| uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4 | |
| with: | |
| name: integration-test-screenshots | |
| path: | | |
| **/*screenshot*.png | |
| **/*Screen*.png | |
| retention-days: 7 | |
| if-no-files-found: ignore |
.github/workflows/ci.yml
Outdated
| TEST_FILES_SUFFIX: '*.vscode.test,*.vscode.common.test' | ||
| CODE_TESTS_WORKSPACE: 'src/test/datascience' |
There was a problem hiding this comment.
The environment variables TEST_FILES_SUFFIX and CODE_TESTS_WORKSPACE are already set in the test:integration script in package.json (line 2589). Setting them again here is redundant. Consider removing these environment variables or document why they need to be overridden.
| TEST_FILES_SUFFIX: '*.vscode.test,*.vscode.common.test' | |
| CODE_TESTS_WORKSPACE: 'src/test/datascience' |
.github/workflows/ci.yml
Outdated
| VSC_JUPYTER_CI_TEST: '1' | ||
| TEST_FILES_SUFFIX: '*.vscode.test,*.vscode.common.test' | ||
| CODE_TESTS_WORKSPACE: 'src/test/datascience' | ||
| DISPLAY: ':99' |
There was a problem hiding this comment.
The DISPLAY environment variable is unnecessary when using 'xvfb-run --auto-servernum' as it automatically manages the display. The --auto-servernum flag already handles display selection, making this explicit setting redundant.
| DISPLAY: ':99' |
.github/workflows/ci.yml
Outdated
| - name: Install npm dependencies | ||
| run: | | ||
| npm ci --prefer-offline --no-audit | ||
| # Verify Tailwind CSS native modules are installed for Linux |
There was a problem hiding this comment.
The comment on this line should match the more complete comment in the build job (lines 102-103) which includes a reference to the npm issue. The comment should be: "Verify Tailwind CSS native modules are installed for Linux (npm optional deps bug)" followed by "See: npm/cli#4828" on the next line for consistency.
| # Verify Tailwind CSS native modules are installed for Linux | |
| # Verify Tailwind CSS native modules are installed for Linux (npm optional deps bug) | |
| # See: https://github.com/npm/cli/issues/4828 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 184-189: The DISPLAY env var is redundant because xvfb-run
--auto-servernum sets and exports DISPLAY itself; remove the DISPLAY: ':99'
environment variable from the CI job OR alternatively stop using
--auto-servernum and change xvfb-run to use a fixed display number (e.g.,
replace --auto-servernum with -n 99) so the explicit DISPLAY matches the chosen
server number; update the command/env accordingly around the xvfb-run
invocation.
- Line 143: Replace the brittle skip-condition that only checks
github.event.head_commit.message with a fallback that also checks the PR title
(or an empty string) so PR synchronize events work; update the if expression
used in the workflow (the `if: "!contains(github.event.head_commit.message,
'[skip-integration]')"` line) to something like `if:
"!contains(github.event.pull_request.title || github.event.head_commit.message
|| '', '[skip-integration]')"` so the check uses the pull_request title when
head_commit.message is null.
- Around line 191-211: The two GitHub Actions steps "Upload test results on
failure" and "Upload screenshots on failure" both run on failure() and both
upload .png files, causing redundancy; either remove the redundant step or
narrow the patterns so they don't overlap. Fix by editing the artifact upload
steps: either delete the "Upload screenshots on failure" step entirely and keep
the unified paths in "Upload test results on failure" (ensure name reflects
screenshots if kept), or remove **/*.png from the "Upload test results on
failure" step and leave the "Upload screenshots on failure" step as the sole
uploader for screenshot patterns (**/*screenshot*.png, **/*Screen*.png) with
if-no-files-found: ignore; update the artifact names accordingly to avoid
duplicate uploads.
📜 Review details
Configuration used: Organization 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.
📒 Files selected for processing (1)
.github/workflows/ci.yml
⏰ 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: Agent
🔇 Additional comments (4)
.github/workflows/ci.yml (4)
138-142: Job setup looks good.Reasonable timeout, proper dependency on
build, and consistent action versions with the rest of the workflow.
144-168: Standard CI setup steps.Node/Python caching and dependency installation follow existing patterns. The Tailwind native module fallback matches the
buildjob.
178-181: Headless dependencies are appropriate.Correct package set for running VS Code under xvfb on Ubuntu.
170-176: Composite actions are present and properly configured.Both
.github/actions/create-venv-for-testsand.github/actions/set-pythonexist with validaction.ymlconfigurations. TheIPyWidgetVersion: '8'parameter matches the expected input definition.
- Fix skip condition for PR events - Remove redundant env vars (handled by npm script) - Consolidate artifact uploads - Add npm issue reference
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 156-160: The CI job step named "Setup Python" currently uses with:
cache: 'pip' which is ineffective because there is no root-level dependency
file; either add a root-level dependency file (e.g., requirements.txt or
pyproject.toml) containing the repo's Python dependencies (or create a small
file that imports/includes build/venv-test-ipywidgets8-requirements.txt or
references pythonFiles/pyproject.toml) so the actions/setup-python cache can
detect and key by it, or remove the cache: 'pip' line from the "Setup Python"
step to disable pip caching; update the workflow step accordingly and ensure the
change references the same step name ("Setup Python") in
.github/workflows/ci.yml.
📜 Review details
Configuration used: Organization 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.
📒 Files selected for processing (1)
.github/workflows/ci.yml
🔇 Additional comments (6)
.github/workflows/ci.yml (6)
138-145: Job definition and skip condition look correct.The
ifcondition properly handles both PR events (checking title) and push events (checking commit message) via fallback. Timeout and dependency onbuildare sensible.
162-171: Consistent with build job pattern.Dependency installation and TypeScript compilation follow the same approach as the existing
buildjob, including the npm optional deps workaround.
181-189: xvfb setup and test execution look solid.Library dependencies for headless VS Code are comprehensive. The screen resolution (1920x1080x24) is appropriate for UI testing.
191-201: Artifact upload configured well.
if: failure()withif-no-files-found: ignoreis a safe combination. 7-day retention is reasonable for debugging flaky tests.
173-179: No action needed—both composite actions exist. The referenced actions./.github/actions/create-venv-for-testsand./.github/actions/set-pythonare present in the repository.
138-201: No duplication exists. Theintegration-testsjob appears only once in the workflow file. The AI summary's claim of identical duplicate blocks is unfounded.Likely an incorrect or invalid review comment.
| - name: Setup Python | ||
| uses: actions/setup-python@8d9ed9ac5c53483de85588cdf95a591a75ab9f55 # v5.5.0 | ||
| with: | ||
| python-version: '3.11' | ||
| cache: 'pip' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for pip dependency files that setup-python can use for caching
fd -t f -g 'requirements*.txt' -g 'pyproject.toml' -g 'setup.py' -g 'Pipfile.lock' -d 2Repository: deepnote/vscode-deepnote
Length of output: 283
🏁 Script executed:
#!/bin/bash
# Search for Python dependency files
fd -t f 'requirements' -d 2
fd -t f 'pyproject\.toml' -d 2
fd -t f 'setup\.py' -d 2
fd -t f 'Pipfile\.lock' -d 2Repository: deepnote/vscode-deepnote
Length of output: 173
🏁 Script executed:
#!/bin/bash
# Check the workflow file context around the Python setup
sed -n '150,180p' .github/workflows/ci.ymlRepository: deepnote/vscode-deepnote
Length of output: 1259
The pip cache setting is ineffective without a root-level dependency file.
The cache: 'pip' setting looks for requirements.txt, pyproject.toml, setup.py, or Pipfile.lock at the repository root to generate a cache key. The only Python dependency files in this repo are in subdirectories (build/venv-test-ipywidgets8-requirements.txt, pythonFiles/pyproject.toml). Move or create a root-level dependency file, or remove the cache setting.
🤖 Prompt for AI Agents
In @.github/workflows/ci.yml around lines 156 - 160, The CI job step named
"Setup Python" currently uses with: cache: 'pip' which is ineffective because
there is no root-level dependency file; either add a root-level dependency file
(e.g., requirements.txt or pyproject.toml) containing the repo's Python
dependencies (or create a small file that imports/includes
build/venv-test-ipywidgets8-requirements.txt or references
pythonFiles/pyproject.toml) so the actions/setup-python cache can detect and key
by it, or remove the cache: 'pip' line from the "Setup Python" step to disable
pip caching; update the workflow step accordingly and ensure the change
references the same step name ("Setup Python") in .github/workflows/ci.yml.
1 participant
Description
Implements #216 - Run integration tests in CI
This PR adds a new
integration-testsjob to the CI workflow that runs VS Code extension integration tests on every PR and push to main.Changes
integration-testsjob to.github/workflows/ci.ymlbuildcompletes successfully[skip-integration]to commit messageTesting
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.