Provide fmw_source contextual variable #140

lruzicka · 2026-05-11T15:36:47Z

lruzicka commented

2026-05-11 15:36:47 +00:00

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

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

lruzicka added 2 commits

2026-05-11 15:36:47 +00:00

Add gnome_initial_setup test suite without reporting 7be0c7bcca

Add a special media writer test case to enable reporting

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

Details

CI via Tox / checkwiki (pull_request) Failing after 45s

Details

202a340974

This adds a special variant of the test case to report
Flathub media writer results because the current reports
to Fedora versions and this new reports to Flathub column.

lruzicka force-pushed report_flathub_mediawriter from 202a340974

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

Details

CI via Tox / checkwiki (pull_request) Failing after 45s

Details

to 101a685048

CI via Tox / tox (pull_request) Failing after 1m55s

Details

CI via Tox / checkwiki (pull_request) Failing after 45s

Details

2026-05-11 16:02:48 +00:00

Compare

lruzicka force-pushed report_flathub_mediawriter from 101a685048

CI via Tox / tox (pull_request) Failing after 1m55s

Details

CI via Tox / checkwiki (pull_request) Failing after 45s

Details

to 4bf6b75882

CI via Tox / tox (pull_request) Failing after 1m56s

Details

CI via Tox / checkwiki (pull_request) Failing after 44s

Details

2026-05-11 16:12:06 +00:00

Compare

lruzicka force-pushed report_flathub_mediawriter from 4bf6b75882

CI via Tox / tox (pull_request) Failing after 1m56s

Details

CI via Tox / checkwiki (pull_request) Failing after 44s

Details

to b6ac282ef3

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

Details

CI via Tox / checkwiki (pull_request) Failing after 45s

Details

2026-05-12 08:43:13 +00:00

Compare

lruzicka force-pushed report_flathub_mediawriter from b6ac282ef3

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

Details

CI via Tox / checkwiki (pull_request) Failing after 45s

Details

to 6c0c2edac3

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

Details

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

Details

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

Details

2026-05-12 08:51:02 +00:00

Compare

forgejo-actions commented

2026-05-12 09:35:09 +00:00

AI Code Review

📋 MR Summary

This MR adds a new media_writer_flatpak test suite and updates the reporting logic to use Flathub as the environment source for this specific suite, while falling back to Fedora <VERSION> for the default media writer test.

Key Changes:
- Added gnome_initial_setup (empty) and media_writer_flatpak to TESTSUITES in conf_test_suites.py.
- Replaced hardcoded Fedora $VERSION$ with $FMW_SOURCE$ for QA:Testcase_USB_fmw in conf_test_suites.py.
- Updated _uniqueres_replacements in report.py to substitute $FMW_SOURCE$ with either 'Flathub' (if test is media_writer_flatpak) or 'Fedora '.
- Added unit tests for the new media_writer_flatpak and default media_writer FMW source replacement logic.
Impact: src/fedora_openqa/conf_test_suites.py, src/fedora_openqa/report.py, tests/test_report.py
Risk Level: 🟢 Low - The changes only affect reporting logic for a specific test suite and include unit tests validating the intended behavior.

Detailed Code Review

The implementation correctly separates the environment reporting string based on the test type for media writer test cases. The changes are straightforward and well-tested. One minor issue is a typo in the documentation string for test_uniqueres_replacements_media_writer_default.

📂 File Reviews

📄 `tests/test_report.py` - Added tests for the new FMW source replacement logic.

Minor [Documentation]: The docstring for test_uniqueres_replacements_media_writer_default incorrectly states media_writer_flatpak instead of media_writer.
- Suggestion: Update the docstring to correctly refer to the media_writer test suite.

```python
def test_uniqueres_replacements_media_writer_default():
    """ media_writer should use Fedora VERSION as the FMW source """
```

✅ Summary

Overall Assessment: The changes successfully implement the desired reporting behavior. No critical issues were identified.
Minor Suggestions:
- Fix the docstring typo in tests/test_report.py.

