feat: honor skip_install by not installing Copr artifacts in Testing Farm - #3190
Conversation
|
✔️ pre-commit SUCCESS in 1m 56s |
feat: add skip_install option for test jobs Add a 'skip_install' configuration option next to 'skip_build' for Testing Farm test jobs. When enabled, the Copr build still runs and its artifacts are attached to the Testing Farm request, but their packages are not installed automatically (Testing Farm receives 'install: false'). Installing the desired components is left to the tmt plan/tests. This option is independent of 'skip_build'. Fixes #2775 Assisted-by: Claude Opus 4.8 noreply@anthropic.com TODO: Write new tests or update the old ones to cover new functionality. Update doc-strings where appropriate. Update or write new documentation in packit/packit.dev. packit/packit-service#3190 packit/packit.dev#1174 RELEASE NOTES BEGIN New skip_install option for test jobs to omit automatic installation of COPR packages on Testing Farm runner. RELEASE NOTES END Warning: All tests pass, but I do not know how to perform end-to-end testing, so I have not been able to actually try it. Reviewed-by: Nikola Forró
|
recheck |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 54s |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 5:26 PM UTC · Completed 5:39 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.95 |
|
Risk Assessment: low (1/5) DetailsLow-risk feature addition: small 2-file change with 50% test coverage ratio, well-scoped to a single linked feature issue, no protected or security-sensitive paths touched, stable git history with no recent fixes or reverts. Previous runRisk Assessment: low (1/5) DetailsLow-risk, well-scoped feature addition behind an explicit opt-in configuration flag. Only 2 files changed with excellent test coverage ratio (0.50). No protected paths, security-sensitive files, CI workflows, or dependencies touched. Known contributor with a clear linked issue. |
|
Looks good to me Previous runReviewFindingsLow
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…Farm When a test job sets 'skip_install', attach all Copr artifacts to the Testing Farm request with 'install: false'. Testing Farm then makes the builds available in the test-artifacts repository but does not install their packages, leaving installation to the tmt plan/tests. This applies to the main build and additional builds from other PRs, and is independent of 'skip_build'. Related to packit/packit#2775 Assisted-by: Claude Opus 4.8 <noreply@anthropic.com>
308d99f to
372a91d
Compare
|
Fixed. |
|
❌ pre-commit FAILURE in 1m 57s |
Widening the artifact dict's value type to include bool (for the new `install` key) broke mypy on the existing `for package in artifact["packages"]` loop, since bool isn't iterable. A TypedDict gives each key its correct fixed type instead of a shared Union. ``` packit_service/worker/helpers/testing_farm.py:540: error: Item "bool" of "Union[list[str], str, bool]" has no attribute "__iter__" (not iterable) [union-attr] ``` Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 59s |
|
Fixed. |
@pbrezina Just an FYI, I noticed the pair of the PRs (the feature here and docs) yesterday after the deployment, so it’s safe to use the config option, but the feature itself will probably land next Monday/Tuesday. I could probably run one more deployment if it is urgent. |
|
@mfocko Hi, if these gets merged soon, I would certainly appreciate a quick deployment, if it is not too much troubles. Otherwise, next week is good. Thank you! |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 6:36 AM UTC · Completed 6:49 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.28 |
Superseded by updated review
fullsend-ai-review
Bot
added
the
ready-for-merge
Related to packit/packit#2775 Assisted-by: Claude Opus 4.8 <noreply@anthropic.com> <!-- TODO list --> TODO: - [x] Write new tests or update the old ones to cover new functionality. - [x] Update doc-strings where appropriate. - [x] Update or write new documentation in `packit/packit.dev`. <!-- notes for reviewers --> <!-- Links to other issues or pull requests, for cross-repository links use: ‹namespace›/‹repository›#‹ID of issue› (‹namespace›/‹repository›!‹ID of PR› respectively) --> * packit/packit-service#3190 * packit/packit#2776 <!-- release notes footer --> RELEASE NOTES BEGIN New `skip_install` option for test jobs to omit automatic installation of COPR packages on Testing Farm runner. RELEASE NOTES END Warning: All tests pass, but I do not know how to perform end-to-end testing, so I have not been able to actually try it.
4 participants
When a test job sets 'skip_install', attach all Copr artifacts to the
Testing Farm request with 'install: false'. Testing Farm then makes the
builds available in the test-artifacts repository but does not install
their packages, leaving installation to the tmt plan/tests. This applies
to the main build and additional builds from other PRs, and is
independent of 'skip_build'.
Related to packit/packit#2775
Assisted-by: Claude Opus 4.8 noreply@anthropic.com
TODO:
packit/packit.dev.RELEASE NOTES BEGIN
New
skip_installoption for test jobs to omit automatic installation ofCOPR packages on Testing Farm runner.
RELEASE NOTES END
Warning: All tests pass, but I do not know how to perform end-to-end testing, so I have not been able to actually try it.