Add pyartcd unit tests for update-golang pipeline

[vfreex]

No description provided.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign lgarciaaco for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Walkthrough

This pull request adds a comprehensive test suite for the golang update pipeline. The new test module validates utility functions for extracting and validating golang NVRs, retrieving latest builds from Koji, checking build availability, and moving golang bugs, as well as extensive tests for the UpdateGolangPipeline class covering initialization, validation, build processing, tagging logic, and integration with Koji and Konflux systems.

Changes

Cohort / File(s)	Summary
New golang update pipeline test suite pyartcd/tests/pipelines/test_update_golang.py	Adds comprehensive unit tests for golang update pipeline including tests for NVR extraction/validation, latest build retrieval, build availability checking, bug movement operations, and UpdateGolangPipeline class (initialization, validation, build processing, tagging, and Koji/Konflux integration) with extensive mocking of external dependencies and both async and sync code paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Mock configuration verification: Ensure Koji session mocks, Elliott interactions, and subprocess stubs accurately reflect production behavior and don't mask real issues
• Test coverage validation: Confirm that all tested code paths (async operations, Brew/Konflux branches, dry-run scenarios, el8/el9 variations) exercise the intended pipeline logic
• Error scenario completeness: Verify that error conditions and boundary cases are properly tested with appropriate assertions
• Fixture and setup consistency: Review consistency across test setups, especially for environment variables, kubeconfig handling, and command construction validation

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

@vfreex: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/security	fa18370	link	false	/test security

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

pyartcd/tests/pipelines/test_update_golang.py (2)

260-377: Tidy up unused patched arguments and helper params (Ruff ARG001/ARG002)

Ruff’s ARG001/ARG002 hints are all about unused parameters introduced by @patch and the async helper:

• Methods like test_init_brew_build_system, test_init_konflux_build_system, test_brew_login_*, test_get_content_repo_url, test_get_module_tag*, test_process_build_*, test_tag_build_*, test_rebase_*, test_build_*, test_rebase_and_build_* have an unused mock_konflux_db argument injected by @patch("pyartcd.pipelines.update_golang.KonfluxDb").
• The async generator mock_search_builds(*args, **kwargs) doesn’t use args / kwargs.

To keep the patching behavior while silencing these warnings, you can:

• Rename unused patch parameters to start with an underscore (Ruff treats them as intentionally unused), e.g.:

-    @patch("pyartcd.pipelines.update_golang.KonfluxDb")
-    def test_init_brew_build_system(self, mock_konflux_db):
+    @patch("pyartcd.pipelines.update_golang.KonfluxDb")
+    def test_init_brew_build_system(self, _mock_konflux_db):

• Likewise for the async helper:

-        async def mock_search_builds(*args, **kwargs):
+        async def mock_search_builds(*_args, **_kwargs):
             yield mock_build_record

This keeps the tests’ behavior identical while making Ruff happy.

Also applies to: 391-521, 670-748, 738-741, 754-803, 786-808, 811-838, 893-945, 948-995

---

248-995: Optional: address /tmp/working and large test class warnings (S108, R0904)

Two static-analysis hints are low‑impact but worth noting:

• Multiple tests use Path("/tmp/working") as a dummy working directory. Since these tests don’t actually touch the filesystem, the S108 “insecure temp dir” warning is mostly noise; if you want to appease it, you could switch to something obviously non‑real (e.g. Path("/nonexistent/working")) or use tempfile.TemporaryDirectory() within the test.
• TestUpdateGolangPipeline has many test methods, triggering Pylint’s “too many public methods” (R0904). If this becomes annoying, you could split it into smaller classes grouped by concern (e.g. TestInit, TestBrewOps, TestKonfluxOps, TestTagging), which may also improve readability.

Both are cosmetic and can be deferred if your linters aren’t blocking on them.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 859cd00 and fa18370.

📒 Files selected for processing (1)

• pyartcd/tests/pipelines/test_update_golang.py (1 hunks)

🧰 Additional context used

🪛 Pylint (4.0.3)

pyartcd/tests/pipelines/test_update_golang.py

[error] 1-1: Unrecognized option found: suggestion-mode

(E0015)

---

[refactor] 248-248: Too many public methods (28/20)

(R0904)

🪛 Ruff (0.14.5)

pyartcd/tests/pipelines/test_update_golang.py

261-261: Unused method argument: mock_konflux_db

(ARG002)

---

265-265: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

297-297: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

321-321: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

337-337: Unused method argument: mock_konflux_db

(ARG002)

---

341-341: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

359-359: Unused method argument: mock_konflux_db

(ARG002)

---

363-363: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

391-391: Unused method argument: mock_konflux_db

(ARG002)

---

395-395: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

