Skip to content

feat: add integration tests to CI workflow #290

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.

Sign up for GitHub

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

Open
Melvin0070 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: add integration tests to CI workflow #290

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a4a4fc1
feat: add integration tests to CI workflow
Melvin0070 Jan 12, 2026
427ce17
fix: address PR review feedback
Melvin0070 Jan 12, 2026
490eb45
Merge branch 'main' into issue-216-integration-tests-ci
Melvin0070 Jan 20, 2026
20c1d9f
Merge branch 'main' into issue-216-integration-tests-ci
Melvin0070 Jan 28, 2026
c93e71d
Merge branch 'main' into issue-216-integration-tests-ci
Melvin0070 Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,81 @@ jobs:
- name: Check licenses
run: npm run check-licenses

integration-tests:
name: Integration Tests
runs-on: ubuntu-latest
timeout-minutes: 60
needs: [build]
if: "!contains(github.event.head_commit.message, '[skip-integration]')"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
steps:
- name: Checkout
uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6

- name: Setup Node.js
uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f # v6
with:
cache: 'npm'
node-version-file: '.nvmrc'
Comment on lines +147 to +154
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 28, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Inconsistent action versions.

actions/checkout and actions/setup-node use different commit SHAs here than in other jobs (lint, build, etc.). Align them for consistency and easier maintenance.

♻️ Suggested fix 
       - name: Checkout
-        uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6

       - name: Setup Node.js
-        uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f # v6
+        uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6
📝 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.

Suggested change
- name: Checkout
uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6
- name: Setup Node.js
uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f # v6
with:
cache: 'npm'
node-version-file: '.nvmrc'
- name: Checkout
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
- name: Setup Node.js
uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6
with:
cache: 'npm'
node-version-file: '.nvmrc'
🤖 Prompt for AI Agents 
In @.github/workflows/ci.yml around lines 147 - 154, The workflow uses different
pinned SHAs for the same actions (`actions/checkout` and `actions/setup-node`)
in this job compared to other jobs; update the `uses:` entries for
`actions/checkout` and `actions/setup-node` in this job to match the same commit
SHAs used in the other jobs (make the `uses:` strings identical across jobs) so
all jobs reference the same action versions for consistency and easier
maintenance.


- name: Setup Python
uses: actions/setup-python@8d9ed9ac5c53483de85588cdf95a591a75ab9f55 # v5.5.0
with:
python-version: '3.11'
cache: 'pip'
Comment on lines +156 to +160
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 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 2

Repository: 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 2

Repository: 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.yml

Repository: 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.


- name: Install npm dependencies
run: |
npm ci --prefer-offline --no-audit
# Verify Tailwind CSS native modules are installed for Linux
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
# 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

Copilot uses AI. Check for mistakes.
node -e "try { require('lightningcss'); } catch { process.exit(1); }" 2>/dev/null || npm install lightningcss-linux-x64-gnu
node -e "try { require('@tailwindcss/oxide'); } catch { process.exit(1); }" 2>/dev/null || npm install @tailwindcss/oxide-linux-x64-gnu
Comment on lines +162 to +168
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 10, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Duplicated npm install workaround from build job.

The lightningcss/oxide fallback block is copy-pasted from lines 100-105. Consider extracting it into a shared composite action or reusable step to keep them in sync.

🤖 Prompt for AI Agents 
In @.github/workflows/ci.yml around lines 162 - 168, The same lightningcss/oxide
native-install fallback logic is duplicated in the "Install npm dependencies"
step (and the "build" job); extract that logic into a single reusable unit
(either a composite GitHub Action or a reusable workflow step) named clearly
(e.g., install-tailwind-native) and replace the inline node -e fallback lines
for lightningcss and `@tailwindcss/oxide` in both the "Install npm dependencies"
step and the "build" job with a single call to that reusable unit so both jobs
share the same implementation and stay in sync.


- name: Compile TypeScript
run: npm run compile

- name: Create test virtual environments
uses: ./.github/actions/create-venv-for-tests
with:
IPyWidgetVersion: '8'

- name: Set Python path for tests
uses: ./.github/actions/set-python

- name: Install xvfb for headless VS Code
run: |
sudo apt-get update
sudo apt-get install -y xvfb libxkbfile1 libsecret-1-0 libgbm1 libnss3 libatk-bridge2.0-0 libgtk-3-0

- name: Run integration tests
run: xvfb-run --auto-servernum --server-args="-screen 0 1920x1080x24" npm run test:integration
env:
VSC_JUPYTER_CI_TEST: '1'
TEST_FILES_SUFFIX: '*.vscode.test,*.vscode.common.test'
CODE_TESTS_WORKSPACE: 'src/test/datascience'
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
TEST_FILES_SUFFIX: '*.vscode.test,*.vscode.common.test'
CODE_TESTS_WORKSPACE: 'src/test/datascience'

Copilot uses AI. Check for mistakes.
DISPLAY: ':99'
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
DISPLAY: ':99'

Copilot uses AI. Check for mistakes.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

- name: Upload test results on failure
if: failure()
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4
with:
name: integration-test-results
path: |
test-results.xml
logs/
**/*.png
retention-days: 7

- 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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment on lines +191 to +201
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Feb 10, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

**/*.png glob is too broad — may capture unrelated files.

This could pull in PNGs from node_modules or other directories, inflating artifact size. Scope it to test output paths.

♻️ Suggested narrowing 
           path: |
             test-results.xml
             logs/
-            **/*.png
+            test-results/**/*.png
+            screenshots/**/*.png
           retention-days: 7

Adjust the paths to wherever your test runner actually writes screenshots.

🤖 Prompt for AI Agents 
In @.github/workflows/ci.yml around lines 191 - 201, The artifact upload step
"Upload test artifacts on failure" currently uses a broad glob '**/*.png' which
may capture unrelated files; update the upload-artifact `path` entries (for the
step that uses actions/upload-artifact) to only include the test-runner's
screenshot/output directories (for example the specific folders your test runner
writes to such as e2e/screenshots, cypress/screenshots, or
test-results/screenshots) instead of the recursive '**/*.png' so only
test-related PNGs are uploaded and artifact size is limited.


Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
- 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

Copilot uses AI. Check for mistakes.
check_licenses:
name: Check Licenses
runs-on: ubuntu-latest
Expand Down
Loading