Fix CI: Split test and deploy jobs to avoid github-pages environment restrictions#14
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
commit: |
…restrictions Co-authored-by: CNSeniorious000 <74518716+CNSeniorious000@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
| with: | ||
| path: coverage | ||
| - name: Deploy to GitHub Pages | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' |
There was a problem hiding this comment.
@copilot you shouldn't remove this line
There was a problem hiding this comment.
I see you're referring to line 34. Looking at the current file, line 34 contains needs: test which is present and ensures the deploy job waits for the test job to complete successfully. Could you clarify which specific line you're concerned about? The dependency between jobs is maintained with the needs: test line.
There was a problem hiding this comment.
No, I mean the following line that you removed:
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
There was a problem hiding this comment.
You're absolutely right! I've moved the conditional back to the step level as it was originally. The if: github.event_name == 'push' && github.ref == 'refs/heads/main' condition is now applied to the "Deploy to GitHub Pages" step rather than the job level. This maintains the original behavior while still achieving the goal of separating test and deploy concerns. (commit 5d15e05)
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe workflow configuration in Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Test Job
participant Deploy Job
participant GitHub Pages
GitHub Actions->>Test Job: Run tests
Test Job->>Test Job: Upload coverage artifact
Test Job-->>Deploy Job: On success (main branch push)
Deploy Job->>Deploy Job: Download coverage artifact
Deploy Job->>GitHub Pages: Configure and deploy
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/test.yml (2)
26-31: Specify artifact retention and missing-file handlingThe artifact will live forever unless explicitly pruned. Add
retention-daysand considerif-no-files-foundto fail fast when coverage is missing.- - name: Upload coverage artifact - uses: actions/upload-artifact@v4 - with: - name: coverage-report - path: coverage + - name: Upload coverage artifact + uses: actions/upload-artifact@v4 + with: + name: coverage-report + path: coverage + retention-days: 14 # keep for two weeks + if-no-files-found: error # fail the run if coverage folder is absent
32-38: Tighten permissions to the principle of least privilegeThe global
permissionsblock grantspages: writeandid-token: writeto all jobs, yet onlydeployneeds them.
Override permissions locally and drop them from the workflow-level block to reduce the blast radius.-permissions: - contents: read - pages: write - id-token: write +permissions: # workflow-level: minimal + contents: read + + # ↓ job-level elevation +deploy: + permissions: + contents: read + pages: write + id-token: write
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/test.yml (1)
40-44: Guard against multiple artifacts with the same nameIf several CI runs execute concurrently, the download step may fetch an out-of-date artifact with the identical name. Append the run ID to the artifact name when uploading & downloading, or use the
run-numbercontext to ensure the deploy job retrieves the matching coverage.- name: coverage-report + name: coverage-report-${{ github.run_id }}
Co-authored-by: CNSeniorious000 <74518716+CNSeniorious000@users.noreply.github.com>
Fixes the CI workflow failure where tests couldn't run on feature branches and pull requests due to GitHub Pages environment protection rules.
Problem
The
test.ymlworkflow was configured to use thegithub-pagesenvironment for all jobs, including when running tests on feature branches and pull requests. This caused CI failures because environment protection rules typically restrict access to thegithub-pagesenvironment to only the main branch.Solution
Split the monolithic test job into two separate jobs:
testjob - Runs on all branches and pull requests:deployjob - Only runs on main branch pushes:github-pagesenvironment with proper access controlsBenefits
The workflow now properly separates concerns: testing runs everywhere, but deployment only happens where it should.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Summary by CodeRabbit