Skip to content

another fix: push checkout/installation into if block and add vault setup for custom E2E reporter#3341

Merged
Dave Shoup (shouples) merged 2 commits intomainfrom
djs/another-e2e-reporting-followup
Mar 23, 2026
Merged

another fix: push checkout/installation into if block and add vault setup for custom E2E reporter#3341
Dave Shoup (shouples) merged 2 commits intomainfrom
djs/another-e2e-reporting-followup

Conversation

@shouples
Copy link
Copy Markdown
Contributor

Good catch by Copilot #3340 (review)

@shouples Dave Shoup (shouples) requested a review from a team as a code owner March 23, 2026 14:45
Copilot AI review requested due to automatic review settings March 23, 2026 14:45
Copy link
Copy Markdown
Contributor

@jlrobins James Robinson (jlrobins) left a comment

Choose a reason for hiding this comment

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

yeah, don't do half work unless going to also do the other half, eh?

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

Adjusts the Semaphore Playwright E2E pipeline’s after_pipeline reporting job so it only performs repository checkout / dependency installation / Vault setup when the workflow is schedule-triggered, enabling the custom Playwright reporter to be available and authenticated.

Changes:

  • Wrap checkout, cache restore, npm ci, and Vault setup inside the scheduled-run if block in after_pipeline.
  • Add Vault setup before running the custom Kafka Playwright results reporter during scheduled runs.
  • Minor YAML formatting change to the blob-report artifact push command.
Comments suppressed due to low confidence (1)

.semaphore/playwright-e2e.yml:69

  • blob-report/*.zip can expand to multiple files, but $(basename blob-report/*.zip) and artifact push ... --destination ... expect a single destination path. This will fail or upload to an unintended destination when multiple blob report zips exist (the make target docs indicate multiple zips are expected). Consider looping over each zip (computing basename per file) or using an artifact destination directory that preserves filenames.
            - artifact push workflow blob-report/*.zip --destination blob-report/$(basename blob-report/*.zip)
            - make remove-test-env

checkout
make ci-bin-sem-cache-restore
npm ci --prefer-offline --include=dev
. vault-setup
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.

vault-setup is sourced unconditionally inside the scheduled-run block. Earlier in this pipeline the Vault setup is guarded by checking SEMAPHORE_ORGANIZATION_URL to skip Vault on Semaphore CI; without the same guard here, scheduled runs on hosted Semaphore could fail during after_pipeline. Consider reusing the same conditional skip logic before sourcing vault-setup.

Suggested change
. vault-setup
if [[ -z "${SEMAPHORE_ORGANIZATION_URL:-}" ]]; then
. vault-setup
fi

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No need for this since scheduled runs aren't set up for the public Semaphore instance

@shouples Dave Shoup (shouples) merged commit 39fad2a into main Mar 23, 2026
3 of 12 checks passed
@shouples Dave Shoup (shouples) deleted the djs/another-e2e-reporting-followup branch March 23, 2026 14:49
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.

4 participants