chore(ci): parametrize update e2e tests in e2e-tests-main workflow #13608
Conversation
|
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. 📝 WalkthroughWalkthroughAdds three workflow_dispatch inputs to .github/workflows/e2e-main.yaml: update_target_owner, update_target_repo, and update_prerelease. Sets UPDATE_TARGET_OWNER / UPDATE_TARGET_REPO / UPDATE_PRERELEASE from inputs and persists via GITHUB_ENV. Per-OS steps (Windows/macOS) adjust PD_DIST_PATH/PD_APP_PATH targets, replace owner/repo in app-update.yml (PowerShell/sed), and print updated files. Conditional blocks enable autoUpdater.allowPrerelease when UPDATE_PRERELEASE == 'true' (Windows: append with file-exists guard; macOS: insert via sed). These envs are propagated into win-update-e2e-test and mac-update-e2e-test jobs. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In the Windows job block your updateFile path is declared as "env:PD_DIST_PATH/..." but should use "${env:PD_DIST_PATH}/..." (e.g. "$env:PD_DIST_PATH/..."), otherwise your Test-Path check will always fail.
- In the macOS step you’re using ${{ env.OWNER }} which isn’t defined; you probably meant ${{ env.UPDATE_TARGET_OWNER }} and also need to swap out the ‘owner:’ key instead of reusing the repo replacement logic.
- The default env var initialization is duplicated across multiple jobs—consider pulling that into a reusable step or using YAML anchors to DRY up the workflow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the Windows job block your updateFile path is declared as "env:PD_DIST_PATH/..." but should use "${env:PD_DIST_PATH}/..." (e.g. "$env:PD_DIST_PATH/..."), otherwise your Test-Path check will always fail.
- In the macOS step you’re using ${{ env.OWNER }} which isn’t defined; you probably meant ${{ env.UPDATE_TARGET_OWNER }} and also need to swap out the ‘owner:’ key instead of reusing the repo replacement logic.
- The default env var initialization is duplicated across multiple jobs—consider pulling that into a reusable step or using YAML anchors to DRY up the workflow.
## Individual Comments
### Comment 1
<location> `.github/workflows/e2e-main.yaml:273` </location>
<code_context>
run: |
- echo "Replace app-update.yml repo to a testing-prerelease, which are more stable update target then the prerelease"
- $updateFile = "$env:PD_DIST_PATH/resources/app-update.yml"
+ $updateFile = "env:PD_DIST_PATH/resources/app-update.yml"
if (-Not (Test-Path $updateFile)) {
Write-Error "app-update.yml not found at $updateFile"
</code_context>
<issue_to_address>
Incorrect PowerShell variable expansion for $updateFile path.
Use "$env:PD_DIST_PATH/resources/app-update.yml" to ensure the environment variable is expanded and the path is resolved correctly.
</issue_to_address>
### Comment 2
<location> `.github/workflows/e2e-main.yaml:387` </location>
<code_context>
+ - name: Manually set update target owner/repo ${{ env.UPDATE_TARGET_OWNER }}/${{ env.UPDATE_TARGET_REPO }}
+ run: |
+ echo "Replace app-update.yml repo to a testing-prerelease"
+ sudo sed -i '' 's/repo: podman-desktop/owner: ${{ env.OWNER}}/' "$PD_APP_PATH/Contents/Resources/app-update.yml"
+ sudo sed -i '' 's/repo: podman-desktop/repo: ${{ env.UPDATE_TARGET_REPO}}/' "$PD_APP_PATH/Contents/Resources/app-update.yml"
+ echo "Show app-update.yml after replace..."
</code_context>
<issue_to_address>
Incorrect environment variable reference in sed command.
Update the sed command to use ${{ env.UPDATE_TARGET_OWNER }} instead of ${{ env.OWNER }} to match the input variable and ensure correct substitution.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/e2e-main.yaml (2)
271-283: Fix Windows path expansion and use -Raw for content replacement.
- Path bug: "$updateFile = "env:PD_DIST_PATH/..." uses a literal string. It must be "$env:PD_DIST_PATH/...".
- Replacement bug: Same array vs string issue as above; use Get-Content -Raw and do both replacements on one buffer.
- YAMLlint flagged trailing spaces at Line 283; remove them.
Apply this diff:
- $updateFile = "env:PD_DIST_PATH/resources/app-update.yml" + $updateFile = "$env:PD_DIST_PATH/resources/app-update.yml" if (-Not (Test-Path $updateFile)) { Write-Error "app-update.yml not found at $updateFile" } - (Get-Content $updateFile).Replace('owner: podman-desktop', "owner: " + $env:UPDATE_TARGET_OWNER) | - Set-Content -ErrorAction Stop $updateFile - (Get-Content $updateFile).Replace('repo: podman-desktop', "repo: " + $env:UPDATE_TARGET_REPO) | - Set-Content -ErrorAction Stop $updateFile + $content = Get-Content -Raw -ErrorAction Stop $updateFile + $content = $content -replace 'owner:\s*podman-desktop', "owner: $env:UPDATE_TARGET_OWNER" + $content = $content -replace 'repo:\s*podman-desktop', "repo: $env:UPDATE_TARGET_REPO" + Set-Content -Path $updateFile -Value $content -Encoding UTF8 -ErrorAction Stop echo "Show app-update.yml after replace..." cat "$env:PD_DIST_PATH/resources/app-update.yml" - +
384-391: Wrong env var in sed; should use UPDATE_TARGET_OWNER.The first sed uses ${{ env.OWNER }} which isn’t defined. Use UPDATE_TARGET_OWNER (set earlier). Also the echo message mentions “testing-prerelease” which doesn’t reflect dynamic owner/repo; optional to adjust.
Apply this diff:
- echo "Replace app-update.yml repo to a testing-prerelease" - sudo sed -i '' 's/owner: podman-desktop/owner: ${{ env.OWNER}}/' "$PD_APP_PATH/Contents/Resources/app-update.yml" + echo "Replace app-update.yml owner/repo to ${{ env.UPDATE_TARGET_OWNER }}/${{ env.UPDATE_TARGET_REPO }}" + sudo sed -i '' 's/owner: podman-desktop/owner: ${{ env.UPDATE_TARGET_OWNER}}/' "$PD_APP_PATH/Contents/Resources/app-update.yml" sudo sed -i '' 's/repo: podman-desktop/repo: ${{ env.UPDATE_TARGET_REPO}}/' "$PD_APP_PATH/Contents/Resources/app-update.yml" echo "Show app-update.yml after replace..." cat "$PD_APP_PATH/Contents/Resources/app-update.yml"
🧹 Nitpick comments (3)
.github/workflows/e2e-main.yaml (3)
213-222: GitHub ENV writes on Windows are correct; minor indentation warning.
- The GITHUB_ENV appends via PowerShell are correct and will persist values for subsequent steps.
- YAMLlint flagged indentation at Line 213 (“expected 4 but found 6”). Consider aligning with the surrounding steps to keep the linter happy.
271-271: Step name may not resolve ${{ env. }} set via GITHUB_ENV.*env values appended via GITHUB_ENV are not available to the expression context used in step names. You might see blanks here. If you want dynamic names, set these vars at job.env or pass them via an explicit env: block so they are in the ${{ env }} context.
Example:
win-update-e2e-test: name: ${{ matrix.os }} update e2e tests - ${{ matrix.installation }} runs-on: ${{ matrix.os }} + env: + UPDATE_TARGET_OWNER: ${{ github.event.inputs.update_target_owner || 'podman-desktop' }} + UPDATE_TARGET_REPO: ${{ github.event.inputs.update_target_repo || 'testing-prereleases' }} + UPDATE_PRERELEASE: ${{ github.event.inputs.update_prerelease || 'true' }}Then you can drop the separate “Set the default env. variables” step and safely use ${{ env.UPDATE_TARGET_OWNER }} in names.
320-329: macOS env defaults are fine; minor indentation warning.
- Similar to Windows, persisting to $GITHUB_ENV is correct.
- YAMLlint flagged indentation at Line 320. Align with the surrounding steps.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/e2e-main.yaml(7 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/e2e-main.yaml
[warning] 213-213: wrong indentation: expected 4 but found 6
(indentation)
[error] 283-283: trailing spaces
(trailing-spaces)
[warning] 320-320: wrong indentation: expected 4 but found 6
(indentation)
🔇 Additional comments (3)
.github/workflows/e2e-main.yaml (3)
60-77: Inputs look good; sensible defaults and types.The new workflow_dispatch inputs are consistent and self-explanatory. Using a choice for update_prerelease avoids YAML boolean pitfalls.
362-367: AllowPrerelease injection looks correct.BSD sed usage with -i '' and multi-line append is correct for macOS. Guard is correct, and the message clarifies intent.
206-210: Runner labels verified — windows-2025 is GA; windows-11-arm is GA for public repositories only.Verified from GitHub changelogs/docs (windows-2025 GA Apr 10, 2025; windows-11-arm GA for public repos Aug 7, 2025).
- File: .github/workflows/e2e-main.yaml (lines 206-210) — current matrix: os: [windows-2025, windows-11-arm]
- Action: If this repository is public no change is needed. If the repo is private (or you need private-run support), add a fallback (e.g., windows-2022) or guard the windows-11-arm entry so it only runs for public repos.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Run using prerelease bits and testing-prerelease target (defaults): https://github.com/odockal/podman-desktop/actions/runs/16971986632/job/48111095465 |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/e2e-main.yaml (2)
252-259: PowerShell .Replace on array; use Get-Content -Raw and -replace.(Get-Content $updaterFile) returns an array of lines; calling .Replace will not work. Read as a single string and then write back.
Apply this diff:
- if ($env:UPDATE_PRERELEASE -eq 'true') { - $updaterFile = "packages/main/src/plugin/updater.ts" - if (-Not (Test-Path $updaterFile)) { - Write-Error "Updater file not found at $updaterFile" - } else { - (Get-Content $updaterFile).Replace('autoUpdater.autoDownload = false;', 'autoUpdater.autoDownload = false;autoUpdater.allowPrerelease=true;') | Set-Content $updaterFile -ErrorAction Stop - } - } + if ($env:UPDATE_PRERELEASE -eq 'true') { + $updaterFile = "packages/main/src/plugin/updater.ts" + if (-Not (Test-Path $updaterFile)) { + Write-Error "Updater file not found at $updaterFile" + } else { + $content = Get-Content -Raw -ErrorAction Stop $updaterFile + $content = $content -replace 'autoUpdater\.autoDownload = false;', 'autoUpdater.autoDownload = false;autoUpdater.allowPrerelease=true;' + Set-Content -Path $updaterFile -Value $content -Encoding UTF8 -ErrorAction Stop + } + }
277-279: Same PowerShell Replace issue for app-update.yml; switch to -Raw and single pass.As above, use -Raw and the -replace operator. Also write once and set UTF-8 to avoid encoding issues.
Apply this diff:
- (Get-Content $updateFile).Replace('owner: podman-desktop', "owner: " + $env:UPDATE_TARGET_OWNER) | - Set-Content -ErrorAction Stop $updateFile - (Get-Content $updateFile).Replace('repo: podman-desktop', "repo: " + $env:UPDATE_TARGET_REPO) | - Set-Content -ErrorAction Stop $updateFile + $content = Get-Content -Raw -ErrorAction Stop $updateFile + $content = $content -replace 'owner:\s*podman-desktop', ('owner: ' + $env:UPDATE_TARGET_OWNER) + $content = $content -replace 'repo:\s*podman-desktop', ('repo: ' + $env:UPDATE_TARGET_REPO) + Set-Content -Path $updateFile -Value $content -Encoding UTF8 -ErrorAction Stop
🧹 Nitpick comments (5)
.github/workflows/e2e-main.yaml (5)
213-222: Fix YAML indentation to satisfy linter and avoid parsing surprises.YAMLlint flags wrong indentation on Line 213. Align this step’s indentation with sibling steps under steps:.
Apply this diff (remove two leading spaces across the block):
- - name: Set the default env. variables + - name: Set the default env. variables - env: + env: - DEFAULT_UPDATE_TARGET_OWNER: 'podman-desktop' - DEFAULT_UPDATE_TARGET_REPO: 'testing-prereleases' - DEFAULT_UPDATE_PRERELEASE: 'true' - run: | - echo "UPDATE_TARGET_OWNER=${{ github.event.inputs.update_target_owner || env.DEFAULT_UPDATE_TARGET_OWNER }}" >> $env:GITHUB_ENV - echo "UPDATE_TARGET_REPO=${{ github.event.inputs.update_target_repo || env.DEFAULT_UPDATE_TARGET_REPO }}" >> $env:GITHUB_ENV - echo "UPDATE_PRERELEASE=${{ github.event.inputs.update_prerelease || env.DEFAULT_UPDATE_PRERELEASE }}" >> $env:GITHUB_ENV + DEFAULT_UPDATE_TARGET_OWNER: 'podman-desktop' + DEFAULT_UPDATE_TARGET_REPO: 'testing-prereleases' + DEFAULT_UPDATE_PRERELEASE: 'true' + run: | + echo "UPDATE_TARGET_OWNER=${{ github.event.inputs.update_target_owner || env.DEFAULT_UPDATE_TARGET_OWNER }}" >> $env:GITHUB_ENV + echo "UPDATE_TARGET_REPO=${{ github.event.inputs.update_target_repo || env.DEFAULT_UPDATE_TARGET_REPO }}" >> $env:GITHUB_ENV + echo "UPDATE_PRERELEASE=${{ github.event.inputs.update_prerelease || env.DEFAULT_UPDATE_PRERELEASE }}" >> $env:GITHUB_ENV
271-271: Step name may not render dynamic env values.Expressions like ${{ env.UPDATE_TARGET_OWNER }} in the step name won’t see values written via GITHUB_ENV (those are runtime env vars, not expression context). The name may display empty placeholders.
Options:
- Move these vars to job-level env derived from inputs so they are in expression context; or
- Keep a static step name without interpolation.
273-276: Good fix on Windows path expansion; remove trailing whitespace.
- Line 273: Using "$env:PD_DIST_PATH/resources/app-update.yml" correctly expands the path on Windows. Nice.
- Line 283: YAMLlint flags trailing spaces. Remove the trailing whitespace-only line to keep lint clean.
Also applies to: 283-283
320-329: Fix YAML indentation here as well.YAMLlint flags wrong indentation on Line 320. Align to the same level as sibling steps.
Apply this diff (remove two leading spaces across the block):
- - name: Set the default env. variables + - name: Set the default env. variables - env: + env: - DEFAULT_UPDATE_TARGET_OWNER: 'podman-desktop' - DEFAULT_UPDATE_TARGET_REPO: 'testing-prereleases' - DEFAULT_UPDATE_PRERELEASE: 'true' - run: | - echo "UPDATE_TARGET_OWNER=${{ github.event.inputs.update_target_owner || env.DEFAULT_UPDATE_TARGET_OWNER }}" >> $GITHUB_ENV - echo "UPDATE_TARGET_REPO=${{ github.event.inputs.update_target_repo || env.DEFAULT_UPDATE_TARGET_REPO }}" >> $GITHUB_ENV - echo "UPDATE_PRERELEASE=${{ github.event.inputs.update_prerelease || env.DEFAULT_UPDATE_PRERELEASE }}" >> $GITHUB_ENV + DEFAULT_UPDATE_TARGET_OWNER: 'podman-desktop' + DEFAULT_UPDATE_TARGET_REPO: 'testing-prereleases' + DEFAULT_UPDATE_PRERELEASE: 'true' + run: | + echo "UPDATE_TARGET_OWNER=${{ github.event.inputs.update_target_owner || env.DEFAULT_UPDATE_TARGET_OWNER }}" >> $GITHUB_ENV + echo "UPDATE_TARGET_REPO=${{ github.event.inputs.update_target_repo || env.DEFAULT_UPDATE_TARGET_REPO }}" >> $GITHUB_ENV + echo "UPDATE_PRERELEASE=${{ github.event.inputs.update_prerelease || env.DEFAULT_UPDATE_PRERELEASE }}" >> $GITHUB_ENV
384-391: Owner/repo sed substitution looks correct; consider robust YAML editing.
- Good: You now reference ${{ env.UPDATE_TARGET_OWNER }} / ${{ env.UPDATE_TARGET_REPO }} (resolves the earlier feedback).
- Optional: Using sed is brittle if spacing changes. A small node or python snippet to parse and rewrite YAML would be more robust.
For example, with Node (no extra deps):
node -e " const fs=require('fs'); const p='$PD_APP_PATH/Contents/Resources/app-update.yml'; let t=fs.readFileSync(p,'utf8') .replace(/owner:\s*.*/, 'owner: ${{ env.UPDATE_TARGET_OWNER }}') .replace(/repo:\s*.*/, 'repo: ${{ env.UPDATE_TARGET_REPO }}'); fs.writeFileSync(p,t); "Also, minor nit: The echo says “Replace app-update.yml repo to a testing-prerelease” but now both owner and repo are dynamic—update the message for accuracy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/e2e-main.yaml(7 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/e2e-main.yaml
[warning] 213-213: wrong indentation: expected 4 but found 6
(indentation)
[error] 283-283: trailing spaces
(trailing-spaces)
[warning] 320-320: wrong indentation: expected 4 but found 6
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: win update e2e tests (windows-2025)
- GitHub Check: mac update e2e tests
- GitHub Check: win update e2e tests (windows-11-arm)
- GitHub Check: smoke e2e tests (production)
- GitHub Check: smoke e2e tests (development)
- GitHub Check: unit tests / windows-2025
- GitHub Check: unit tests / macos-15
- GitHub Check: unit tests / ubuntu-24.0 8000 4
- GitHub Check: typecheck
- GitHub Check: macOS
- GitHub Check: linter, formatters
- GitHub Check: build website
- GitHub Check: Linux
- GitHub Check: Windows
🔇 Additional comments (2)
.github/workflows/e2e-main.yaml (2)
60-77: Inputs for update parametrization look good.The new workflow_dispatch inputs are well-typed with sensible defaults. String choices for update_prerelease align with the string comparisons used later.
362-367: Prerelease flag injection LGTM; add a guard to fail if the anchor line isn’t found.The sed append is fine. To avoid silent no-ops if the pattern changes, verify the insertion succeeded.
Append a quick check:
sed -i '' '/autoUpdater.autoDownload = false;/a \ autoUpdater.allowPrerelease = true; \ ' packages/main/src/plugin/updater.ts + grep -q 'autoUpdater\.allowPrerelease = true;' packages/main/src/plugin/updater.ts || { echo "Failed to set allowPrerelease"; exit 1; }
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/e2e-main.yaml (2)
252-259: PowerShell Replace will fail without -Raw (array vs string); make it idempotent.
(Get-Content $updaterFile).Replace(...)operates on an array and will not work as intended. Read as a single string (-Raw) and use-replace. Also guard against double-insertion.Apply this diff:
- Write-host "Allow Prerelease when updating, setting it inline in the code" - (Get-Content $updaterFile).Replace('autoUpdater.autoDownload = false;', 'autoUpdater.autoDownload = false;autoUpdater.allowPrerelease=true;') | Set-Content $updaterFile -ErrorAction Stop + Write-Host "Allow Prerelease when updating, setting it inline in the code" + $content = Get-Content -Raw -ErrorAction Stop $updaterFile + if ($content -notmatch 'autoUpdater\.allowPrerelease\s*=\s*true;') { + $content = $content -replace 'autoUpdater\.autoDownload\s*=\s*false;', 'autoUpdater.autoDownload = false;autoUpdater.allowPrerelease = true;' + Set-Content -Path $updaterFile -Value $content -Encoding UTF8 -ErrorAction Stop + } else { + Write-Host "autoUpdater.allowPrerelease already enabled; skipping" + }
272-284: Robustly update app-update.yml; avoid multiple Get-Content calls; remove trailing whitespace.
- Use
-Rawand perform both replacements in-memory once; then write back with UTF-8.- Fix trailing spaces at Line 284 flagged by YAMLlint.
Apply this diff:
- - name: Manually set update target owner/repo ${{ env.UPDATE_TARGET_OWNER }}/${{ env.UPDATE_TARGET_REPO }} + - name: Manually set update target owner/repo ${{ env.UPDATE_TARGET_OWNER }}/${{ env.UPDATE_TARGET_REPO }} run: | - $updateFile = "$env:PD_DIST_PATH/resources/app-update.yml" + $updateFile = "$env:PD_DIST_PATH/resources/app-update.yml" if (-Not (Test-Path $updateFile)) { Write-Error "app-update.yml not found at $updateFile" } - (Get-Content $updateFile).Replace('owner: podman-desktop', "owner: " + $env:UPDATE_TARGET_OWNER) | - Set-Content -ErrorAction Stop $updateFile - (Get-Content $updateFile).Replace('repo: podman-desktop', "repo: " + $env:UPDATE_TARGET_REPO) | - Set-Content -ErrorAction Stop $updateFile + $content = Get-Content -Raw -ErrorAction Stop $updateFile + $content = $content -replace 'owner:\s*podman-desktop', ('owner: ' + $env:UPDATE_TARGET_OWNER) + $content = $content -replace 'repo:\s*podman-desktop', ('repo: ' + $env:UPDATE_TARGET_REPO) + Set-Content -Path $updateFile -Value $content -Encoding UTF8 -ErrorAction Stop echo "Show app-update.yml after replace..." - cat "$env:PD_DIST_PATH/resources/app-update.yml" - + cat "$env:PD_DIST_PATH/resources/app-update.yml"
🧹 Nitpick comments (6)
.github/workflows/e2e-main.yaml (6)
60-77: Inputs look good; minor simplification possible for boolean.The three workflow_dispatch inputs are clear and well-described. Consider using
type: booleanforupdate_prereleaseto avoid string comparisons downstream (optional).Apply this diff if you want to switch to a boolean:
- update_prerelease: - description: 'Update to the prerelease GH release' - type: choice - options: - - 'true' - - 'false' - default: 'true' - required: false + update_prerelease: + description: 'Update to the prerelease GH release' + type: boolean + default: true + required: falseDownstream condition examples:
- if ($env:UPDATE_PRERELEASE -eq 'true') { + if ("${{ github.event.inputs.update_prerelease || env.DEFAULT_UPDATE_PRERELEASE }}" -eq 'true') {
213-222: Set default envs on Windows: OK; fix YAML indentation warning.Logic for setting defaults is fine. YAMLlint 8000 flags indentation at Line 213; adjust indentation to match surrounding steps.
Apply this minimal formatting change:
- - name: Set the default env. variables + - name: Set the default env. variables env: DEFAULT_UPDATE_TARGET_OWNER: 'podman-desktop' DEFAULT_UPDATE_TARGET_REPO: 'testing-prereleases' DEFAULT_UPDATE_PRERELEASE: 'true' run: | echo "UPDATE_TARGET_OWNER=${{ github.event.inputs.update_target_owner || env.DEFAULT_UPDATE_TARGET_OWNER }}" >> $env:GITHUB_ENV echo "UPDATE_TARGET_REPO=${{ github.event.inputs.update_target_repo || env.DEFAULT_UPDATE_TARGET_REPO }}" >> $env:GITHUB_ENV echo "UPDATE_PRERELEASE=${{ github.event.inputs.update_prerelease || env.DEFAULT_UPDATE_PRERELEASE }}" >> $env:GITHUB_ENV
321-330: mac defaults step: OK; fix YAML indentation warning.Functionality is fine. YAMLlint reported indentation at Line 321; align with sibling steps.
Apply this small format change:
- - name: Set the default env. variables + - name: Set the default env. variables env: DEFAULT_UPDATE_TARGET_OWNER: 'podman-desktop' DEFAULT_UPDATE_TARGET_REPO: 'testing-prereleases' DEFAULT_UPDATE_PRERELEASE: 'true' run: | echo "UPDATE_TARGET_OWNER=${{ github.event.inputs.update_target_owner || env.DEFAULT_UPDATE_TARGET_OWNER }}" >> $GITHUB_ENV echo "UPDATE_TARGET_REPO=${{ github.event.inputs.update_target_repo || env.DEFAULT_UPDATE_TARGET_REPO }}" >> $GITHUB_ENV echo "UPDATE_PRERELEASE=${{ github.event.inputs.update_prerelease || env.DEFAULT_UPDATE_PRERELEASE }}" >> $GITHUB_ENV
359-369: Make the prerelease insertion idempotent to prevent duplicate lines.If this step runs multiple times, the
sedappend will duplicate the line. Guard withgrepfirst.Apply this diff:
- if [[ "${{ env.UPDATE_PRERELEASE }}" == "true" ]]; then - echo "Allow Prerelease when updating, setting it inline in the code" - sed -i '' '/autoUpdater.autoDownload = false;/a \ - autoUpdater.allowPrerelease = true; \ - ' packages/main/src/plugin/updater.ts - fi + if [[ "${{ env.UPDATE_PRERELEASE }}" == "true" ]]; then + echo "Allow Prerelease when updating, setting it inline in the code" + if ! grep -q 'autoUpdater\.allowPrerelease\s*=\s*true;' packages/main/src/plugin/updater.ts; then + sed -i '' '/autoUpdater.autoDownload = false;/a \ + autoUpdater.allowPrerelease = true; \ + ' packages/main/src/plugin/updater.ts + else + echo "autoUpdater.allowPrerelease already enabled; skipping" + fi + fi
385-392: Sed substitutions may break on special characters; consider a safer approach.If owner/repo ever contain sed meta characters (e.g.,
/,&,\), the replacement can fail or corrupt the file. Prefer a YAML-aware tool or a small Node/Python patch to ensure correctness.Option 1 (Python, no extra deps):
- echo "Replace app-update.yml repo to a testing-prerelease" - sudo sed -i '' 's/owner: podman-desktop/owner: ${{ env.UPDATE_TARGET_OWNER}}/' "$PD_APP_PATH/Contents/Resources/app-update.yml" - sudo sed -i '' 's/repo: podman-desktop/repo: ${{ env.UPDATE_TARGET_REPO}}/' "$PD_APP_PATH/Contents/Resources/app-update.yml" + echo "Replace app-update.yml owner/repo safely" + python3 - <<'PY' +import io, sys, re +p = sys.argv[1] +owner = """${{ env.UPDATE_TARGET_OWNER }}""" +repo = """${{ env.UPDATE_TARGET_REPO }}""" +with io.open(p, 'r', encoding='utf-8') as f: + c = f.read() +c = re.sub(r'owner:\s*podman-desktop', f'owner: {owner}', c) +c = re.sub(r'repo:\s*podman-desktop', f'repo: {repo}', c) +with io.open(p, 'w', encoding='utf-8') as f: + f.write(c) +print("Updated:", p) +PY + "$PD_APP_PATH/Contents/Resources/app-update.yml"Option 2: keep sed but switch to a delimiter unlikely to collide and escape ampersands:
owner='${{ env.UPDATE_TARGET_OWNER }}'; repo='${{ env.UPDATE_TARGET_REPO }}' owner_esc=${owner//&/\\&}; repo_esc=${repo//&/\\&} sudo sed -i '' "s|owner: podman-desktop|owner: ${owner_esc}|" "$PD_APP_PATH/Contents/Resources/app-update.yml" sudo sed -i '' "s|repo: podman-desktop|repo: ${repo_esc}|" "$PD_APP_PATH/Contents/Resources/app-update.yml"
284-284: Remove trailing whitespace.YAMLlint flagged trailing spaces at Line 284. Remove them to keep CI linters green.
- +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/e2e-main.yaml(7 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/e2e-main.yaml
[warning] 213-213: wrong indentation: expected 4 but found 6
(indentation)
[error] 284-284: trailing spaces
(trailing-spaces)
[warning] 321-321: wrong indentation: expected 4 but found 6
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: mac update e2e tests
- GitHub Check: win update e2e tests (windows-2025)
- GitHub Check: win update e2e tests (windows-11-arm)
- GitHub Check: smoke e2e tests (development)
- GitHub Check: smoke e2e tests (production)
- GitHub Check: unit tests / windows-2025
- GitHub Check: unit tests / macos-15
- GitHub Check: typecheck
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: linter, formatters
- GitHub Check: build website
- GitHub Check: macOS
- GitHub Check: Linux
- GitHub Check: Windows
🔇 Additional comments (1)
.github/workflows/e2e-main.yaml (1)
209-210: Verify runner labels exist for all org contexts.
windows-2025andwindows-11-armrunner labels may not be available in all GitHub-hosted environments. Please confirm they are present (or self-hosted) in this org; otherwise, consider a fallback towindows-latest/ an ARM strategy include.
Signed-off-by: Ondrej Dockal <odockal@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
.github/workflows/e2e-main.yaml (3)
272-276: Resolved: Correct PowerShell env var expansion in path.Good fix using "$env:PD_DIST_PATH/resources/app-update.yml" for $updateFile. This addresses the previous bug-risk note about environment variable expansion.
272-281: Same PowerShell Replace issue for app-update.yml (Windows); fix with -Raw and single write.Replace operations are invoked on arrays and piped into Set-Content twice. Read once with -Raw, do both replacements, then write once.
Apply this diff:
- (Get-Content $updateFile).Replace('owner: podman-desktop', "owner: " + $env:UPDATE_TARGET_OWNER) | - Set-Content -ErrorAction Stop $updateFile - (Get-Content $updateFile).Replace('repo: podman-desktop', "repo: " + $env:UPDATE_TARGET_REPO) | - Set-Content -ErrorAction Stop $updateFile + $content = Get-Content -Raw -ErrorAction Stop $updateFile + $content = $content -replace 'owner:\s*podman-desktop', ('owner: ' + $env:UPDATE_TARGET_OWNER) + $content = $content -replace 'repo:\s*podman-desktop', ('repo: ' + $env:UPDATE_TARGET_REPO) + Set-Content -Path $updateFile -Value $content -Encoding UTF8 -ErrorAction Stop
248-260: PowerShell Replace on array will fail; use Get-Content -Raw (Windows prerelease gating).(Get-Content $updaterFile) returns an array. Calling .Replace on it won’t work. Read the file as a single string, perform the replacement, then write it back.
Apply this diff:
- if ($env:UPDATE_PRERELEASE -eq 'true') { - $updaterFile = "packages/main/src/plugin/updater.ts" - if (-Not (Test-Path $updaterFile)) { - Write-Error "Updater file not found at $updaterFile" - } else { - Write-host "Allow Prerelease when updating, setting it inline in the code" - (Get-Content $updaterFile).Replace('autoUpdater.autoDownload = false;', 'autoUpdater.autoDownload = false;autoUpdater.allowPrerelease=true;') | Set-Content $updaterFile -ErrorAction Stop - } - } + if ($env:UPDATE_PRERELEASE -eq 'true') { + $updaterFile = "packages/main/src/plugin/updater.ts" + if (-Not (Test-Path $updaterFile)) { + Write-Error "Updater file not found at $updaterFile" + } else { + Write-Host "Allow Prerelease when updating, setting it inline in the code" + $content = Get-Content -Raw -ErrorAction Stop $updaterFile + $content = $content -replace 'autoUpdater\.autoDownload = false;', 'autoUpdater.autoDownload = false;autoUpdater.allowPrerelease=true;' + Set-Content -Path $updaterFile -Value $content -Encoding UTF8 -ErrorAction Stop + } + }
🧹 Nitpick comments (3)
.github/workflows/e2e-main.yaml (3)
389-396: Nit: Log message and robustness of sed replacements (macOS).
- The log mentions “testing-prerelease” but you’re now parameterizing both owner and repo; adjust the message to reflect actual values.
- Optional: Anchor the patterns to start of line to avoid accidental partial matches.
Apply this diff:
- echo "Replace app-update.yml repo to a testing-prerelease" - sudo sed -i '' 's/owner: podman-desktop/owner: ${{ env.UPDATE_TARGET_OWNER}}/' "$PD_APP_PATH/Contents/Resources/app-update.yml" - sudo sed -i '' 's/repo: podman-desktop/repo: ${{ env.UPDATE_TARGET_REPO}}/' "$PD_APP_PATH/Contents/Resources/app-update.yml" + echo "Set app-update.yml to owner=${UPDATE_TARGET_OWNER}, repo=${UPDATE_TARGET_REPO}" + sudo sed -i '' 's/^owner:\s*podman-desktop/owner: '"${UPDATE_TARGET_OWNER}"'/' "$PD_APP_PATH/Contents/Resources/app-update.yml" + sudo sed -i '' 's/^repo:\s*podman-desktop/repo: '"${UPDATE_TARGET_REPO}"'/' "$PD_APP_PATH/Contents/Resources/app-update.yml"Note: This uses runtime env vars in the shell to avoid expression-time evaluation limits.
248-248: Step name interpolation uses ${{ env. }} values set at runtime; may render empty.*Names are cosmetic, but expressions are evaluated before previous steps write to GITHUB_ENV. If you want dynamic labels, define these envs at job-level env:, or hardcode a stable name.
Example (job-level env):
env: UPDATE_TARGET_OWNER: ${{ github.event.inputs.update_target_owner || 'podman-desktop' }} UPDATE_TARGET_REPO: ${{ github.event.inputs.update_target_repo || 'testing-prereleases' }} UPDATE_PRERELEASE: ${{ github.event.inputs.update_prerelease || 'true' }}Also applies to: 363-363
284-284: Trim trailing whitespace to satisfy linters.There’s trailing space on this otherwise blank line.
- +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/e2e-main.yaml(7 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/e2e-main.yaml
[warning] 213-213: wrong indentation: expected 4 but found 6
(indentation)
[error] 284-284: trailing spaces
(trailing-spaces)
[warning] 325-325: wrong indentation: expected 4 but found 6
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: windows-2025 update e2e tests
- GitHub Check: macos-13 update e2e tests
- GitHub Check: windows-11-arm update e2e tests
- GitHub Check: macos-15 update e2e tests
- GitHub Check: linter, formatters
- GitHub Check: macOS
- GitHub Check: smoke e2e tests (production)
- GitHub Check: build website
- GitHub Check: unit tests / windows-2025
- GitHub Check: smoke e2e tests (development)
- GitHub Check: Linux
- GitHub Check: unit tests / macos-15
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: typecheck
- GitHub Check: Windows
🔇 Additional comments (2)
.github/workflows/e2e-main.yaml (2)
60-77: New workflow_dispatch inputs look good.The added inputs for update_target_owner, update_target_repo, and update_prerelease are well-scoped with sensible defaults. This makes the workflow flexible without breaking existing usage.
213-222: Windows defaults propagation via GITHUB_ENV looks correct.Good use of $env:GITHUB_ENV and sensible defaults for owner/repo/prerelease.
| exclude: | ||
| - installation: ${{ (github.event.inputs.update_with_extensions && github.event.inputs.update_with_extensions == 'true') && 'N/A' || 'custom-extensions' }} | ||
| steps: | ||
| - name: Set the default env. variables |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
YAML indentation warnings flagged by linter; please verify.
YAMLlint reports wrong indentation on these lines. It might be an artifact, but worth verifying locally to keep CI green.
Run:
Also applies to: 325-325
🏁 Script executed:
#!/bin/bash
pip install yamllint >/dev/null 2>&1 || true
yamllint .github/workflows/e2e-main.yamlLength of output: 5866
Fix YAML lint errors in .github/workflows/e2e-main.yaml
yamllint reports multiple errors/warnings (indentation, line-length, trailing spaces, colon/comment spacing and a truthy-value warning). These must be fixed to keep CI green.
-
File: .github/workflows/e2e-main.yaml — key failures:
- Indentation errors (error): lines 50, 57, 74, 211 (expected 10, found 8).
- Line-length errors (>80 chars) (error): 38, 93, 105, 106, 110, 112, 113, 114, 117, 120, 122, 123, 124, 148, 153, 177, 187, 211, 219, 220, 221, 225, 232, 237, 246, 248, 257, 258, 268, 272, 278, 280, 294, 304, 331, 332, 333, 337, 344, 349, 363, 389, 392, 393, 405, 415.
- Trailing spaces (error): lines 284, 316.
- Too many spaces after colon (error): lines 184, 301, 412.
- Too few spaces before comment (warning): 89, 91, 97, 148, 153, 177, 187, 201, 223, 229, 232, 237, 294, 295, 304, 335, 341, 344, 349, 405, 406, 415.
- Truthy-value warning (use true/false): line 20.
-
The originally flagged snippet (around line ~213) is:
- name: Set the default env. variables
The indentation error at line 211 likely affects this step — fix the surrounding block indentation.
Please address the above issues (fix indentation, shorten/wrap long lines, remove trailing spaces, correct colon/comment spacing and truthy values) and re-run yamllint to verify.
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 213-213: wrong indentation: expected 4 but found 6
(indentation)
🤖 Prompt for AI Agents
.github/workflows/e2e-main.yaml lines ~20–415: yamllint reports multiple style
errors — fix the truthy-value at line 20 by replacing non-boolean with
true/false; correct indentation at lines 50, 57, 74, 211 (ensure the "Set the
default env. variables" step and its surrounding block are indented to the
expected 10 spaces); wrap or shorten all lines flagged for length so no line
exceeds 80 chars (lines listed in report); remove trailing spaces at lines 284
and 316; ensure only one space after colons (fix lines 184, 301, 412) and add a
single space before comments where warned; re-run yamllint to verify no
remaining errors.
| - name: Build Podman Desktop locally, mac dmg, ${{ env.UPDATE_PRERELEASE == 'true' && 'Allow Prerelease' || 'Using Latest' }} | ||
| env: | ||
| ELECTRON_ENABLE_INSPECT: true | ||
| run: | | ||
| if [[ "${{ env.UPDATE_PRERELEASE }}" == "true" ]]; then | ||
| echo "Allow Prerelease when updating, setting it inline in the code" | ||
| sed -i '' '/autoUpdater.autoDownload = false;/a \ | ||
| autoUpdater.allowPrerelease = true; \ | ||
| ' packages/main/src/plugin/updater.ts | ||
| fi | ||
| pnpm compile:current --mac dmg |
There was a problem hiding this comment.
Bash condition uses expression context instead of runtime env; prerelease gating may never trigger on macOS.
Within run:, ${{ env.* }} is evaluated before the step starts and won’t see values written via GITHUB_ENV. Use the shell env var instead.
Apply this diff:
- if [[ "${{ env.UPDATE_PRERELEASE }}" == "true" ]]; then
+ if [[ "${UPDATE_PRERELEASE}" == "true" ]]; then
echo "Allow Prerelease when updating, setting it inline in the code"
sed -i '' '/autoUpdater.autoDownload = false;/a \
autoUpdater.allowPrerelease = true; \
' packages/main/src/plugin/updater.ts
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Build Podman Desktop locally, mac dmg, ${{ env.UPDATE_PRERELEASE == 'true' && 'Allow Prerelease' || 'Using Latest' }} | |
| env: | |
| ELECTRON_ENABLE_INSPECT: true | |
| run: | | |
| if [[ "${{ env.UPDATE_PRERELEASE }}" == "true" ]]; then | |
| echo "Allow Prerelease when updating, setting it inline in the code" | |
| sed -i '' '/autoUpdater.autoDownload = false;/a \ | |
| autoUpdater.allowPrerelease = true; \ | |
| ' packages/main/src/plugin/updater.ts | |
| fi | |
| pnpm compile:current --mac dmg | |
| - name: Build Podman Desktop locally, mac dmg, ${{ env.UPDATE_PRERELEASE == 'true' && 'Allow Prerelease' || 'Using Latest' }} | |
| env: | |
| ELECTRON_ENABLE_INSPECT: true | |
| run: | | |
| if [[ "${UPDATE_PRERELEASE}" == "true" ]]; then | |
| echo "Allow Prerelease when updating, setting it inline in the code" | |
| sed -i '' '/autoUpdater.autoDownload = false;/a \ | |
| autoUpdater.allowPrerelease = true; \ | |
| ' packages/main/src/plugin/updater.ts | |
| fi | |
| pnpm compile:current --mac dmg |
…odman-desktop#13608) Signed-off-by: Ondrej Dockal <odockal@redhat.com>
What does this PR do?
Allows to parametrize the e2e-tests-main workflow so that we can run various update-e2e test combinations.
Screenshot / video of UI
What issues does this PR fix or reference?
#13405
How to test this PR?
One can run the workflow from the own fork, but must comment every job if part.
I have enable it on my fork: https://github.com/odockal/podman-desktop/actions/runs/16969669892