🤖 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 ### 📋 MR Summary This MR adds a new `media_writer_flatpak` test suite and updates the reporting logic to use `Flathub` as the environment source for this specific suite, while falling back to `Fedora <VERSION>` for the default media writer test. - **Key Changes:** - Added `gnome_initial_setup` (empty) and `media_writer_flatpak` to `TESTSUITES` in `conf_test_suites.py`. - Replaced hardcoded `Fedora $VERSION$` with `$FMW_SOURCE$` for `QA:Testcase_USB_fmw` in `conf_test_suites.py`. - Updated `_uniqueres_replacements` in `report.py` to substitute `$FMW_SOURCE$` with either 'Flathub' (if test is `media_writer_flatpak`) or 'Fedora <version>'. - Added unit tests for the new `media_writer_flatpak` and default `media_writer` FMW source replacement logic. - **Impact:** src/fedora_openqa/conf_test_suites.py, src/fedora_openqa/report.py, tests/test_report.py - **Risk Level:** 🟢 Low - The changes only affect reporting logic for a specific test suite and include unit tests validating the intended behavior. ### Detailed Code Review The implementation correctly separates the environment reporting string based on the test type for media writer test cases. The changes are straightforward and well-tested. One minor issue is a typo in the documentation string for `test_uniqueres_replacements_media_writer_default`. #### 📂 File Reviews <details> <summary><strong>📄 `tests/test_report.py`</strong> - Added tests for the new FMW source replacement logic.</summary> - **Minor** [Documentation]: The docstring for `test_uniqueres_replacements_media_writer_default` incorrectly states `media_writer_flatpak` instead of `media_writer`. - *Suggestion:* Update the docstring to correctly refer to the `media_writer` test suite. ```` ```python def test_uniqueres_replacements_media_writer_default(): """ media_writer should use Fedora VERSION as the FMW source """ ``` ```` </details> ### ✅ Summary - **Overall Assessment:** The changes successfully implement the desired reporting behavior. No critical issues were identified. - **Minor Suggestions:** - Fix the docstring typo in `tests/test_report.py`. --- 🤖 **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 added 1 commit

2026-05-12 09:44:14 +00:00

Fix comment typo as AI requested

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

Details

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

Details

b991c07fec

lruzicka commented

2026-05-12 09:44:28 +00:00

Not squashed yet.

lruzicka changed title from ~~(WIP) Add Silverblue related tests to reporting~~ to Add Silverblue related tests to reporting

2026-05-12 09:44:47 +00:00

adamwill reviewed

2026-05-12 17:07:06 +00:00

src/fedora_openqa/report.py Outdated

					
				@ -73,0 +74,4 @@

				    # to report to the Flathub column (which does not have version in it).

				    # For Flatpak media_writer test, take "Flathub" instead of version.

				    fmw_source = f"Fedora {version}"

				    if job["test"] == "media_writer_flatpak":

if we just change this to checking the CANNED variable, we don't need to duplicate the test in o-a-d-f at all.

if we just change this to checking the `CANNED` variable, we don't need to duplicate the test in o-a-d-f at all.

adamwill commented

2026-05-12 17:07:52 +00:00

Oh, so now I see you more or less did this the way I suggested - but then we don't need to duplicate the test under a different name at all, with one simple tweak...

