quality/fedora_openqa

Fork 0

older-media-writer #142

Merged

adamwill merged 1 commit from older-media-writer into main

2026-05-30 04:32:11 +00:00

lruzicka commented

2026-05-20 13:03:52 +00:00

Owner

No description provided.

lruzicka added 2 commits

2026-05-20 13:03:52 +00:00

Provide fmw_source contextual variable

CI via Tox / tox (pull_request) Successful in 1m58s

Details

CI via Tox / checkwiki (pull_request) Successful in 48s

Details

e77a5ced0d

Previously, we would use the $VERSION$ variable to
create a header string, such as Fedora 44, that would
refer to a column in the test matrices. However,
how the variable was provided made us unable to
report to a column such as Flathub.

This adds a different mechanism that provides
a correct string based on the environment where
the test is running, be it Fedora XX or Flathub, to
the report.py and modifies the tests in test_report.py
to cover this change.

Fixes: quality/os-autoinst-distri-fedora#529

report: handle stable and legacy Fedora Media Writer sources

CI via Tox / tox (pull_request) Has been cancelled

Details

CI via Tox / checkwiki (pull_request) Has been cancelled

Details

ea3f1e0023

Fedora Media Writer validation can now test multiple Fedora release
sources, not only the current compose release and Flathub. Derive the
wiki result column from the test name so that stable and legacy variants
report to the previous Fedora releases:

- media_writer_stable reports to Fedora N-1
- media_writer_legacy reports to Fedora N-2
- CANNED jobs continue to report to Flathub

Add coverage for the new Fedora Media Writer source replacement logic in
test_uniqueres_replacements.

lruzicka force-pushed older-media-writer from ea3f1e0023

CI via Tox / tox (pull_request) Has been cancelled

Details

CI via Tox / checkwiki (pull_request) Has been cancelled

Details

to 205af3666b

CI via Tox / tox (pull_request) Successful in 2m4s

Details

CI via Tox / checkwiki (pull_request) Successful in 47s

Details

AI Code Review / ai-review (pull_request_target) Successful in 24s

Details

2026-05-20 13:05:35 +00:00

Compare

forgejo-actions commented

2026-05-20 13:08:58 +00:00

AI Code Review

Detailed Code Review

The implementation correctly maps the new 'stable' and 'legacy' test names to their appropriate test suites and computes the expected Fedora versions for the test reports. The unit tests are well-structured and cover the new edge cases effectively. However, a potential critical issue exists in report.py regarding the handling of non-numeric Fedora versions (such as 'Rawhide' or 'Branched'), which could cause unhandled exceptions during execution.

📂 File Reviews

📄 `src/fedora_openqa/report.py` - Updated logic to calculate the target Fedora version based on test name suffixes.

Critical [Bug]: The unconditional conversion of version to an integer via int(version) will raise a ValueError if the version string is non-numeric, such as 'Rawhide', 'ELN', or 'Branched'. This will crash the report generation for these branches. You should handle cases where the version cannot be parsed into an integer.
- Suggestion: Wrap the integer conversion and math logic in a try-except block to gracefully handle non-numeric versions, falling back to the original string version.

testname = job["test"]
try:
    vm_version = int(version)
    if testname.endswith("stable"):
        vm_version -= 1
    elif testname.endswith("legacy"):
        vm_version -= 2
    fmw_source = f"Fedora {vm_version}"
except ValueError:
    fmw_source = f"Fedora {version}"

✅ Summary

Overall Assessment: One critical issue identified regarding unhandled exceptions on non-numeric version strings, which must be fixed to prevent crashes during report generation.
Priority Issues:
- Address the ValueError crash risk in report.py when version contains non-numeric values like 'Rawhide'.

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### Detailed Code Review The implementation correctly maps the new 'stable' and 'legacy' test names to their appropriate test suites and computes the expected Fedora versions for the test reports. The unit tests are well-structured and cover the new edge cases effectively. However, a potential critical issue exists in `report.py` regarding the handling of non-numeric Fedora versions (such as 'Rawhide' or 'Branched'), which could cause unhandled exceptions during execution. #### 📂 File Reviews <details> <summary><strong>📄 `src/fedora_openqa/report.py`</strong> - Updated logic to calculate the target Fedora version based on test name suffixes.</summary> - **Critical** [Bug]: The unconditional conversion of `version` to an integer via `int(version)` will raise a `ValueError` if the version string is non-numeric, such as 'Rawhide', 'ELN', or 'Branched'. This will crash the report generation for these branches. You should handle cases where the version cannot be parsed into an integer. - *Suggestion:* Wrap the integer conversion and math logic in a try-except block to gracefully handle non-numeric versions, falling back to the original string version. ```` testname = job["test"] try: vm_version = int(version) if testname.endswith("stable"): vm_version -= 1 elif testname.endswith("legacy"): vm_version -= 2 fmw_source = f"Fedora {vm_version}" except ValueError: fmw_source = f"Fedora {version}" ```` </details> ### ✅ Summary - **Overall Assessment:** One critical issue identified regarding unhandled exceptions on non-numeric version strings, which must be fixed to prevent crashes during report generation. - **Priority Issues:** - Address the `ValueError` crash risk in `report.py` when `version` contains non-numeric values like 'Rawhide'. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*

lruzicka commented

2026-05-20 13:15:58 +00:00

Author

Owner

The Rawhide thing is handled a couple of lines above the place I use version, so I guess it it safe to assume that it has a numeric value.

The `Rawhide` thing is handled a couple of lines above the place I use `version`, so I guess it it safe to assume that it has a numeric value.