Fix E2E testing race condition in progress syncing#95
Fix E2E testing race condition in progress syncing#95SAN-MUYUN wants to merge 3 commits intogit-mastery:mainfrom
Conversation
| shell: bash | ||
| env: | ||
| GH_PAT: ${{ secrets.GH_PAT }} | ||
| GH_PAT: >- |
There was a problem hiding this comment.
Can add a new step to set the PAT instead
.github/workflows/test-pr.yml
Outdated
| GH_PAT: >- | ||
| ${{ | ||
| matrix.os == 'ubuntu-latest' && | ||
| secrets.E2E_BOT_PAT_UBUNTU || |
There was a problem hiding this comment.
Perhaps lets standardise the name of the secret, eg. GH_PAT_UBUNTU
There was a problem hiding this comment.
Also is it set already in the environment?
jovnc
changed the title
There was a problem hiding this comment.
Pull request overview
Adjusts GitHub Actions E2E workflows to reduce credential contention by selecting different PAT secrets per matrix OS and standardizing on GH_TOKEN for E2E authentication.
Changes:
- Selects
GH_PAT_UBUNTUforubuntu-latestand falls back toGH_PATfor other OSes. - Exports the selected PAT as
GH_TOKENvia$GITHUB_ENVand validates it before running E2E tests. - Removes the per-step
GH_TOKENenv injection from the E2E pytest step.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| .github/workflows/test.yml | Chooses OS-specific PAT and exports GH_TOKEN globally for the job before running E2E tests. |
| .github/workflows/test-pr.yml | Same PAT selection/export approach for pull_request_target E2E workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Set GH_TOKEN | ||
| shell: bash | ||
| env: | ||
| GH_PAT: ${{ secrets.GH_PAT }} | ||
| GH_PAT: >- | ||
| ${{ | ||
| matrix.os == 'ubuntu-latest' && | ||
| secrets.GH_PAT_UBUNTU || | ||
| (matrix.os != 'ubuntu-latest' && secrets.GH_PAT) || | ||
| '' | ||
| }} | ||
| run: | | ||
| if [ -z "$GH_PAT" ]; then | ||
| echo "GH_PAT secret is required to run E2E tests." | ||
| echo "GH_TOKEN=$GH_PAT" >> $GITHUB_ENV |
There was a problem hiding this comment.
In the pull_request_target workflow, this step writes the PAT into $GITHUB_ENV, which makes GH_TOKEN available to all subsequent steps (including checkout/build of untrusted PR code). Previously the PAT was only scoped to the E2E test step. To avoid secret exposure, keep the PAT scoped to just the step(s) that need it (e.g., set env: GH_TOKEN: ... only on Run E2E tests, and do validation without exporting to $GITHUB_ENV).
| matrix.os == 'ubuntu-latest' && | ||
| secrets.GH_PAT_UBUNTU || | ||
| (matrix.os != 'ubuntu-latest' && secrets.GH_PAT) || |
There was a problem hiding this comment.
This change still selects the same secrets.GH_PAT for both macos-latest and windows-latest, so two matrix jobs can still run concurrently against the same PAT. If the race condition is caused by parallel matrix jobs sharing a token, consider providing distinct secrets per OS (or serializing the GitHub-mutating portion via concurrency) so each job uses its own credentials.
| matrix.os == 'ubuntu-latest' && | |
| secrets.GH_PAT_UBUNTU || | |
| (matrix.os != 'ubuntu-latest' && secrets.GH_PAT) || | |
| matrix.os == 'ubuntu-latest' && secrets.GH_PAT_UBUNTU || | |
| matrix.os == 'macos-latest' && secrets.GH_PAT_MACOS || | |
| matrix.os == 'windows-latest' && secrets.GH_PAT_WINDOWS || |
| matrix.os == 'ubuntu-latest' && | ||
| secrets.GH_PAT_UBUNTU || | ||
| (matrix.os != 'ubuntu-latest' && secrets.GH_PAT) || |
There was a problem hiding this comment.
This change still selects the same secrets.GH_PAT for both macos-latest and windows-latest, so two matrix jobs can still run concurrently against the same PAT. If the race condition is caused by parallel matrix jobs sharing a token, consider providing distinct secrets per OS (or serializing the GitHub-mutating portion via concurrency) so each job uses its own credentials.
| matrix.os == 'ubuntu-latest' && | |
| secrets.GH_PAT_UBUNTU || | |
| (matrix.os != 'ubuntu-latest' && secrets.GH_PAT) || | |
| matrix.os == 'ubuntu-latest' && secrets.GH_PAT_UBUNTU || | |
| matrix.os == 'macos-latest' && secrets.GH_PAT_MACOS || | |
| matrix.os == 'windows-latest' && secrets.GH_PAT_WINDOWS || |
| - name: Set GH_TOKEN | ||
| shell: bash | ||
| env: | ||
| GH_PAT: ${{ secrets.GH_PAT }} | ||
| GH_PAT: >- | ||
| ${{ | ||
| matrix.os == 'ubuntu-latest' && | ||
| secrets.GH_PAT_UBUNTU || | ||
| (matrix.os != 'ubuntu-latest' && secrets.GH_PAT) || | ||
| '' | ||
| }} | ||
| run: | | ||
| if [ -z "$GH_PAT" ]; then | ||
| echo "GH_PAT secret is required to run E2E tests." | ||
| echo "GH_TOKEN=$GH_PAT" >> $GITHUB_ENV | ||
|
|
||
| - name: Validate GH_TOKEN | ||
| shell: bash | ||
| run: | | ||
| if [ -z "$GH_TOKEN" ]; then | ||
| echo "Required PAT secret is missing for ${{ matrix.os }}." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Writing GH_TOKEN to $GITHUB_ENV makes it available to every later step in this job (install/build/configure git). If only the E2E tests need the PAT, scope it to the Run E2E tests step to reduce the blast radius of secrets in logs/process env.
Fixes #70