-
Notifications
You must be signed in to change notification settings - Fork 60
Build comment from fields of Koji build target #3174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,6 +73,10 @@ def test_logdetective_koji_build_scratch_downstream( | |
| flexmock(ServiceConfig).should_receive("get_service_config").and_return(service_config) | ||
|
|
||
| koji_build_pr_downstream.target = "rawhide" | ||
| koji_build_pr_downstream.nvr = "packit-0.123.0-1.fc00" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] test-inadequate The integration test sets new attributes (build_submission_stdout, nvr, scratch, sidetag) on koji_build_pr_downstream but the requests.post mock accepts any arguments, so the dynamic commentary payload is never validated end-to-end. Additionally, db_project_object resolves to a flexmock that will fail isinstance(db_project_object, PullRequestModel) (flexmock objects are not instances of the real model class), causing the PR-specific commentary line to be silently skipped. Suggested fix: Add a payload assertion on the requests.post mock to verify the commentary string, or use flexmock(PullRequestModel) to ensure isinstance checks work correctly in tests. |
||
| koji_build_pr_downstream.scratch = True | ||
| koji_build_pr_downstream.sidetag = None | ||
| koji_build_pr_downstream.build_submission_stdout = "MOCK STDOUT" | ||
| flexmock(koji.result.Task).should_receive("get_packages_config").and_return(None) | ||
| flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").and_return( | ||
| koji_build_pr_downstream | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[low] edge-case
_build_commentary accesses self.koji_event.build_model and immediately dereferences build.nvr without a None guard. If build_model were None, this would raise AttributeError. However, this mirrors the existing pattern in trigger_log_detective_analysis_for_arch and is not practically reachable — the handler returns early if the build model is absent.