Oh, so now I see you more or less did this the way [I suggested](https://forge.fedoraproject.org/quality/os-autoinst-distri-fedora/issues/529#issuecomment-710569) - but then we don't need to duplicate the test under a different name at all, with one simple tweak...

lruzicka commented

2026-05-12 21:35:08 +00:00

@adamwill wrote in #140 (comment):

Oh, so now I see you more or less did this the way I suggested - but then we don't need to duplicate the test under a different name at all, with one simple tweak...

Just to be sure. My solution here could stay if I look for CANNED instead of job['test']? And I will return the templates to its original state. Or do you want me to do it exactly like in your suggestion?

@adamwill wrote in https://forge.fedoraproject.org/quality/fedora_openqa/pulls/140#issuecomment-710588: > Oh, so now I see you more or less did this the way [I suggested](https://forge.fedoraproject.org/quality/os-autoinst-distri-fedora/issues/529#issuecomment-710569) - but then we don't need to duplicate the test under a different name at all, with one simple tweak... Just to be sure. My solution here could stay if I look for CANNED instead of job['test']? And I will return the templates to its original state. Or do you want me to do it exactly like in your suggestion?

adamwill reviewed

2026-05-12 23:14:53 +00:00

tests/test_report.py Outdated

					
				@ -133,6 +133,44 @@ def test_uniqueres_replacements(jobdict01):

				        ret = fosreport._uniqueres_replacements(jobdict01, basedict)

				    assert ret['base_section'] == 'RPM-based non-blocking environments (x86_64)'

				def test_uniqueres_replacements_media_writer_flatpak():

I would prefer you just add the tests to the big existing test_uniqueres_replacements test, following the existing patterns, than add another 40 lines just to test two things.

I would prefer you just add the tests to the big existing `test_uniqueres_replacements` test, following the existing patterns, than add another 40 lines just to test two things.

adamwill commented

2026-05-12 23:16:35 +00:00

Your version is fine, the name of the substitution can be whatever so long as it's understandable. Aside from the comment above, I think this is fine, if we adjust it to just use the existing test case name, and drop the g-i-s commit as I already merged that.

lruzicka added 1 commit

2026-05-13 08:46:54 +00:00

Use the existing replacement test

CI via Tox / tox (pull_request) Failing after 2m2s

Details

CI via Tox / checkwiki (pull_request) Has been cancelled

Details

fcdd5523e5

Previously, there were two extra tests to check for correctness
of the fmw_source variable.
This PR moves the check into the main testing chunk.

lruzicka added 1 commit

2026-05-13 08:49:37 +00:00

Modify version string on CANNED environments

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

Details

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

Details

97ad365739

Previously, we have modified on the specific name of the test,
which required an extra test to be run with the correct name.

This PR changes the behaviour to modify on CANNED environments,
so the extra test will not be necessary.

lruzicka force-pushed report_flathub_mediawriter from 97ad365739

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

Details

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

Details

to 9b8de87940

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

Details

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

Details

2026-05-13 09:41:05 +00:00

Compare

lruzicka force-pushed report_flathub_mediawriter from 9b8de87940

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

Details

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

Details

to b5150f0331

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

Details

CI via Tox / checkwiki (pull_request) Failing after 47s

Details

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

Compare

lruzicka force-pushed report_flathub_mediawriter from b5150f0331

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

Details

CI via Tox / checkwiki (pull_request) Failing after 47s

Details

to 85964499ac

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

Details

CI via Tox / checkwiki (pull_request) Failing after 49s

Details

2026-05-13 11:00:40 +00:00

Compare

lruzicka force-pushed report_flathub_mediawriter from 85964499ac

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

Details

CI via Tox / checkwiki (pull_request) Failing after 49s

Details

to e77a5ced0d

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

Details

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

Details

2026-05-13 11:18:44 +00:00

Compare

adamwill commented

2026-05-13 15:46:03 +00:00

LGTM now, thanks.

adamwill changed title from ~~Add Silverblue related tests to reporting~~ to Provide fmw_source contextual variable

2026-05-13 15:46:36 +00:00

adamwill force-pushed report_flathub_mediawriter from e77a5ced0d

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

Details

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

Details

to bd0b1eeb95

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

Details

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

Details

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

Details

2026-05-13 15:48:32 +00:00

Compare

forgejo-actions commented

2026-05-13 17:00:26 +00:00

AI Code Review

Detailed Code Review

The implementation correctly introduces the fmw_source contextual variable, replacing the hardcoded $VERSION$ string. It appropriately evaluates the CANNED environment flag to determine if the target is Flathub, as discussed in previous reviews. The integration of the tests into the existing test_uniqueres_replacements function adheres perfectly to the project's testing guidelines and prevents unnecessary code duplication. No logical errors, performance bottlenecks, or security vulnerabilities were identified during the review.

✅ Summary

Overall Assessment: Review completed successfully. The code accurately implements the agreed-upon design without introducing new issues.
Minor Suggestions:
- Consider removing the stray blank line introduced in src/fedora_openqa/conf_test_suites.py at line 683 to maintain clean file formatting.

🤖 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 introduces the `fmw_source` contextual variable, replacing the hardcoded `$VERSION$` string. It appropriately evaluates the `CANNED` environment flag to determine if the target is Flathub, as discussed in previous reviews. The integration of the tests into the existing `test_uniqueres_replacements` function adheres perfectly to the project's testing guidelines and prevents unnecessary code duplication. No logical errors, performance bottlenecks, or security vulnerabilities were identified during the review. ### ✅ Summary - **Overall Assessment:** Review completed successfully. The code accurately implements the agreed-upon design without introducing new issues. - **Minor Suggestions:** - Consider removing the stray blank line introduced in `src/fedora_openqa/conf_test_suites.py` at line 683 to maintain clean file formatting. --- 🤖 **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.*

adamwill force-pushed report_flathub_mediawriter from bd0b1eeb95

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

Details

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

Details

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

Details

to b71c8c4277

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

Details

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

Details

2026-05-13 17:02:54 +00:00

Compare

adamwill commented

2026-05-13 17:03:08 +00:00

Blank line fixed.

adamwill merged commit b71c8c4277 into main

2026-05-13 17:06:03 +00:00

adamwill deleted branch report_flathub_mediawriter

2026-05-13 17:06:04 +00:00

Sign in to join this conversation.

No reviewers

No milestone

No project

No assignees

3 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

quality/fedora_openqa!140

Rows
Columns