417-417: Unused method argument: mock_konflux_db

(ARG002)

---

421-421: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

443-443: Unused method argument: mock_konflux_db

(ARG002)

---

447-447: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

467-467: Unused method argument: mock_konflux_db

(ARG002)

---

471-471: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

496-496: Unused method argument: mock_konflux_db

(ARG002)

---

500-500: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

524-524: Unused method argument: mock_konflux_db

(ARG002)

---

529-529: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

550-550: Unused method argument: mock_konflux_db

(ARG002)

---

555-555: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

574-574: Unused method argument: mock_konflux_db

(ARG002)

---

578-578: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

605-605: Unused method argument: mock_konflux_db

(ARG002)

---

609-609: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

633-633: Unused method argument: mock_konflux_db

(ARG002)

---

637-637: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

670-670: Unused method argument: mock_konflux_db

(ARG002)

---

674-674: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

715-715: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

738-738: Unused function argument: args

(ARG001)

---

738-738: Unused function argument: kwargs

(ARG001)

---

754-754: Unused method argument: mock_konflux_db

(ARG002)

---

758-758: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

786-786: Unused method argument: mock_konflux_db

(ARG002)

---

790-790: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

811-811: Unused method argument: mock_konflux_db

(ARG002)

---

815-815: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

841-841: Unused method argument: mock_konflux_db

(ARG002)

---

845-845: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

867-867: Unused method argument: mock_konflux_db

(ARG002)

---

871-871: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

893-893: Unused method argument: mock_konflux_db

(ARG002)

---

897-897: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

922-922: Unused method argument: mock_konflux_db

(ARG002)

---

926-926: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

948-948: Unused method argument: mock_konflux_db

(ARG002)

---

952-952: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

---

973-973: Unused method argument: mock_konflux_db

(ARG002)

---

977-977: Probable insecure usage of temporary file or directory: "/tmp/working"

(S108)

🔇 Additional comments (9)

pyartcd/tests/pipelines/test_update_golang.py (9)

24-87: Strong coverage for extract_and_validate_golang_nvrs validations

The tests here do a solid job covering both the happy paths and a wide range of error conditions (OCP version, package name, EL mapping, duplicates, and “too many NVRs”) with precise message expectations. No issues from a test‑logic perspective.

---

89-111: get_latest_nvr_in_tag tests match expected Koji interaction

Both “found” and “not found” cases are exercised, and the assertion on listTagged arguments (including latest=True and inherit=False) looks correct for this helper.

---

114-143: is_latest_build tests exercise key edge cases

Latest, non-latest, and “no latest found” behaviors are all validated, including the tag name and package passed to getLatestBuilds. Looks consistent with how such a helper should behave.

---

145-183: Async is_latest_and_available tests look correct and robust

The tests cleanly cover: latest+available, not latest (short‑circuited), and latest but unavailable (non‑zero rc). Decorator order vs. argument order is correct, and the expectations on is_latest_build and cmd_gather_async invocations make sense.

---

185-246: move_golang_bugs command construction is well covered

These tests nicely pin down the elliott CLI arguments for CVEs, tracker updates, and dry‑run behavior, while still allowing flexibility for other options via assertIn. No issues spotted.

---

248-521: Comprehensive UpdateGolangPipeline init and helper tests

The tests in this block give good confidence in constructor behavior (brew vs konflux, env var requirements, kubeconfig/data_path/gitref handling), helper methods (get_golang_branch, brew_login, content repo URL, module tag lookup), and process_build branching. Mocks are used appropriately to isolate behavior from real Koji/Konflux/Elliott interactions.

---

574-668: tag_build tests cover dry‑run, el9, and el8+module paths well

The three scenarios (dry‑run, simple el9 override, and el8 with module builds) exercise the main branches of the tagging logic, including ensuring nothing is tagged in dry‑run, correct override tag for el9, and multiple tag operations for el8/module builds. Behavior and expectations look consistent with the intended workflow.

---

670-751: Existing-builder lookup tests for Brew and Konflux are well structured

Both Brew and Konflux paths are exercised: Koji listBuilds + elliott for Brew, and KonfluxDb async search + elliottutil for Konflux. The tests assert the final mapping of EL version to builder NVRs rather than over‑specifying intermediate calls, which keeps them resilient to internal refactors.

---

754-995: Rebase/build orchestration tests capture key command semantics

The _rebase_*, _build_*, and combined _rebase_and_build_* tests ensure:

• Doozer/konflux commands are invoked with the right group/branch, version, and push/dry‑run flags.
• Dry‑run behavior affects the presence of --push / --dry-run rather than skipping the commands entirely.
• Combined helpers invoke both rebase and build (validated via call counts).

This is a solid level of coverage for these orchestration helpers.