Skip to content

ci: skip live e2e tests on fork PRs#654

Merged
serhalp merged 2 commits into
mainfrom
serhalp/skip-live-tests-forks
Apr 14, 2026
Merged

ci: skip live e2e tests on fork PRs#654
serhalp merged 2 commits into
mainfrom
serhalp/skip-live-tests-forks

Conversation

@serhalp

@serhalp serhalp commented Apr 14, 2026

Copy link
Copy Markdown
Member

Some tests always fail on PRs from forks. This is a poor experience for external contributors as well as for maintainers, and it does not foster OSS contributions.

Skip tests that require access to secrets and use GitHub "annotations" to remind maintainers that this has happened.

@coderabbitai

coderabbitai Bot commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a650ec9d-3606-4db1-9ab7-e596c52da86f

📥 Commits

Reviewing files that changed from the base of the PR and between 056fbf0 and 4e6f05f.

📒 Files selected for processing (2)
  • .github/workflows/test.yaml
  • packages/vite-plugin-tanstack-start/test/e2e/build.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • .github/workflows/test.yaml
  • packages/vite-plugin-tanstack-start/test/e2e/build.test.ts

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • Chores
    • Modified CI/CD pipeline to skip live end-to-end tests for fork pull requests and display warnings when tests are skipped, protecting sensitive credentials.

Walkthrough

The changes implement a mechanism to skip live end-to-end tests for pull requests from forked repositories. A GitHub Actions workflow now conditionally sets SKIP_LIVE_TESTS=true when the PR originates from a fork (not netlify/primitives), and emits a warning in the job summary. A test file was updated to check this environment variable and skip the Netlify deployment test suite accordingly when the flag is active.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: skipping live e2e tests on fork PRs, which matches the workflow modifications and test gating added in this changeset.
Description check ✅ Passed The description clearly explains the motivation (tests failing on forks hurts contributor experience) and the solution (skip tests requiring secrets and use GitHub annotations), which directly relates to the changes made in the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch serhalp/skip-live-tests-forks

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/test.yaml:
- Around line 60-62: The "Flag fork PR" step's condition uses
github.event.pull_request.head.repo.full_name without first ensuring the event
is a pull_request, causing push and merge events to treat null as an empty
string and erroneously set SKIP_LIVE_TESTS; update the condition on the "Flag
fork PR" step to first check that github.event_name equals "pull_request" and
only then compare github.event.pull_request.head.repo.full_name against
"netlify/primitives" so SKIP_LIVE_TESTS is set only for forked pull requests.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5e3ca105-28cc-4263-acb1-9f5eb4118f0b

📥 Commits

Reviewing files that changed from the base of the PR and between 336e407 and 056fbf0.

📒 Files selected for processing (2)
  • .github/workflows/test.yaml
  • packages/vite-plugin-tanstack-start/test/e2e/build.test.ts

Comment thread .github/workflows/test.yaml
@serhalp serhalp force-pushed the serhalp/skip-live-tests-forks branch from 056fbf0 to a4e396f Compare April 14, 2026 14:10
@serhalp serhalp force-pushed the serhalp/skip-live-tests-forks branch from a4e396f to dfdaf69 Compare April 14, 2026 14:13
@serhalp serhalp marked this pull request as ready for review April 14, 2026 15:02
@serhalp serhalp requested a review from a team as a code owner April 14, 2026 15:02
@serhalp serhalp requested review from eduardoboucas and removed request for a team April 14, 2026 15:02

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dfdaf699fa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if:
${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name !=
'netlify/primitives' }}
run: echo "SKIP_LIVE_TESTS=true" >> "$GITHUB_ENV"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Write SKIP_LIVE_TESTS using a shell-agnostic command

This step executes in the test matrix on windows-latest as well, but echo "SKIP_LIVE_TESTS=true" >> "$GITHUB_ENV" uses Bash-style variable expansion. Because no shell override is set, Windows runners use PowerShell, where this expression does not resolve the environment file path correctly, so fork PR jobs can fail here instead of skipping live tests. Please either force shell: bash or use PowerShell-compatible syntax (and apply the same fix to the later $GITHUB_STEP_SUMMARY writes).

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

GHA defaults to bash even on Windows runners!

@serhalp serhalp enabled auto-merge (squash) April 14, 2026 17:30
@serhalp serhalp merged commit ecba898 into main Apr 14, 2026
18 checks passed
@serhalp serhalp deleted the serhalp/skip-live-tests-forks branch April 14, 2026 17:43
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.

2 participants