Skip to content

fix after_pipeline for E2E reporting#3340

Merged
Dave Shoup (shouples) merged 1 commit intomainfrom
djs/e2e-afterpipeline-checkout
Mar 23, 2026
Merged

fix after_pipeline for E2E reporting#3340
Dave Shoup (shouples) merged 1 commit intomainfrom
djs/e2e-afterpipeline-checkout

Conversation

@shouples
Copy link
Copy Markdown
Contributor

Follow-up based on the errors we're seeing in CI For the scheduled E2E test runs:

[Mar 23 07:56:31.361] Successfully pulled artifact for current workflow.
[Mar 23 07:56:31.361] * Remote source: 'artifacts/workflows/33e65626-f701-4c4e-a707-6b8b066c142c/blob-report'.
[Mar 23 07:56:31.361] * Local destination: 'blob-report'.
[Mar 23 07:56:31.361] Pulled 8 files. Total of 172.6 MB
npm warn exec The following package was not found and will be installed: playwright@1.58.2
Error: Cannot find module './tests/e2e/reporters/kafka-results-reporter.ts'
  • need checkout so tests/e2e/reporters/kafka-results-reporter.ts is available
  • make ci-bin-sem-cache-restore isn't really necessary, but should help speed up npm ci
  • npm ci added since the custom reporter needs to use a few packages

Copilot AI review requested due to automatic review settings March 23, 2026 14:31
@shouples Dave Shoup (shouples) requested a review from a team as a code owner March 23, 2026 14:31
@shouples Dave Shoup (shouples) merged commit 5fd1232 into main Mar 23, 2026
3 checks passed
@shouples Dave Shoup (shouples) deleted the djs/e2e-afterpipeline-checkout branch March 23, 2026 14:33
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes scheduled Semaphore Playwright E2E after_pipeline reporting by ensuring the custom Kafka reporter source and its Node dependencies are available when merging blob reports.

Changes:

  • Add checkout in after_pipeline so tests/e2e/reporters/kafka-results-reporter.ts exists when referenced.
  • Restore Semaphore caches and run npm ci in after_pipeline so the reporter’s runtime dependencies are installed.

- npm ci --prefer-offline --include=dev
# Publish to dedicated Kafka topic for additional test result visualization/analysis
- export E2E_KAFKA_REPORTER_TOPIC=vscode-test-results
- if [[ "$SEMAPHORE_WORKFLOW_TRIGGERED_BY_SCHEDULE" == "true" ]]; then
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

kafka-results-reporter.ts reads credentials via vault kv get at module load time, but this after_pipeline job doesn't run the same Vault setup used by the E2E blocks (the conditional . vault-setup/login). Once the reporter file is available, npx playwright merge-reports is likely to fail here due to missing Vault auth/token. Add the same Vault initialization step in this job (ideally only for the scheduled path).

Suggested change
- if [[ "$SEMAPHORE_WORKFLOW_TRIGGERED_BY_SCHEDULE" == "true" ]]; then
- if [[ "$SEMAPHORE_WORKFLOW_TRIGGERED_BY_SCHEDULE" == "true" ]]; then
. vault-setup;

Copilot uses AI. Check for mistakes.
Comment on lines +220 to 225
- checkout
- make ci-bin-sem-cache-restore
- npm ci --prefer-offline --include=dev
# Publish to dedicated Kafka topic for additional test result visualization/analysis
- export E2E_KAFKA_REPORTER_TOPIC=vscode-test-results
- if [[ "$SEMAPHORE_WORKFLOW_TRIGGERED_BY_SCHEDULE" == "true" ]]; then
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

checkout, cache restore, and npm ci now run unconditionally in after_pipeline, but the blob report merge (and Kafka publishing via the custom reporter) only runs for scheduled workflows. Consider moving these expensive steps inside the SEMAPHORE_WORKFLOW_TRIGGERED_BY_SCHEDULE conditional so non-scheduled runs don’t pay the extra time/network cost.

Copilot uses AI. Check for mistakes.
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