Run desktop smoke tests by default for release builds#8895
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new manually triggered App Smoke Tests workflow with version preparation and platform-specific Linux/Windows smoke-test jobs, updates release workflow smoke-test behavior, and extends reusable build workflow secret inputs. ChangesCI Smoke Test Workflows
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the release workflow to run Windows desktop smoke tests by default (including on scheduled/tag-driven release builds), and adds a standalone, manually-triggered workflow to run Linux/Windows app smoke tests via the existing reusable build workflows.
Changes:
- Enable Windows connect/disconnect smoke tests by default in
.github/workflows/release.yml(and run them automatically for non-manual triggers). - Enable Windows config-URL smoke in release builds (previously hard-disabled).
- Add a new
.github/workflows/app-smoke-tests.ymlworkflow_dispatch entrypoint to run Linux/Windows smoke builds/tests on demand.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/release.yml | Defaults Windows smoke testing on and enables config-URL smoke for non-dispatch release runs. |
| .github/workflows/app-smoke-tests.yml | Adds a manual workflow to run app smoke tests on Linux/Windows via existing reusable build workflows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/app-smoke-tests.yml (1)
76-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting version-prep logic to avoid duplication with release.yml.
This version-computation and pubspec.yaml patching logic (nightly version generation,
+GITHUB_RUN_NUMBERbuild suffix, artifact upload) closely mirrors thepreparejob pattern likely already present inrelease.yml. Extracting it into a shared composite action or reusable workflow would prevent the two prepare implementations from drifting apart over time.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/app-smoke-tests.yml around lines 76 - 96, The version-prep step in the meta job duplicates the same nightly version generation and pubspec.yaml patching flow used elsewhere, so extract that logic into a shared composite action or reusable workflow and have this job call it instead. Keep the behavior around VERSION, BASE_VERSION, FULL_VERSION, INSTALLER_BASE_NAME, and the pubspec.yaml update/upload consistent by reusing the shared implementation rather than maintaining separate copies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/app-smoke-tests.yml:
- Around line 100-101: The smoke-test workflow jobs are inheriting every
repository secret via the reusable workflow call, which is broader than needed.
Update the call sites that use ./.github/workflows/build-linux.yml in the
app-smoke-tests workflow to pass an explicit secrets map instead of secrets:
inherit, including only the downstream secrets actually consumed (for example
JOIN_SERVER_CONFIG_URLS and any truly required signing-related values). Keep the
change scoped to the linux/windows job definitions so the reusable workflow
still receives the minimum necessary credentials.
- Around line 70-74: The Checkout repo step using actions/checkout@v4 should
disable credential persistence to avoid leaving the GITHUB_TOKEN in git config
for the rest of the job. Update the checkout configuration by setting
persist-credentials to false alongside the existing fetch-depth and fetch-tags
options, keeping the fix within the workflow job that contains this checkout
step.
- Around line 55-57: The workflow-level permissions are broader than needed
because `id-token: write` is never used when this smoke-test workflow calls the
reusable Windows job with `skip_signing: true`. Remove the `id-token: write`
entry from the top-level permissions in the app-smoke-tests workflow, leaving
only the minimum required scope so the reusable `build-windows.yml` job cannot
request a broader token than the caller allows.
In @.github/workflows/release.yml:
- Around line 393-395: Add a separate workflow_dispatch input for
windows_config_url_smoke so config-URL smoke can be controlled independently
from windows_connect_smoke. Update the release workflow’s run_config_url_smoke
expression to read github.event.inputs.windows_config_url_smoke instead of
reusing windows_connect_smoke, and keep the naming aligned with the
corresponding inputs used in app-smoke-tests.yml and build-windows.yml.
---
Nitpick comments:
In @.github/workflows/app-smoke-tests.yml:
- Around line 76-96: The version-prep step in the meta job duplicates the same
nightly version generation and pubspec.yaml patching flow used elsewhere, so
extract that logic into a shared composite action or reusable workflow and have
this job call it instead. Keep the behavior around VERSION, BASE_VERSION,
FULL_VERSION, INSTALLER_BASE_NAME, and the pubspec.yaml update/upload consistent
by reusing the shared implementation rather than maintaining separate copies.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d0bb455-a517-4e5d-a168-e32c98cab745
📒 Files selected for processing (2)
.github/workflows/app-smoke-tests.yml.github/workflows/release.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
393-393: 🧹 Nitpick | 🔵 TrivialConnect smoke now runs unconditionally for all non-manual-dispatch triggers.
Previously connect/disconnect smoke only ran on manual dispatch when explicitly enabled; now it runs for every non-
workflow_dispatchevent (push/schedule/release triggers) with no opt-out, and additionally on manual dispatch when the input is true. This matches the PR's stated goal of enabling smoke tests by default for release builds, but every automated windows release build will now incur the added smoke-test runtime/cost with no per-run override for those trigger types.Confirm this added CI time for the Windows release pipeline is acceptable/expected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml at line 393, The Windows release workflow in the release pipeline now runs the connect/disconnect smoke path unconditionally for every non-`workflow_dispatch` trigger via run_connect_smoke, which adds unavoidable CI time and cost. Review the release.yml logic around run_connect_smoke and confirm this default-on behavior is intentional; if not, restore a per-trigger opt-out or gate it behind an explicit input so automated push/schedule/release runs can skip the smoke job when needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/release.yml:
- Line 393: The Windows release workflow in the release pipeline now runs the
connect/disconnect smoke path unconditionally for every non-`workflow_dispatch`
trigger via run_connect_smoke, which adds unavoidable CI time and cost. Review
the release.yml logic around run_connect_smoke and confirm this default-on
behavior is intentional; if not, restore a per-trigger opt-out or gate it behind
an explicit input so automated push/schedule/release runs can skip the smoke job
when needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 794a4fc2-1dd3-4728-8993-58e60ac321e6
📒 Files selected for processing (2)
.github/workflows/app-smoke-tests.yml.github/workflows/release.yml
Summary by CodeRabbit