Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
21 changes: 21 additions & 0 deletions .github/workflows/code-qa.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,27 @@ jobs:
- name: Create .env.local file
working-directory: apps/vscode-e2e
run: echo "OPENROUTER_API_KEY=${{ secrets.OPENROUTER_API_KEY }}" > .env.local
- name: Set VS Code test version
Copy link

Choose a reason for hiding this comment

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

Consider adding a comment explaining why version 1.101.2 was chosen specifically. This would help future maintainers understand when and why to update it. For example:

Suggested change
- name: Set VS Code test version
- name: Set VS Code test version
# Using a specific version instead of 'stable' to ensure deterministic caching
# Update this version periodically to match the minimum supported VS Code version
run: echo "VSCODE_VERSION=1.101.2" >> $GITHUB_ENV

run: echo "VSCODE_VERSION=1.101.2" >> $GITHUB_ENV
Copy link

Choose a reason for hiding this comment

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

The VS Code version is specified both here as an environment variable and as a fallback in line 103. To ensure consistency and avoid potential mismatches, could we reference the environment variable directly in the Node script without the fallback? This would ensure a single source of truth.

- name: Cache VS Code test runtime
uses: actions/cache@v4
with:
path: apps/vscode-e2e/.vscode-test
key: ${{ runner.os }}-vscode-test-${{ env.VSCODE_VERSION }}
restore-keys: |
${{ runner.os }}-vscode-test-
Copy link

Choose a reason for hiding this comment

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

The cache restore-keys fallback to just ${{ runner.os }}-vscode-test- could potentially restore an incompatible cache from a different VS Code version. Is this fallback truly beneficial, or might it cause issues if the cached version doesn't match the expected one? Consider whether a strict version match would be safer.

- name: Pre-download VS Code test runtime
Copy link

Choose a reason for hiding this comment

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

Since network issues are often transient, would it be helpful to add retry logic here? Something like:

Suggested change
- name: Pre-download VS Code test runtime
- name: Pre-download VS Code test runtime
working-directory: apps/vscode-e2e
run: |
for i in 1 2 3; do
node -e "
const { downloadAndUnzipVSCode } = require('@vscode/test-electron');
downloadAndUnzipVSCode({ version: process.env.VSCODE_VERSION || '1.101.2' })
.then(() => {
console.log('✅ VS Code test runtime downloaded successfully');
process.exit(0);
})
.catch(err => {
console.error('❌ Failed to download VS Code (attempt ' + $i + '):', err);
if ($i === 3) process.exit(1);
});
" && break || sleep 5
done

working-directory: apps/vscode-e2e
run: |
node -e "
const { downloadAndUnzipVSCode } = require('@vscode/test-electron');
downloadAndUnzipVSCode({ version: process.env.VSCODE_VERSION || '1.101.2' })
.then(() => console.log('✅ VS Code test runtime downloaded successfully'))
.catch(err => {
console.error('❌ Failed to download VS Code:', err);
process.exit(1);
});
"
- name: Run integration tests
working-directory: apps/vscode-e2e
run: xvfb-run -a pnpm test:ci
Expand Down
1 change: 1 addition & 0 deletions apps/vscode-e2e/src/runTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ async function main() {
extensionTestsPath,
launchArgs: [testWorkspace],
extensionTestsEnv,
version: process.env.VSCODE_VERSION || "1.101.2",
})

// Clean up the temporary workspace
Expand Down
Loading