chore(ci): Better local running #17432
Conversation
This reverts commit fd15e54.
WalkthroughThe pull request introduces modifications to GitHub Actions configuration, CI scripts, and cache handling. The changes focus on improving environment variable management, cache URL configuration, and build process robustness. Key updates include adding a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used📓 Learnings (1).github/workflows/pullrequest.yml (1)🔇 Additional comments (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/actions/get-cache/action.yml (1)
44-45: Consider documenting cache path configuration.The cache paths have been updated to use
$HOME, which is more portable. Consider adding a comment explaining why these specific paths were chosen.Add documentation:
+ # Use $HOME for cache paths to ensure consistent permissions and portability export COREPACK_HOME="$HOME/.corepack-cache" export CYPRESS_CACHE_FOLDER="$HOME/.cypress-cache".github/workflows/pullrequest.yml (1)
105-110: Consider using GitHub Actions environment files.Instead of directly setting environment variables in the script, consider using GitHub Actions environment files for better maintainability.
- run: | - set -euo pipefail - echo "HEAD=$GITHUB_SHA" >> "$GITHUB_ENV" + run: | + { + echo "HEAD=$GITHUB_SHA" + echo "SHELL=/usr/bin/bash" + echo "WORKFLOW_ID=pullrequest" + } >> "$GITHUB_ENV"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/get-cache/action.yml(3 hunks).github/workflows/pullrequest.yml(6 hunks)
🔇 Additional comments (6)
.github/actions/get-cache/action.yml (2)
7-9: LGTM! Well-structured input parameter definition.The new
cache-urlinput parameter is properly defined with a clear description and sensible default value.
24-32: Verify cache URL configuration in different environments.The cache URL configuration has been made environment-aware, but we should verify its behavior in different scenarios.
Run this script to check the cache URL configuration:
✅ Verification successful
Cache URL configuration verified successfully
The cache URL configuration correctly handles different environments and maintains consistency across workflow files. The environment-aware implementation properly manages:
- Local development runs
- GitHub Actions environment
- Environment variable propagation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify cache URL configuration across different environments # Test: Check if the cache URL is properly configured in the workflow rg -A 3 "ACTIONS_CACHE_URL" .github/workflows/ # Test: Check if there are any hardcoded cache URLs that should use the new parameter rg -i "cache.*\.is"Length of output: 86076
.github/workflows/pullrequest.yml (4)
43-49: LGTM! Helpful debug information added.The debug step provides useful information for troubleshooting local runs, especially when using act.
89-90: LGTM! Proper local run handling.The condition to skip artifact upload during local runs is appropriate.
128-129: Fix permission check error handling.The permission check logic has been updated with
continue-on-error, but the conditions still need to be consistent.Also applies to: 132-132, 137-137
224-225: LGTM! Improved step naming.The step name has been updated to be more descriptive, which improves workflow readability.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17432 +/- ##
==========================================
+ Coverage 35.57% 35.60% +0.02%
==========================================
Files 7027 7016 -11
Lines 150413 150224 -189
Branches 42930 42878 -52
==========================================
- Hits 53516 53493 -23
+ Misses 96897 96731 -166
Flags with carried forward coverage won't be shown. Click here to find out more. see 59 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
AndesKrrrrrrrrrrr
added
the
deprecated:automerge
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/actions/get-cache/action.yml (2)
7-9: Consider making the cache URL configurable through environment variables.The hardcoded cache URL in the default value might not be suitable for all environments. Consider making it more configurable.
cache-url: description: 'Cache URL' - default: 'https://cache.dev01.devland.is/' + default: '${{ env.CACHE_URL || "https://cache.dev01.devland.is/" }}'
24-25: Improve local development experience.The condition for skipping when running locally is good, but the error handling could be improved to provide better feedback.
# Skip when running locally if: ${{ !github.event.localrun }} + continue-on-error: true uses: actions/github-script@v7 env: cache-url: ${{ inputs.cache-url }} github-token: ${{ inputs.GITHUB_TOKEN }} with: script: | + if (!process.env['cache-url']) { + core.warning('Cache URL not set, using default') + } core.exportVariable('ACTIONS_CACHE_URL', process.env['cache-url'])Also applies to: 28-32
.github/workflows/pullrequest.yml (2)
43-49: Consider adding error handling for directory creation.While creating the home directory is helpful for local development, the script should handle potential permission issues.
- name: Debug home etc. run: | + set -euo pipefail echo "User: HOME=$HOME, PWD=$PWD, id=$(id)" - mkdir -p "$HOME" + if ! mkdir -p "$HOME" 2>/dev/null; then + echo "::warning::Failed to create home directory, continuing anyway" + fi ls -lah "$HOME"
105-110: Consider using GitHub Actions default environment variables.Some of the environment variables being set are already available as default GitHub Actions environment variables.
env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - HTML_URL: ${{ github.event.pull_request.html_url }} + # Using default GITHUB_EVENT_PATH instead of explicit HTML_URL ISSUE_REPORTING_SLACK_WEBHOOK_URL: ${{ secrets.SLACK_BUILD_ISSUES_REPORTING_WEBHOOK_URL }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/get-cache/action.yml(3 hunks).github/workflows/pullrequest.yml(6 hunks)
🧰 Additional context used
📓 Learnings (1)
.github/workflows/pullrequest.yml (1)
Learnt from: AndesKrrrrrrrrrrr
PR: island-is/island.is#17432
File: .github/workflows/pullrequest.yml:71-75
Timestamp: 2025-01-09T12:32:05.402Z
Learning: In `.github/workflows/pullrequest.yml`, the `-m` flag is intentionally used with `git checkout` commands to preserve uncommitted changes when running the workflow locally with `act`. While unnecessary in GitHub Actions environment where the workspace is always clean, it's beneficial for local development workflow.
🔇 Additional comments (3)
.github/actions/get-cache/action.yml (1)
44-45: Verify cache paths are created with appropriate permissions.Using
$HOMEfor cache paths is a good practice, but ensure the directories are created with appropriate permissions.✅ Verification successful
Cache directory permissions are handled automatically ✅
The cache directories are managed automatically by GitHub Actions, Corepack, and Cypress. These tools handle directory creation and permissions internally, requiring no explicit permission setup. The environment variables only configure the cache locations, and there's no evidence of permission issues in the workflow logs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if cache directories are created with correct permissions # Test: Look for any file permission issues in the workflow logs rg "permission denied.*\.cache" .github/workflows/Length of output: 52
Script:
#!/bin/bash # Check for cache operations using these paths rg -l "corepack-cache|cypress-cache" .github/ # Look for directory creation commands rg "mkdir.*cache" .github/ # Check cache action usage rg "actions/cache" .github/Length of output: 688
.github/workflows/pullrequest.yml (2)
71-75: LGTM! The-mflag is intentionally used for local development.Based on the retrieved learnings, the
-mflag is intentionally used to preserve uncommitted changes when running the workflow locally withact.
224-225: LGTM! Improved step naming.The step naming has been improved to be more descriptive, which enhances workflow readability.
* Fixed import-merge syntax * Add conditional publishing steps * Set runner image by env * Only check existence of relevant branches * Don't run locally * Names and debug * Improve get-cache * Revert "Improve get-cache" This reverts commit fd15e54. * Add cache-url input and only set when not local * pipefail by default * Install java if missing (for codegen * Only home paths, and quoting * // For local development * Home debug * Only override cypress cache when no env variable is set * Fall back to all affected if stickman fails * Allow and handle failure in permission check * Don't collect failures * Revert codemagic.yaml to main * Comment, and improve readability * Revert image config * Revert default shell * Subshell node list * debug checking out branches * Fetch branch and comment * Create main branches as well * Remove duplicate branch creation * Remove java install step * Refactor 'Preparing HEAD and BASE tags' * Only try to create branches if they don't exist * Revert "Only try to create branches if they don't exist" This reverts commit c0b1146. * Spacing * Verify or create branches * Remove // For local development * Comment on Cypress override * Reduce diff for 00_prepare-base-tags.sh * pullrequest.yml cleanup * Always wait for collector (revert to main) * Support localrun & less diff * Re-add octokit auth * Debug and pipefail get-cache exporting * Revert actions/ to main * Remove pipefail for export * Rabbit review * Move get-cache required env vars back to github-script * Revert checking out relevant branches to main, but with force * checkout, but preserve local changes * localrun cleanup * Only use github.event.localrun for local condition * Use if for permission, not continue-on-error (reviewfix) * Don't run permission check locally
Summary by CodeRabbit
Summary by CodeRabbit
Chores
New Features
Bug Fixes