quality/os-autoinst-distri-fedora
7
0
Fork 0
Code Issues 39 Pull requests 6 Projects Releases Packages Activity Actions

Create test suite for Fedora Media Writer #462

Merged
adamwill merged 9 commits from newtest/fedora-media-writer into main 2026-01-31 01:14:18 +00:00
Conversation 84 Commits 9 Files changed 77 +957 -26
lruzicka commented 2025-11-26 15:01:41 +00:00
Owner
Copy link

This PR adds the test suite for Fedora Media Writer, which is
an application to download and copy the installation media files
to the a USB disk which can be then used for installation.
So far, we have been testing this application with manual testing,
this is an attempt to automate it. It tests the following:

  • It chooses the Workstation media, downloads it, saves it locally
    and writes it to the USB disk. It also checks that the usual
    content exists on the USB disk.
  • It opens the previously downloaded ISO and writes it to the USB disk.
    Then it check that the usual content exists on the USB disk.
  • Removes the installation from the USB disk and restores it to its
    original state.
  • This does not test any hotplugging mechanisms yet.

Fixes: https://pagure.io/fedora-qa/os-autoinst-distri-fedora/issue/450

This PR adds the test suite for Fedora Media Writer, which is an application to download and copy the installation media files to the a USB disk which can be then used for installation. So far, we have been testing this application with manual testing, this is an attempt to automate it. It tests the following: * It chooses the Workstation media, downloads it, saves it locally and writes it to the USB disk. It also checks that the usual content exists on the USB disk. * It opens the previously downloaded ISO and writes it to the USB disk. Then it check that the usual content exists on the USB disk. * Removes the installation from the USB disk and restores it to its original state. * This does not test any hotplugging mechanisms yet. Fixes: https://pagure.io/fedora-qa/os-autoinst-distri-fedora/issue/450
zuul commented 2025-11-26 15:09:15 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/9683833692974ee48094d3962d06b099

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/9683833692974ee48094d3962d06b099 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/4c602fc2caec4f83a94b3e1faeaa04ec) : SUCCESS in 4m 44s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/82322035b5a6436b8bfd84c7aa37ed43) : FAILURE in 7m 17s
lruzicka commented 2025-11-26 15:21:19 +00:00
Author
Owner
Copy link

It seems that I do not have the disconnect_usb yet in installation, so the Perl check above fails.

It seems that I do not have the `disconnect_usb` yet in installation, so the Perl check above fails.
lruzicka commented 2025-11-26 15:21:48 +00:00
Author
Owner
Copy link

Passes at https://openqa.stg.fedoraproject.org/tests/5611820#details

Passes at https://openqa.stg.fedoraproject.org/tests/5611820#details
adamwill commented 2025-11-26 23:18:43 +00:00
Owner
Copy link

rebased onto 9a451c32a2

rebased onto 9a451c32a2629f897e5a4589cbad1428492d018c
zuul commented 2025-11-26 23:26:02 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/8e6dfbd184da4af2ac44c8669dbbffc5

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/8e6dfbd184da4af2ac44c8669dbbffc5 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/054c22784ed4479c8c21e358cba57e55) : SUCCESS in 4m 04s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/924ad8e200b94c03989a0d7ba2dde9d3) : FAILURE in 7m 01s
adamwill commented 2025-11-27 20:09:33 +00:00
Owner
Copy link

rebased onto 93606be074

rebased onto 93606be0740acf8983c0c3cfc0445f2c3135a19f
zuul commented 2025-11-27 20:16:37 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/6439f8e87980431fbbe062f98e402fcb

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/6439f8e87980431fbbe062f98e402fcb - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/ac0268ba5e344b618f49c4ee9d53151d) : SUCCESS in 4m 10s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/8255bc07d67c46b19ca19e2569595a31) : FAILURE in 6m 47s
adamwill commented 2025-11-27 20:46:37 +00:00
Owner
Copy link

Tests still fail because they're running on F41, because nobody merged https://pagure.io/fedora-project-config/pull-request/351 yet. :(

Tests still fail because they're running on F41, because nobody merged https://pagure.io/fedora-project-config/pull-request/351 yet. :(
adamwill commented 2025-12-02 22:36:47 +00:00
Owner
Copy link

rebased onto 712ca067a8

rebased onto 712ca067a8de805b212a5995f45c4e234dcea3f1
adamwill commented 2025-12-02 22:36:53 +00:00
Owner
Copy link

robot overlord review. edit: it came out a bit weird - it seems to think it's adding an entirely new automated test suite to the FMW application itself, which is...not what we're doing at all. still, the specific review notes are...sort of on point.

[robot overlord review](https://github.com/AdamWill/os-autoinst-distri-fedora-ai-test/pull/38). edit: it came out a bit weird - it seems to think it's adding an entirely new automated test suite to the FMW application itself, which is...not what we're doing at all. still, the specific review notes are...sort of on point.
zuul commented 2025-12-02 22:45:22 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/41f8115a770e4aaa8d026b6ca7acdb7e

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/41f8115a770e4aaa8d026b6ca7acdb7e - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/a65ec7a646eb426da80414976fbb8e52) : SUCCESS in 4m 07s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/a5c30da7fdda4000acea708b4cc5e57c) : FAILURE in 8m 18s
adamwill commented 2025-12-02 23:00:27 +00:00
Owner
Copy link

Do we really need both mwriter_download_finished and mwriter_finish_button? Couldn't the until loop just wait for the finish button to appear? Presumably it only appears when the download is finished anyway.

I think we already have a needle the same as mwriter_shortcut_downloaded in the Nautilus tests - nautilus_directory_downloads.

We already set QEMU_APPEND for the 64bit machine, which will be used to run this test on x86_64. Setting it in the test suite effectively overrides that setting, so when running this test, we won't pass the args set at the machine level any more:

global driver=cfi.pflash01,property=secure,value=on

that's a bit of cargo culting from when I enabled Secure Boot testing. Best I can figure out/remember, it locks down the UEFI pflash variables drive so only the firmware should be able to write to it - this is a more accurate simulation of a 'real' system with Secure Boot, and should catch any problems caused by the OS trying to write to it or something. Skipping it for this one test is probably fine, but I thought I'd note it.

I don't love the use of QEMU_APPEND to attach the disk as a matter of design, but AFAICS os-autoinst upstream doesn't offer this functionality so it's about all we can do. Long term it would be nice to look at adding this to os-autoinst, though, and maybe even allowing for hotplugging via QMP.

Do we really need both `mwriter_download_finished` and `mwriter_finish_button`? Couldn't the `until` loop just wait for the finish button to appear? Presumably it only appears when the download is finished anyway. I think we already have a needle the same as `mwriter_shortcut_downloaded` in the Nautilus tests - `nautilus_directory_downloads`. We already set `QEMU_APPEND` for the `64bit` machine, which will be used to run this test on x86_64. Setting it in the test suite effectively overrides that setting, so when running this test, we won't pass the args set at the machine level any more: ``` global driver=cfi.pflash01,property=secure,value=on ``` that's a bit of cargo culting from when I enabled Secure Boot testing. Best I can figure out/remember, it locks down the UEFI pflash variables drive so only the firmware should be able to write to it - this is a more accurate simulation of a 'real' system with Secure Boot, and should catch any problems caused by the OS trying to write to it or something. Skipping it for this one test is *probably* fine, but I thought I'd note it. I don't *love* the use of `QEMU_APPEND` to attach the disk as a matter of design, but AFAICS os-autoinst upstream doesn't offer this functionality so it's about all we can do. Long term it would be nice to look at adding this to os-autoinst, though, and maybe even allowing for hotplugging via QMP.
lruzicka commented 2025-12-10 11:09:45 +00:00
Author
Owner
Copy link

1 new commit added

  • Fix after review
**1 new commit added** * ``Fix after review``
zuul commented 2025-12-10 11:17:39 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/fe995dc70e1a4ad7b11296456686e76b

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/fe995dc70e1a4ad7b11296456686e76b - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/68c63019b7104291bbdbaa9191f7ceaf) : FAILURE in 4m 58s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/1a9aaa491bd54064b043dd910458d7c8) : FAILURE in 7m 34s
lruzicka commented 2025-12-10 11:18:47 +00:00
Author
Owner
Copy link

rebased onto fcb4dcbd98

rebased onto fcb4dcbd980d5a73f3837e0ddc7730a79d57beb5
lruzicka commented 2025-12-10 11:26:18 +00:00
Author
Owner
Copy link

I changed the needle things, rebased on main, however got some missing needles for ps_* connected with Plasma settings, I am not aware to have used any of them. They might have come from the rebase though.

As far as QEMU_APPEND is concerned:

  1. is it ok to leave it as is and do not comply to the EFI settings?
  2. shall I copy the machine QEMU params into the test QEMU params and use both?
  3. shall I introduce a different QEMU variable and try to set up a mechanism to merge both before boot?
I changed the needle things, rebased on main, however got some missing needles for `ps_*` connected with Plasma settings, I am not aware to have used any of them. They might have come from the rebase though. As far as `QEMU_APPEND` is concerned: 1) is it ok to leave it as is and do not comply to the EFI settings? 2) shall I copy the machine QEMU params into the test QEMU params and use both? 3) shall I introduce a different QEMU variable and try to set up a mechanism to merge both before boot?
adamwill commented 2025-12-10 16:24:54 +00:00
Owner
Copy link

sorry, yeah, the ps_ is from the plasma setup changes I merged, I must've missed running the needle check script.

For QEMU_APPEND I think it's probably OK to leave it. I mostly just wanted to write it down here so we have it as a reference if it ever becomes an issue in the future.

sorry, yeah, the ps_ is from the plasma setup changes I merged, I must've missed running the needle check script. For QEMU_APPEND I think it's probably OK to leave it. I mostly just wanted to write it down here so we have it as a reference if it ever becomes an issue in the future.
lruzicka commented 2025-12-11 09:27:37 +00:00
Author
Owner
Copy link

rebased onto 672916955f

rebased onto 672916955fce6b829b2999e52830882499f5bb2f
zuul commented 2025-12-11 09:35:28 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/36e49764811c4a0a9560cd7751ec62aa

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/36e49764811c4a0a9560cd7751ec62aa - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/91718617131b4e50914c821ad32f4988) : FAILURE in 4m 10s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/e97c4eca7d1c4c119a479d02dbd665e3) : FAILURE in 7m 29s
adamwill commented 2025-12-13 01:03:45 +00:00
Owner
Copy link

rebased onto 656f44641c

rebased onto 656f44641c27fe41fade9308d316da98156b4158
adamwill commented 2025-12-13 01:04:20 +00:00
Owner
Copy link

Why do we bother with making $mediagroup configurable as MEDIA var but never actually change it? It's always 'official'. Is there a reason not to just hardcode it?

Why do we bother with making `$mediagroup` configurable as `MEDIA` var but never actually change it? It's always 'official'. Is there a reason not to just hardcode it?
zuul commented 2025-12-13 01:10:58 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/84ae7a50917c4e2580eb3eeda33db56f

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/84ae7a50917c4e2580eb3eeda33db56f - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/a5c569cef2054fb39a1f08e931978f6c) : SUCCESS in 4m 30s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/bd1d5836f0d0447e94fc8ea388fde563) : FAILURE in 6m 57s
adamwill commented 2025-12-13 01:16:16 +00:00
Owner
Copy link

You also didn't address the robot overlord review. I disagree with a couple of its notes, but it has a valid one: the definition of my $password in tests/applications/media-writer/download_write.pm is unnecessary, it is never used.

You also didn't address the robot overlord review. I disagree with a couple of its notes, but it has a valid one: the definition of `my $password` in `tests/applications/media-writer/download_write.pm` is unnecessary, it is never used.
adamwill commented 2025-12-13 01:21:22 +00:00
Owner
Copy link

Why do we do the mkdir -p /mnt/usbdisk outside of the check function? Why not just do it in there so we only have the code once, not three times?

Why do we do the `mkdir -p /mnt/usbdisk` outside of the check function? Why not just do it in there so we only have the code once, not three times?
lruzicka commented 2025-12-15 11:08:48 +00:00
Author
Owner
Copy link

Why do we bother with making $mediagroup configurable as MEDIA var but never actually change it? It's always 'official'. Is there a reason not to just hardcode it?

I was thinking that maybe in the future, when we'd like to test a different image or something. Changing to the hardcoded version.

> Why do we bother with making `$mediagroup` configurable as `MEDIA` var but never actually change it? It's always 'official'. Is there a reason not to just hardcode it? I was thinking that maybe in the future, when we'd like to test a different image or something. Changing to the hardcoded version.
lruzicka commented 2025-12-15 11:10:55 +00:00
Author
Owner
Copy link

You also didn't address the robot overlord review. I disagree with a couple of its notes, but it has a valid one: the definition of my $password in tests/applications/media-writer/download_write.pm is unnecessary, it is never used.

I expected the overlord to provide a link, then realized it was you who had provided it. Went and found the review and fixed it.

> You also didn't address the robot overlord review. I disagree with a couple of its notes, but it has a valid one: the definition of `my $password` in `tests/applications/media-writer/download_write.pm` is unnecessary, it is never used. I expected the overlord to provide a link, then realized it was you who had provided it. Went and found the review and fixed it.
lruzicka commented 2025-12-15 11:15:40 +00:00
Author
Owner
Copy link

3 new commits added

  • Move mount point creation in the check function
  • Hardcode mediagroup
  • Remove
**3 new commits added** * ``Move mount point creation in the check function`` * ``Hardcode mediagroup`` * ``Remove``
lruzicka commented 2025-12-15 11:17:20 +00:00
Author
Owner
Copy link

Why do we do the mkdir -p /mnt/usbdisk outside of the check function? Why not just do it in there so we only have the code once, not three times?

Brain fog? Fixed.

> Why do we do the `mkdir -p /mnt/usbdisk` outside of the check function? Why not just do it in there so we only have the code once, not three times? Brain fog? Fixed.
zuul commented 2025-12-15 11:22:49 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/0f574f4b56df4cf1ac7c42e832e846b0

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/0f574f4b56df4cf1ac7c42e832e846b0 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/7ce4fa0095d14c2faa3c069b40b7043c) : SUCCESS in 4m 33s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/a4c5ec224b6e40488bc9afffe904ebb1) : FAILURE in 6m 53s
adamwill commented 2025-12-15 17:40:38 +00:00
Owner
Copy link

thanks!

the robot overlord stuff is all manual for now. I'm literally just doing git push ai-test main; git push ai-test <branchname> then opening a pull request from the link (obviously, ai-test is https://github.com/AdamWill/os-autoinst-distri-fedora-ai-test). The overlord review happens automatically once the PR is opened, then I just have to link it back here.

I could automate that, but it feels like a waste of effort. The stuff I'm doing on ai-code-review is meant to be a more permanent alternative, once we move this repo to forgejo.

thanks! the robot overlord stuff is all manual for now. I'm literally just doing `git push ai-test main; git push ai-test <branchname>` then opening a pull request from the link (obviously, `ai-test` is https://github.com/AdamWill/os-autoinst-distri-fedora-ai-test). The overlord review happens automatically once the PR is opened, then I just have to link it back here. I could automate that, but it feels like a waste of effort. The stuff I'm doing on ai-code-review is meant to be a more permanent alternative, once we move this repo to forgejo.
lruzicka commented 2025-12-17 11:54:13 +00:00
Author
Owner
Copy link

rebased onto 13811e106c

rebased onto 13811e106cc4f4e76d426ef4f9706952ba56cc7c
zuul commented 2025-12-17 12:01:44 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/71590efa76b045238507c7fe775f16f9

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/71590efa76b045238507c7fe775f16f9 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/b65c2c74b29f4418829e2a3200138eb9) : SUCCESS in 4m 36s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/6dbf136aa3b44f2986006b06b3bd495d) : FAILURE in 7m 10s
adamwill commented 2025-12-23 00:22:43 +00:00
Owner
Copy link

rebased onto 56bfde0f7a

rebased onto 56bfde0f7a46dffaa974b8f428dc16eca9fed851
zuul commented 2025-12-23 00:30:03 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/aaed023263b04a8fb0f43c963eda84b5

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/aaed023263b04a8fb0f43c963eda84b5 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/1d133ac180364fb2bd72577fe1c2c3c2) : SUCCESS in 4m 08s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/9d162a361c984792ad9479c7b9f986be) : FAILURE in 7m 07s
adamwill commented 2026-01-02 21:58:27 +00:00
Owner
Copy link

rebased onto e43f7eac09

rebased onto e43f7eac0980f0b104a44dca808dc10ede560f6b
adamwill commented 2026-01-02 22:02:46 +00:00
Owner
Copy link

So now I'm looking at the results of this over the shutdown, it commonly fails with an error like this:

[2026-01-02T07:06:27.043925Z] [warn] [pid:3198130] !!! : qemu-system-x86_64: -device usb-storage,drive=usb_disk,bus=usb.0,port=4: Failed to get "write" lock
[2026-01-02T07:06:27.043983Z] [debug] [pid:3198130] QEMU: Is another process using the image [/var/lib/openqa/factory/hdd/fixed/disk_usb.img]?

which I'm gonna guess happens because we get multiple instances of the test running at the same time, all trying to lock the same file. So that's going to be a problem.

I think we might need to extend os-autoinst upstream to support adding USB devices with overlay storage, just like how it handles hard disk images currently...

So now I'm looking at the results of this over the shutdown, it commonly fails with an error like this: ``` [2026-01-02T07:06:27.043925Z] [warn] [pid:3198130] !!! : qemu-system-x86_64: -device usb-storage,drive=usb_disk,bus=usb.0,port=4: Failed to get "write" lock [2026-01-02T07:06:27.043983Z] [debug] [pid:3198130] QEMU: Is another process using the image [/var/lib/openqa/factory/hdd/fixed/disk_usb.img]? ``` which I'm gonna guess happens because we get multiple instances of the test running at the same time, all trying to lock the same file. So that's going to be a problem. I think we might need to extend os-autoinst upstream to support adding USB devices with overlay storage, just like how it handles hard disk images currently...
zuul commented 2026-01-02 22:05:57 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/0b1ba1168d39423886703446b31d6f17

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/0b1ba1168d39423886703446b31d6f17 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/d16a9bfed9144b38a5601eb6d2eee5f2) : SUCCESS in 4m 19s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/d6c91bd2c21d40aabc3a30a4f8827deb) : SUCCESS in 7m 19s
lruzicka commented 2026-01-07 10:21:58 +00:00
Author
Owner
Copy link

rebased onto 80ec5ea718

rebased onto 80ec5ea71806db4c0b32c59b72a3d3cded6cad06
zuul commented 2026-01-07 10:29:04 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/13645a3f438c4961989efcd887e3d0f6

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/13645a3f438c4961989efcd887e3d0f6 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/54fc08191ae944b99b672b44802494af) : FAILURE in 3m 06s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/ba180462c9ba4c1e886662c25cd0f7ba) : SUCCESS in 6m 50s
adamwill commented 2026-01-07 17:48:16 +00:00
Owner
Copy link

rebased onto abf1ab7755

rebased onto abf1ab7755acaf9df436c38acc71e388f52af339
zuul commented 2026-01-07 17:55:42 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/0d9555b1ff6d4f23907239fcef8f1ed5

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/0d9555b1ff6d4f23907239fcef8f1ed5 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/af6bf42d83954ed8b7de5a5be733e837) : FAILURE in 2m 54s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/5394fa130a6c46f5ac0d6613e71a30d8) : SUCCESS in 7m 12s
adamwill commented 2026-01-07 22:42:36 +00:00
Owner
Copy link

rebased onto d8cdebe87e

rebased onto d8cdebe87e23a57fb20c2a9ec12a13a83e942108
zuul commented 2026-01-07 22:52:31 +00:00
First-time contributor
Copy link

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/d9aeafb5cdef4c8aa2cee5bcafe6aa9e

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci https://fedora.softwarefactory-project.io/zuul/buildset/d9aeafb5cdef4c8aa2cee5bcafe6aa9e - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/43ef86bd140840919f8f18d856c5b974) : FAILURE in 3m 53s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/a48a69681fcb494fa80ba00d69563825) : SUCCESS in 9m 42s
kashyapc commented 2026-01-08 15:16:41 +00:00
First-time contributor
Copy link

A small note on this part:

"QEMU_APPEND=-device qemu-xhci,id=usb -drive
id=usb_disk,file=/var/lib/openqa/factory/hdd/fixed/disk_usb.img,format=raw,discard=unmap,if=none
-device usb-storage,drive=usb_disk,bus=usb.0,port=4"

Please consider using -blockdev, as QEMU upstream considers -drive to be legacy and might remove it in the future

Untested translation of your current QEMU_APPEND line:

-device qemu-xhci,id=usb \
-blockdev driver=file,filename=/path/to/disk_usb.img,node-name=usb_file \
-blockdev driver=raw,file=usb_file,node-name=usb_disk \
-device usb-storage,drive=usb_disk,bus=usb.0,port=4
A small note on this part: "QEMU_APPEND=-device qemu-xhci,id=usb -drive id=usb_disk,file=/var/lib/openqa/factory/hdd/fixed/disk_usb.img,format=raw,discard=unmap,if=none -device usb-storage,drive=usb_disk,bus=usb.0,port=4" Please consider using `-blockdev`, as QEMU upstream considers `-drive` to be legacy and might remove it in the future Untested translation of your current QEMU_APPEND line: -device qemu-xhci,id=usb \ -blockdev driver=file,filename=/path/to/disk_usb.img,node-name=usb_file \ -blockdev driver=raw,file=usb_file,node-name=usb_disk \ -device usb-storage,drive=usb_disk,bus=usb.0,port=4
kashyapc commented 2026-01-08 15:47:29 +00:00
First-time contributor
Copy link

Please consider using -blockdev, as QEMU upstream considers -drive to be legacy and might remove it in the future

Also, this is not a "cosmetic change", although it can look like that. -drive has weird hacks and historical baggage, the -blockdev is a major low-level rework (from 5 years ago, now it's stable) in the QEMU block layer.

While there's no urgency here just wanted to note that QEMU upstream discourages -drive for new applications. And OpenQA already uses -blockdev for disk images.

> Please consider using -blockdev, as QEMU upstream considers -drive to be legacy and might remove it in the future Also, this is not a "cosmetic change", although it can look like that. `-drive` has weird hacks and historical baggage, the `-blockdev` is a major low-level rework (from 5 years ago, now it's stable) in the QEMU block layer. While there's no urgency here just wanted to note that QEMU upstream discourages `-drive` for new applications. And OpenQA already uses `-blockdev` for disk images.
lruzicka commented 2026-01-14 14:08:31 +00:00
Author
Owner
Copy link

rebased onto 9a97fdcaf1

rebased onto 9a97fdcaf1ba3d01dd6bb36ae45b6d1b9fc9cbcd
zuul commented 2026-01-14 14:17:20 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/d1fa52c18f504a5db23e2f55b3105640

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/d1fa52c18f504a5db23e2f55b3105640 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/0de8273045ba429385ea853c6b1176d9) : SUCCESS in 4m 12s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/639f9a17d362458aa74878ae26d0c740) : SUCCESS in 8m 34s
lruzicka commented 2026-01-14 17:48:30 +00:00
Author
Owner
Copy link

1 new commit added

  • Change -drive to -blockdev
**1 new commit added** * ``Change -drive to -blockdev``
zuul commented 2026-01-14 17:55:46 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/7317583777a7454d8560c8f258b474d0

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/7317583777a7454d8560c8f258b474d0 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/1a9f52db8a084429b64e3ae27d6c85a5) : SUCCESS in 3m 43s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/9b8dd248ad464bd9953297019fe2edd0) : SUCCESS in 6m 59s
lruzicka commented 2026-01-14 18:42:25 +00:00
Author
Owner
Copy link

1 new commit added

  • Try something else
**1 new commit added** * ``Try something else``
zuul commented 2026-01-14 18:49:41 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/d37b80a24cce42fbb3e4c6d50af3cd37

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/d37b80a24cce42fbb3e4c6d50af3cd37 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/cb78cb4a4ed143f2afab24ccc6c414fb) : SUCCESS in 3m 57s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/d9786c4d53c548489261972b32a0bfa8) : SUCCESS in 6m 59s
lruzicka commented 2026-01-14 18:57:33 +00:00
Author
Owner
Copy link

Everything worked until I started to experiment with -blockdev that is not able to provide the functionality that -drive could. At least I could not figure that out just yet.

Everything worked until I started to experiment with `-blockdev` that is not able to provide the functionality that `-drive` could. At least I could not figure that out just yet.
lruzicka commented 2026-01-14 20:25:38 +00:00
Author
Owner
Copy link

I might have figured that out after all, I will need to check tomorrow once more, but I have seen tests passing with the latest update.

I might have figured that out after all, I will need to check tomorrow once more, but I have seen tests passing with the latest update.
lruzicka commented 2026-01-15 11:29:08 +00:00
Author
Owner
Copy link

rebased onto 9a97fdcaf1

rebased onto 9a97fdcaf1ba3d01dd6bb36ae45b6d1b9fc9cbcd
zuul commented 2026-01-15 11:38:54 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/b1eeb5ed834647c19906dbe4e2685a6b

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/b1eeb5ed834647c19906dbe4e2685a6b - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/d5c37d6dbc0e4b61aaac38bec43382b0) : SUCCESS in 4m 04s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/f26ce2b39a554b1c9cbfd682db2116b8) : SUCCESS in 9m 30s
lruzicka commented 2026-01-15 15:39:14 +00:00
Author
Owner
Copy link

1 new commit added

  • Try making overlay in the variables
**1 new commit added** * ``Try making overlay in the variables``
zuul commented 2026-01-15 15:45:53 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/6935f13ac7ab4b62970022091174aebd

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/6935f13ac7ab4b62970022091174aebd - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/fdd127d7874244cd9c44a0a23e6965ab) : SUCCESS in 3m 58s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/1f7b9b5e285c4d5a834b1cdc41b6ddc1) : SUCCESS in 6m 26s
lruzicka commented 2026-01-15 16:22:42 +00:00
Author
Owner
Copy link

4 new commits added

  • Try making overlay in the variables
  • Change -drive to -blockdev
  • Fix the locking mechanism and add tests as requested
  • Create test suite for Fedora Media Writer
**4 new commits added** * ``Try making overlay in the variables`` * ``Change -drive to -blockdev`` * ``Fix the locking mechanism and add tests as requested`` * ``Create test suite for Fedora Media Writer``
zuul commented 2026-01-15 16:30:20 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/6d405b5b2c3743949df470609e51316b

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/6d405b5b2c3743949df470609e51316b - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/484c74bd3d674f32b7779d21dc064e5d) : SUCCESS in 4m 10s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/49dd4f43c0d24de9845d3f5360a0e89b) : SUCCESS in 7m 27s
lruzicka commented 2026-01-16 11:12:57 +00:00
Author
Owner
Copy link

2 new commits added

  • Make a locally working solution
  • Revert "Try making overlay in the variables"
**2 new commits added** * ``Make a locally working solution`` * ``Revert "Try making overlay in the variables"``
zuul commented 2026-01-16 11:21:34 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/49507ff856844480807b7e99658c520b

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/49507ff856844480807b7e99658c520b - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/b02867d93a52416789490e2f5d3357e4) : SUCCESS in 4m 11s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/365744034f4f4d1b8ec665fc9633463b) : SUCCESS in 8m 21s
lruzicka commented 2026-01-16 13:11:54 +00:00
Author
Owner
Copy link

3 new commits added

  • Change -drive to -blockdev
  • Fix the locking mechanism and add tests as requested
  • Create test suite for Fedora Media Writer
**3 new commits added** * ``Change -drive to -blockdev`` * ``Fix the locking mechanism and add tests as requested`` * ``Create test suite for Fedora Media Writer``
zuul commented 2026-01-16 13:18:56 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/c97e226256eb44148c318c4ce7cd0f46

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/c97e226256eb44148c318c4ce7cd0f46 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/6a20a2c739f24819a4469119c7544d95) : SUCCESS in 4m 01s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/0dfe6d9b8d0f4737802d04335d70ebbf) : SUCCESS in 6m 48s
lruzicka commented 2026-01-16 13:28:51 +00:00
Author
Owner
Copy link

So, I think I have lost all the stammina here and I feel drenched to the bone, however:

  • the Fedora Media tests works on Staging on multiple instances
  • it performs the checks requested by Kamil - checkmd5iso and cmp
  • it does not lock the asset but creates an overlay on a per job basis so the test can run parallel with other similar tests
  • it invokes QEMU with -blockdev and not -drive as requested by Kashyap.

Sorry, it has taken so long, but I tried numerous ways to set up the overlay, something only worked with -drive, something only worked locally and not on Staging. When something worked and I wanted to build something else above, it failed apart again, so I had to try different combinations of the variables, using either the HDD or the QEMU_APPEND approach, and each failure took ages to retest.

Now, I am not sure I can come with something better than this.

So, I think I have lost all the stammina here and I feel drenched to the bone, however: * the Fedora Media tests works on Staging on multiple instances * it performs the checks requested by Kamil - `checkmd5iso` and `cmp` * it does not lock the asset but creates an overlay on a `per job` basis so the test can run parallel with other similar tests * it invokes QEMU with `-blockdev` and not `-drive` as requested by Kashyap. Sorry, it has taken so long, but I tried numerous ways to set up the overlay, something only worked with `-drive`, something only worked locally and not on Staging. When something worked and I wanted to build something else above, it failed apart again, so I had to try different combinations of the variables, using either the `HDD` or the `QEMU_APPEND` approach, and each failure took ages to retest. Now, I am not sure I can come with something better than this.
lruzicka commented 2026-01-16 13:30:30 +00:00
Author
Owner
Copy link

rebased onto f6c8cb3bce

rebased onto f6c8cb3bcebcfcfc03949dbb3d7bad5bcad06816
zuul commented 2026-01-16 13:37:28 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/bbdce6e61eb5478e866b3198070f3018

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/bbdce6e61eb5478e866b3198070f3018 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/6e2e18a40c2b4beaa4f67d22afea5f1c) : SUCCESS in 3m 51s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/00a031f339e344fda21f213252080909) : SUCCESS in 6m 42s
adamwill commented 2026-01-16 17:12:00 +00:00
Owner
Copy link

I do think since it took this much work it might have been worth looking at doing it upstream instead, but congratulations :D I'll try and take a look at it soon once I can deal with all these piles of fire...

I do think since it took this much work it might have been worth looking at doing it upstream instead, but congratulations :D I'll try and take a look at it soon once I can deal with all these piles of fire...
adamwill commented 2026-01-22 16:23:50 +00:00
Owner
Copy link

rebased onto c72066ae32

rebased onto c72066ae32ee7891aafb4b8ed842df3f8417fc01
zuul commented 2026-01-22 16:31:03 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/0ad09c97a12440a88158127c33a3aa79

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/0ad09c97a12440a88158127c33a3aa79 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/a981ad368346471ba3fb6c6615672e40) : SUCCESS in 3m 47s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/e870fb85b82c4899ac6aa245e3b656b9) : SUCCESS in 6m 55s
adamwill commented 2026-01-22 19:42:00 +00:00
Owner
Copy link

rebased onto 1d6a96231b

rebased onto 1d6a96231b1b3d5c6b76184064eb430638fd7eea
zuul commented 2026-01-22 19:49:21 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/652fcbe415b1495581d8ac996eab347e

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/652fcbe415b1495581d8ac996eab347e - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/adc5c608da5d4fad86cf32c2d08b6224) : SUCCESS in 3m 43s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/1cc5e4d070c64313a92fb94984b89a8d) : SUCCESS in 7m 04s
adamwill commented 2026-01-22 22:10:27 +00:00
Owner
Copy link

rebased onto 41df08d878

rebased onto 41df08d87861ddf915e1c602b06a9005ef19448b
zuul commented 2026-01-22 22:21:44 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/a4c23dce815c46e5bca93bfec51a452e

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/a4c23dce815c46e5bca93bfec51a452e - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/c02860c1595541bb889f127b6a893b59) : SUCCESS in 4m 19s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/572bf448b03441aebd71b623dee0d368) : SUCCESS in 11m 03s
adamwill commented 2026-01-23 01:38:21 +00:00
Owner
Copy link

rebased onto 0ebd4fdc31

rebased onto 0ebd4fdc31d219f615fc40f4f58155ba46f82314
zuul commented 2026-01-23 01:45:42 +00:00
First-time contributor
Copy link

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/baa9c28d69db46eb8fc05578ae610241

Build succeeded. https://fedora.softwarefactory-project.io/zuul/buildset/baa9c28d69db46eb8fc05578ae610241 - [os-autoinst-tox ](https://fedora.softwarefactory-project.io/zuul/build/96a483af6bcf4ecdb6a6b4d6985da6c0) : SUCCESS in 4m 15s - [os-autoinst-perl ](https://fedora.softwarefactory-project.io/zuul/build/2487aecfb1db466e948704ac66938182) : SUCCESS in 7m 09s
adamwill force-pushed newtest/fedora-media-writer from a6e6f93fd2 to 5a41509f09
All checks were successful
CI via Tox and perl / tox (pull_request) Successful in 1m34s
Details
CI via Tox and perl / perl (pull_request) Successful in 3m25s
Details
2026-01-24 07:25:29 +00:00 Compare
adamwill force-pushed newtest/fedora-media-writer from 5a41509f09
All checks were successful
CI via Tox and perl / tox (pull_request) Successful in 1m34s
Details
CI via Tox and perl / perl (pull_request) Successful in 3m25s
Details
to e8f1023702
Some checks failed
CI via Tox and perl / tox (pull_request) Successful in 1m39s
Details
CI via Tox and perl / perl (pull_request) Successful in 3m29s
Details
AI Code Review / ai-review (pull_request_target) Successful in 1m10s
Details
AI Code Review / remove-label (pull_request_target) Failing after 31s
Details
2026-01-26 20:29:36 +00:00 Compare
lruzicka commented 2026-01-27 09:25:12 +00:00
Author
Owner
Copy link

@adamwill wrote in #462 (comment):

I do think since it took this much work it might have been worth looking at doing it upstream instead, but congratulations :D I'll try and take a look at it soon once I can deal with all these piles of fire...

But who would be doing it? I did not feel confident enough to meddle with the upstream because of the hotplugging you mentioned. However, if you mean like add an upstream variable that would enable what I enable manually here. That might be worth trying once this has proved to work.

@adamwill wrote in https://forge.fedoraproject.org/quality/os-autoinst-distri-fedora/pulls/462#issuecomment-345573: > I do think since it took this much work it might have been worth looking at doing it upstream instead, but congratulations :D I'll try and take a look at it soon once I can deal with all these piles of fire... But who would be doing it? I did not feel confident enough to meddle with the upstream because of the hotplugging you mentioned. However, if you mean like add an upstream variable that would enable what I enable manually here. That might be worth trying once this has proved to work.
adamwill commented 2026-01-27 15:02:26 +00:00
Owner
Copy link

Sure, why not? It's a good way to learn - perl skills, and how os-autoinst works upstream. Even if you can't figure it out it's probably worth it as an exercise.

Sure, why not? It's a good way to learn - perl skills, and how os-autoinst works upstream. Even if you can't figure it out it's probably worth it as an exercise.
lruzicka added this to the Sprint 2 project 2026-01-27 16:47:31 +00:00
adamwill commented 2026-01-28 01:43:43 +00:00
Owner
Copy link

Overall this is looking good now, but I do have some nits, mainly to do with organization.

  1. The authenticate sub doesn't have anything much to do with disks, per se. It should probably move into utils.pm now we're calling it from elsewhere. We might also want to grep for other cases of auth_required and see if any of those could use this sub too.
  2. Conversely, the three select_ subs in lib/mwriter.pm are used only once each and are quite small. Why not just in-line them where they're used?
  3. The toolbox actions are only used once each, and I already have a pattern for doing it which seems a bit better, to me. See the two places in utils.pm where we do toolbox -y enter - if you pass -y like that it will automatically create the toolbox if it doesn't exist. You can specify the name to enter explicitly if necessary. Then you can do assert_script_run "true", 180; to wait until the toolbox is actually ready; it's more reliable than a magic sleep. There is also toolbox run for running a single command in a specific container, which also might be useful here (again with -y to create it if necessary I believe). I think it'd be best to follow those patterns for the toolbox stuff here; I'm not sure whether it's worth de-duplicating with the other two uses since the subs would be very short if efficiently implemented, but see what you think.
Overall this is looking good now, but I do have some nits, mainly to do with organization. 1. The `authenticate` sub doesn't have anything much to do with disks, per se. It should probably move into `utils.pm` now we're calling it from elsewhere. We might also want to grep for other cases of `auth_required` and see if any of those could use this sub too. 2. Conversely, the three `select_` subs in `lib/mwriter.pm` are used only once each and are quite small. Why not just in-line them where they're used? 3. The toolbox actions are only used once each, and I already have a pattern for doing it which seems a bit better, to me. See the two places in `utils.pm` where we do `toolbox -y enter` - if you pass `-y` like that it will automatically create the toolbox if it doesn't exist. You can specify the name to enter explicitly if necessary. Then you can do `assert_script_run "true", 180;` to wait until the toolbox is actually ready; it's more reliable than a magic sleep. There is also `toolbox run` for running a single command in a specific container, which also might be useful here (again with `-y` to create it if necessary I believe). I think it'd be best to follow those patterns for the toolbox stuff here; I'm not sure whether it's worth de-duplicating with the other two uses since the subs would be very short if efficiently implemented, but see what you think.
forgejo-actions commented 2026-01-28 01:45:18 +00:00
Copy link

AI Code Review

📋 MR Summary

Implements an automated test suite for Fedora Media Writer, covering installation, downloading/writing media (Workstation & Silverblue), writing existing ISOs, and restoring the USB device.

  • Key Changes:
    • Added lib/mwriter.pm helper for FMW specific interactions and verification logic
    • Introduced toolbox helper in utils.pm for Silverblue package management
    • Created test modules: download_write, open_write, restore, about
    • Added media_writer profile to templates.fif.json using -blockdev compatible settings
  • Impact: tests/applications/media-writer, lib/mwriter.pm, lib/utils.pm, templates.fif.json, check-needles.py
  • Risk Level: 🟡 Medium - The changes introduce new test logic that contains critical assertions errors (specifically in verification steps) which could lead to false positive test results. The changes are isolated to test code, minimizing production risk.

Detailed Code Review

The PR successfully implements the requested test flows for Fedora Media Writer and addresses the infrastructure requirements (blockdev/locking). The separation of logic into mwriter.pm is a good design choice.

However, there are significant logic errors in the verification steps (restore verification and ISO selection) that need to be addressed to ensure the tests actually validate the functionality. Additionally, despite the previous review notes, the unused password variable remains in the code.

📂 File Reviews

📄 `tests/applications/media-writer/restore.pm` - Test module for restoring the USB device to its original state.
  • Critical [Bug]: The assertion to check if the disk is empty is flawed. The command [ ... ] && echo 1 || echo 0 will always exit with code 0 (success), so assert_script_run will never fail regardless of whether the disk is empty or not.
    • Suggestion: Use a shell command that actually fails (exit code != 0) if the directory is not empty.
# Fixed: Assert that the directory is empty
assert_script_run('[ -z "$(ls -A /mnt/usbdisk)" ]');
  • Major [Bug]: The mount command uses script_run instead of assert_script_run. If mounting fails (e.g., partition missing), the script continues, and the subsequent empty-check will pass on the empty mountpoint, causing a false positive.
    • Suggestion: Enforce successful mounting.
# Fixed: Ensure mount succeeds
assert_script_run("mount /dev/sda1 /mnt/usbdisk");
  • Minor [Style]: Redundant mkdir -p /mnt/usbdisk. This directory is already created in aaa_setup.pm, which is a prerequisite for this suite.
    • Suggestion: Remove the redundant mkdir call if aaa_setup.pm is guaranteed to run, matching the addressed issues list.
📄 `lib/mwriter.pm` - Helper library for Media Writer interactions and verification.
  • Major [Bug]: The command ls /home/test/Downloads/*.iso is risky. If previous tests left ISOs or if multiple ISOs exist, this returns multiple lines. Used in cmp, this will break the syntax or compare the wrong files.
    • Suggestion: Ensure exactly one ISO is returned, or pick the most recent one specifically.
# Fixed: Pick the most recently modified ISO
my $iso = script_output("ls -t /home/test/Downloads/*.iso | head -n1");
  • Minor [Security]: Variable interpolation in $compare command is unquoted. If the ISO filename contains spaces or special characters, the command will fail.
    • Suggestion: Quote the filename variable in the shell command.
my $compare = "cmp /dev/sda \"$iso\" 2>&1 | tee /dev/stderr | grep -q 'EOF on .*\.iso'";
📄 `tests/applications/media-writer/download_write.pm` - Main test flow for downloading and writing an image.
  • Minor [Style]: The variable $password is defined but never used. The review context stated this was removed, but it persists in the diff.
    • Suggestion: Remove the unused variable.
# Remove this line
# my $password = get_var('USER_PASSWORD', 'weakpassword');
📄 `lib/utils.pm` - Utility functions, adding `toolbox` helper.
  • Suggestion [Security]: The toolbox subroutine blindly passes $params to sudo dnf. While low risk in a test environment, it allows command injection if caller data is tainted.
    • Suggestion: No immediate change needed for tests, but ensure callers only pass trusted package names.

✅ Summary

  • Overall Assessment: The logic for the main test verification (restore and ISO content check) contains bugs that render the assertions ineffective. These must be fixed before merging to avoid false positives.
  • Priority Issues:
    • Fix broken assertion logic in restore.pm (currently always passes).
    • Change script_run to assert_script_run for the mount command in restore.pm to prevent false positives.
    • Handle multiple ISO files correctly in lib/mwriter.pm to prevent cmp command failures.
    • Remove the unused $password variable in download_write.pm as previously agreed.
  • Minor Suggestions:
    • Quote variables in shell commands in lib/mwriter.pm.
    • Remove redundant mkdir calls that are covered by aaa_setup.pm.

🤖 AI Code Review | Generated with ai-code-review
Platform: Forgejo | AI Provider: gemini | Model: gemini-3-pro-preview

## AI Code Review ### 📋 MR Summary Implements an automated test suite for Fedora Media Writer, covering installation, downloading/writing media (Workstation & Silverblue), writing existing ISOs, and restoring the USB device. - **Key Changes:** - Added `lib/mwriter.pm` helper for FMW specific interactions and verification logic - Introduced `toolbox` helper in `utils.pm` for Silverblue package management - Created test modules: `download_write`, `open_write`, `restore`, `about` - Added `media_writer` profile to `templates.fif.json` using `-blockdev` compatible settings - **Impact:** tests/applications/media-writer, lib/mwriter.pm, lib/utils.pm, templates.fif.json, check-needles.py - **Risk Level:** 🟡 Medium - The changes introduce new test logic that contains critical assertions errors (specifically in verification steps) which could lead to false positive test results. The changes are isolated to test code, minimizing production risk. ### Detailed Code Review The PR successfully implements the requested test flows for Fedora Media Writer and addresses the infrastructure requirements (blockdev/locking). The separation of logic into `mwriter.pm` is a good design choice. However, there are significant logic errors in the verification steps (restore verification and ISO selection) that need to be addressed to ensure the tests actually validate the functionality. Additionally, despite the previous review notes, the unused password variable remains in the code. #### 📂 File Reviews <details> <summary><strong>📄 `tests/applications/media-writer/restore.pm`</strong> - Test module for restoring the USB device to its original state.</summary> - **Critical** [Bug]: The assertion to check if the disk is empty is flawed. The command `[ ... ] && echo 1 || echo 0` will always exit with code 0 (success), so `assert_script_run` will never fail regardless of whether the disk is empty or not. - *Suggestion:* Use a shell command that actually fails (exit code != 0) if the directory is not empty. ```` # Fixed: Assert that the directory is empty assert_script_run('[ -z "$(ls -A /mnt/usbdisk)" ]'); ```` - **Major** [Bug]: The `mount` command uses `script_run` instead of `assert_script_run`. If mounting fails (e.g., partition missing), the script continues, and the subsequent empty-check will pass on the empty mountpoint, causing a false positive. - *Suggestion:* Enforce successful mounting. ```` # Fixed: Ensure mount succeeds assert_script_run("mount /dev/sda1 /mnt/usbdisk"); ```` - **Minor** [Style]: Redundant `mkdir -p /mnt/usbdisk`. This directory is already created in `aaa_setup.pm`, which is a prerequisite for this suite. - *Suggestion:* Remove the redundant mkdir call if `aaa_setup.pm` is guaranteed to run, matching the addressed issues list. </details> <details> <summary><strong>📄 `lib/mwriter.pm`</strong> - Helper library for Media Writer interactions and verification.</summary> - **Major** [Bug]: The command `ls /home/test/Downloads/*.iso` is risky. If previous tests left ISOs or if multiple ISOs exist, this returns multiple lines. Used in `cmp`, this will break the syntax or compare the wrong files. - *Suggestion:* Ensure exactly one ISO is returned, or pick the most recent one specifically. ```` # Fixed: Pick the most recently modified ISO my $iso = script_output("ls -t /home/test/Downloads/*.iso | head -n1"); ```` - **Minor** [Security]: Variable interpolation in `$compare` command is unquoted. If the ISO filename contains spaces or special characters, the command will fail. - *Suggestion:* Quote the filename variable in the shell command. ```` my $compare = "cmp /dev/sda \"$iso\" 2>&1 | tee /dev/stderr | grep -q 'EOF on .*\.iso'"; ```` </details> <details> <summary><strong>📄 `tests/applications/media-writer/download_write.pm`</strong> - Main test flow for downloading and writing an image.</summary> - **Minor** [Style]: The variable `$password` is defined but never used. The review context stated this was removed, but it persists in the diff. - *Suggestion:* Remove the unused variable. ```` # Remove this line # my $password = get_var('USER_PASSWORD', 'weakpassword'); ```` </details> <details> <summary><strong>📄 `lib/utils.pm`</strong> - Utility functions, adding `toolbox` helper.</summary> - **Suggestion** [Security]: The `toolbox` subroutine blindly passes `$params` to `sudo dnf`. While low risk in a test environment, it allows command injection if caller data is tainted. - *Suggestion:* No immediate change needed for tests, but ensure callers only pass trusted package names. </details> ### ✅ Summary - **Overall Assessment:** The logic for the main test verification (restore and ISO content check) contains bugs that render the assertions ineffective. These must be fixed before merging to avoid false positives. - **Priority Issues:** - Fix broken assertion logic in `restore.pm` (currently always passes). - Change `script_run` to `assert_script_run` for the `mount` command in `restore.pm` to prevent false positives. - Handle multiple ISO files correctly in `lib/mwriter.pm` to prevent `cmp` command failures. - Remove the unused `$password` variable in `download_write.pm` as previously agreed. - **Minor Suggestions:** - Quote variables in shell commands in `lib/mwriter.pm`. - Remove redundant `mkdir` calls that are covered by `aaa_setup.pm`. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) **Platform:** Forgejo | **AI Provider:** gemini | **Model:** gemini-3-pro-preview
adamwill commented 2026-01-28 01:51:30 +00:00
Owner
Copy link

I don't have time to check if the robot overlord comments are accurate right now, please check them over and see what you think :)

I don't have time to check if the robot overlord comments are accurate right now, please check them over and see what you think :)
Move the authenticate subroutine to utils.pm
Some checks failed
CI via Tox and perl / tox (pull_request) Successful in 4m43s
Details
CI via Tox and perl / perl (pull_request) Has been cancelled
Details
e19cff13b8 
Also, the subroutine now uses arguments to set some parametres.
First, you need to provide a password to authenticate - previously
it was hardwired and could not handle differences between root and
user passwords.
By default, it waits 15 seconds to see the auth dialogue and 5
seconds after the authentication. You can set these times.
Example: authenticate($password, timeout => 20, stillscreen => 10);
Move specific subroutines back into the test script
Some checks failed
CI via Tox and perl / tox (pull_request) Successful in 3m43s
Details
CI via Tox and perl / perl (pull_request) Failing after 31s
Details
a5b3166873 
Some subroutines are only used once, so there is no need to
have them in a library. This moves them back into the code.
lruzicka commented 2026-01-28 11:40:09 +00:00
Author
Owner
Copy link

@adamwill wrote in #462 (comment):

Overall this is looking good now, but I do have some nits, mainly to do with organization.

1. The `authenticate` sub doesn't have anything much to do with disks, per se. It should probably move into `utils.pm` now we're calling it from elsewhere. We might also want to grep for other cases of `auth_required` and see if any of those could use this sub too.

Done. Actually, I made the routine a little better to enable better control.

2. Conversely, the three `select_` subs in `lib/mwriter.pm` are used only once each and are quite small. Why not just in-line them where they're used?

Done.

3. The toolbox actions are only used once each, and I already have a pattern for doing it which seems a bit better, to me. See the two places in `utils.pm` where we do `toolbox -y enter` - if you pass `-y` like that it will automatically create the toolbox if it doesn't exist. You can specify the name to enter explicitly if necessary. Then you can do `assert_script_run "true", 180;` to wait until the toolbox is actually ready; it's more reliable than a magic sleep. There is also `toolbox run` for running a single command in a specific container, which also might be useful here (again with `-y` to create it if necessary I believe). I think it'd be best to follow those patterns for the toolbox stuff here; I'm not sure whether it's worth de-duplicating with the other two uses since the subs would be very short if efficiently implemented, but see what you think.

The toolbox run -y does not work, it wants to have created the container before, but that's not a big deal. The problem is that when I used to run the command through toolbox run it would not work with the assert_script_run. I will try using it and see how it performs.

@adamwill wrote in https://forge.fedoraproject.org/quality/os-autoinst-distri-fedora/pulls/462#issuecomment-351917: > Overall this is looking good now, but I do have some nits, mainly to do with organization. > > 1. The `authenticate` sub doesn't have anything much to do with disks, per se. It should probably move into `utils.pm` now we're calling it from elsewhere. We might also want to grep for other cases of `auth_required` and see if any of those could use this sub too. Done. Actually, I made the routine a little better to enable better control. > > 2. Conversely, the three `select_` subs in `lib/mwriter.pm` are used only once each and are quite small. Why not just in-line them where they're used? Done. > > 3. The toolbox actions are only used once each, and I already have a pattern for doing it which seems a bit better, to me. See the two places in `utils.pm` where we do `toolbox -y enter` - if you pass `-y` like that it will automatically create the toolbox if it doesn't exist. You can specify the name to enter explicitly if necessary. Then you can do `assert_script_run "true", 180;` to wait until the toolbox is actually ready; it's more reliable than a magic sleep. There is also `toolbox run` for running a single command in a specific container, which also might be useful here (again with `-y` to create it if necessary I believe). I think it'd be best to follow those patterns for the toolbox stuff here; I'm not sure whether it's worth de-duplicating with the other two uses since the subs would be very short if efficiently implemented, but see what you think. The `toolbox run -y` does not work, it wants to have created the container before, but that's not a big deal. The problem is that when I used to run the command through `toolbox run` it would not work with the assert_script_run. I will try using it and see how it performs.
Use Adam's way to handle containers
Some checks failed
CI via Tox and perl / tox (pull_request) Successful in 3m56s
Details
CI via Tox and perl / perl (pull_request) Failing after 31s
Details
bcd474d7f6
Fix the critical error: test for empty dir
Some checks failed
CI via Tox and perl / tox (pull_request) Failing after 31s
Details
CI via Tox and perl / perl (pull_request) Has been cancelled
Details
cac50ff60d
lruzicka force-pushed newtest/fedora-media-writer from cac50ff60d
Some checks failed
CI via Tox and perl / tox (pull_request) Failing after 31s
Details
CI via Tox and perl / perl (pull_request) Has been cancelled
Details
to fb8dc856fb
Some checks failed
CI via Tox and perl / tox (pull_request) Successful in 4m24s
Details
CI via Tox and perl / perl (pull_request) Has been cancelled
Details
2026-01-28 15:24:05 +00:00 Compare
Fix mwriter.pm as told by AI overlord
All checks were successful
CI via Tox and perl / tox (pull_request) Successful in 3m39s
Details
CI via Tox and perl / perl (pull_request) Successful in 3m28s
Details
88f25c639a
Fix the call of authenticate function
All checks were successful
CI via Tox and perl / tox (pull_request) Successful in 4m42s
Details
CI via Tox and perl / perl (pull_request) Successful in 3m33s
Details
AI Code Review / ai-review (pull_request_target) Has been skipped
Details
AI Code Review / remove-label (pull_request_target) Successful in 2s
Details
604c736b9b
lruzicka commented 2026-01-28 16:28:22 +00:00
Author
Owner
Copy link

The test still passes on Staging with the above fixes.

The test still passes on Staging with the above fixes.
forgejo-actions commented 2026-01-31 01:02:36 +00:00
Copy link

AI Code Review

📋 MR Summary

Introduction of a comprehensive test suite for Fedora Media Writer covering download, write, and restore workflows on Workstation and Silverblue.

  • Key Changes:
    • New mwriter.pm library with robust device verification using checkisomd5 and cmp.
    • New test modules: download_write.pm, open_write.pm, restore.pm, about.pm, and aaa_setup.pm.
    • Centralized and improved authenticate and toolbox helpers in lib/utils.pm.
    • Updated templates.fif.json to define the media_writer test scheduling and dual-disk configuration.
    • Removal of ad-hoc authentication logic in existing tests in favor of the new centralized subroutine.
  • Impact: lib/mwriter.pm, lib/utils.pm, tests/applications/media-writer/*, templates.fif.json, check-needles.py, tests/applications/gnome-panel/network.pm, tests/desktop_update_graphical.pm
  • Risk Level: 🟡 Medium - The changes introduce significant new UI automation which can be inherently flaky, and modify core utility functions (authenticate) affecting multiple existing tests. However, the logic changes improve maintainability and the new tests are isolated.

Detailed Code Review

This is a well-structured PR that significantly improves testing coverage for Fedora Media Writer. The centralization of authenticate and toolbox logic is a welcome refactoring. The verification strategy in mwriter.pm (using cmp with EOF detection and checkisomd5) is clever and much more robust than simple file existence checks. The support for Silverblue via toolbox encapsulation is correctly implemented.

📂 File Reviews

📄 `lib/mwriter.pm` - New library for Media Writer verification logic.

  • Minor [Security]: The ls command is injected into script_output without strict validation of the result, though unlikely to be exploitable here.

  • Suggestion [Logic]: The logic assumes cmp is available on the Silverblue host (not inside toolbox). While diffutils is usually part of the base image, it's worth verifying this dependency holds true for minimal Silverblue installs.

    • Suggestion: Ensure cmp is present on the Silverblue host system or run it inside the toolbox like checkisomd5 if missing.
📄 `lib/utils.pm` - Utility updates including centralized authentication and toolbox helpers.

  • Minor [Performance]: In the toolbox subroutine, assert_script_run('true', timeout => 600) follows type_string. If the prompt detection logic fails (e.g., PS1 differs inside toolbox), this might time out or type prematurely.

    • Suggestion: Consider ensuring the toolbox prompt is explicitly matched if the default backend prompt detection is insufficient for the container shell.

  • Suggestion [Logic]: The authenticate subroutine defines %defaults inside the function scope correctly, but ensuring wait_still_screen is only called when necessary (i.e., after typing) is good practice. The current implementation looks correct.

📄 `tests/applications/media-writer/aaa_setup.pm` - Setup script for installation and environment prep.
  • Minor [Logic]: The script runs in $self->root_console(tty => 3). On Silverblue, toolbox is typically designed for unprivileged users. Running toolbox enter as root might create a root toolbox, which is valid but distinct from the user one.
    • Suggestion: Verify that toolbox operations behave as expected when running as root in TTY3 on Silverblue.
📄 `tests/applications/media-writer/restore.pm` - Test for the restore functionality.
  • Major [Logic]: The command assert_script_run('mount /dev/sda1 /mnt/usbdisk') assumes the restore operation created a specific partition layout (sda1). If the restore fails or creates a different layout (e.g., unpartitioned device), this will fail.
    • Suggestion: Add a check for the partition table (e.g., lsblk) before attempting to mount, or handle the mount failure gracefully.
assert_script_run("lsblk /dev/sda");
if (script_run("mount /dev/sda1 /mnt/usbdisk") != 0) {
    record_info("Mount failed", "Could not mount sda1, checking sda...");
    assert_script_run("mount /dev/sda /mnt/usbdisk");
}
📄 `tests/applications/media-writer/download_write.pm` - Main workflow test for downloading and writing media.
  • Suggestion [Logic]: The loop waiting for mwriter_finish_button handles timeouts manually. If the timeout is reached, the loop exits and immediately asserts the button exists, which will fail. This creates the correct failure signal but relying on wait_screen_change or assert_screen with a long timeout is often cleaner than manual sleep loops.

✅ Summary

  • Overall Assessment: The PR is solid and ready for merging pending the minor partition assumption check in restore.pm. The code is clean, follows project patterns, and adequately addresses the complexities of testing physical device emulation.
  • Priority Issues:
    • In tests/applications/media-writer/restore.pm, mount /dev/sda1 might fail if the restored disk layout varies (e.g. whole disk vs partition). Validate lsblk output or try mounting alternative paths.
  • Minor Suggestions:
    • Verify toolbox behavior when running as root in aaa_setup.pm to ensure it works consistently on Silverblue.
    • In lib/mwriter.pm, consider verifying cmp availability on the Silverblue host system.

🤖 AI Code Review | Generated with ai-code-review
Platform: Forgejo | AI Provider: gemini | Model: gemini-3-pro-preview

## AI Code Review ### 📋 MR Summary Introduction of a comprehensive test suite for Fedora Media Writer covering download, write, and restore workflows on Workstation and Silverblue. - **Key Changes:** - New `mwriter.pm` library with robust device verification using `checkisomd5` and `cmp`. - New test modules: `download_write.pm`, `open_write.pm`, `restore.pm`, `about.pm`, and `aaa_setup.pm`. - Centralized and improved `authenticate` and `toolbox` helpers in `lib/utils.pm`. - Updated `templates.fif.json` to define the `media_writer` test scheduling and dual-disk configuration. - Removal of ad-hoc authentication logic in existing tests in favor of the new centralized subroutine. - **Impact:** lib/mwriter.pm, lib/utils.pm, tests/applications/media-writer/*, templates.fif.json, check-needles.py, tests/applications/gnome-panel/network.pm, tests/desktop_update_graphical.pm - **Risk Level:** 🟡 Medium - The changes introduce significant new UI automation which can be inherently flaky, and modify core utility functions (`authenticate`) affecting multiple existing tests. However, the logic changes improve maintainability and the new tests are isolated. ### Detailed Code Review This is a well-structured PR that significantly improves testing coverage for Fedora Media Writer. The centralization of `authenticate` and `toolbox` logic is a welcome refactoring. The verification strategy in `mwriter.pm` (using `cmp` with EOF detection and `checkisomd5`) is clever and much more robust than simple file existence checks. The support for Silverblue via `toolbox` encapsulation is correctly implemented. #### 📂 File Reviews <details> <summary><strong>📄 `lib/mwriter.pm`</strong> - New library for Media Writer verification logic.</summary> - **Minor** [Security]: The `ls` command is injected into `script_output` without strict validation of the result, though unlikely to be exploitable here. - **Suggestion** [Logic]: The logic assumes `cmp` is available on the Silverblue host (not inside toolbox). While `diffutils` is usually part of the base image, it's worth verifying this dependency holds true for minimal Silverblue installs. - *Suggestion:* Ensure `cmp` is present on the Silverblue host system or run it inside the toolbox like `checkisomd5` if missing. </details> <details> <summary><strong>📄 `lib/utils.pm`</strong> - Utility updates including centralized authentication and toolbox helpers.</summary> - **Minor** [Performance]: In the `toolbox` subroutine, `assert_script_run('true', timeout => 600)` follows `type_string`. If the prompt detection logic fails (e.g., PS1 differs inside toolbox), this might time out or type prematurely. - *Suggestion:* Consider ensuring the toolbox prompt is explicitly matched if the default backend prompt detection is insufficient for the container shell. - **Suggestion** [Logic]: The `authenticate` subroutine defines `%defaults` inside the function scope correctly, but ensuring `wait_still_screen` is only called when necessary (i.e., after typing) is good practice. The current implementation looks correct. </details> <details> <summary><strong>📄 `tests/applications/media-writer/aaa_setup.pm`</strong> - Setup script for installation and environment prep.</summary> - **Minor** [Logic]: The script runs in `$self->root_console(tty => 3)`. On Silverblue, `toolbox` is typically designed for unprivileged users. Running `toolbox enter` as root might create a root toolbox, which is valid but distinct from the user one. - *Suggestion:* Verify that `toolbox` operations behave as expected when running as root in TTY3 on Silverblue. </details> <details> <summary><strong>📄 `tests/applications/media-writer/restore.pm`</strong> - Test for the restore functionality.</summary> - **Major** [Logic]: The command `assert_script_run('mount /dev/sda1 /mnt/usbdisk')` assumes the restore operation created a specific partition layout (`sda1`). If the restore fails or creates a different layout (e.g., unpartitioned device), this will fail. - *Suggestion:* Add a check for the partition table (e.g., `lsblk`) before attempting to mount, or handle the mount failure gracefully. ```` assert_script_run("lsblk /dev/sda"); if (script_run("mount /dev/sda1 /mnt/usbdisk") != 0) { record_info("Mount failed", "Could not mount sda1, checking sda..."); assert_script_run("mount /dev/sda /mnt/usbdisk"); } ```` </details> <details> <summary><strong>📄 `tests/applications/media-writer/download_write.pm`</strong> - Main workflow test for downloading and writing media.</summary> - **Suggestion** [Logic]: The loop waiting for `mwriter_finish_button` handles timeouts manually. If the timeout is reached, the loop exits and immediately asserts the button exists, which will fail. This creates the correct failure signal but relying on `wait_screen_change` or `assert_screen` with a long timeout is often cleaner than manual sleep loops. </details> ### ✅ Summary - **Overall Assessment:** The PR is solid and ready for merging pending the minor partition assumption check in `restore.pm`. The code is clean, follows project patterns, and adequately addresses the complexities of testing physical device emulation. - **Priority Issues:** - In `tests/applications/media-writer/restore.pm`, `mount /dev/sda1` might fail if the restored disk layout varies (e.g. whole disk vs partition). Validate `lsblk` output or try mounting alternative paths. - **Minor Suggestions:** - Verify `toolbox` behavior when running as root in `aaa_setup.pm` to ensure it works consistently on Silverblue. - In `lib/mwriter.pm`, consider verifying `cmp` availability on the Silverblue host system. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) **Platform:** Forgejo | **AI Provider:** gemini | **Model:** gemini-3-pro-preview
adamwill commented 2026-01-31 01:14:10 +00:00
Owner
Copy link

I don't think we need to worry about anything in the AI review. Let's merge this. Thanks. If it causes problems we'll work through them...

I don't think we need to worry about anything in the AI review. Let's merge this. Thanks. If it causes problems we'll work through them...
adamwill deleted branch newtest/fedora-media-writer 2026-01-31 01:14:19 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
ai-review-please

label for requesting AI code review

  
Closed As
Duplicate
Closed with the reason of Duplicate

  
Closed As
Fixed
Closed with the reason of Fixed

  
Closed As
Invalid
Closed with the reason of Invalid

  
easyfix

   
freeipa

issues or enhancement requests related to FreeIPA in some way

  
improvetest

some kind of improvement (but not bug fix) to an existing test

  
iot

issues relating to test enablement on Fedora IoT

  
newtest

requests for new tests

  
silverblue

issues relating to test enablement on Fedora Silverblue

  
tracker

tracker issue, only exists to track a set of related issues

  
Backlog Status
Needs Review
Work items that have been created or updated but need review, refinement, or approval before they can be considered ready for sprint planning.

  
Backlog Status
Ready
Work items that have been reviewed, refined, and are ready to be committed to a sprint. These items have clear requirements and can be worked on immediately when capacity is available.

  
chore

Work that needs to be done but doesn't add new functionality or fix bugs. Maintenance tasks that keep the system running smoothly and the codebase clean.

  
documentation

Work focused on creating, updating, or improving documentation for users, developers, or system administrators.

  
points
01
A task that is simple, small, and has very little uncertainty. It should be easy enough for any team member to pick up and complete quickly, without a lot of discussion or research.

  
points
02
This is a task that is slightly more complex or larger than a "1." It might involve two or three simple steps, but the outcome is still very clear and there's little uncertainty.

  
points
03
Medium-sized task with some noticeable complexity. It might require a bit more thought or collaboration. The steps are well-defined, but it's not a trivial change.

  
points
05
A significant piece of work. It has more complexity and a higher degree of uncertainty than smaller tasks. It might require some research, a design discussion, or collaboration across multiple teams.

  
points
08
Complex task with a lot of uncertainty. It's often the largest size that a team will consider for a single sprint. It likely requires a detailed breakdown and multiple team members to complete.

  
points
13
Any task that feels like it's a "13" or more is considered an Epic. These are too large and uncertain to be worked on in a single sprint and should be broken down.

  
Priority
Critical
This is for work that is urgent and of critical importance and so and must be addressed immediately with a higher priority than all other work, even if not originally planned for the current sprint.

  
Priority
High
This is for work that is critical and needs to be addressed in the next sprint. It's often for urgent bugs, important features, or a major piece of technical debt that is blocking other work.

  
Priority
Low
This is for work that is good to do but not urgent. It's a candidate for being worked on when there's extra time or when it rises in importance. It can stay in the backlog for a longer period.

  
Priority
Medium
This is for important work that should be done soon, but isn't as urgent as a high-priority item. It will likely be considered for the next 1-2 sprints.

  
Sprint Status
Blocked
Work items that cannot progress due to external dependencies, blockers, or issues that need resolution.

  
Sprint Status
Done
Work items that have been completed, reviewed, and integrated. No further work is needed within the current sprint.

  
Sprint Status
In Progress
Work items currently being worked on by a team member. Active development, research, or implementation is happening.

  
Sprint Status
Review
Work items that are complete but need review, testing, or approval before being marked as done.

  
Sprint Status
To Do
Work items that are committed to the sprint but haven't been started yet. These items are ready to be picked up by team members.

  
Technical Debt

Work that addresses shortcuts, workarounds, or suboptimal implementations that were done previously to meet deadlines. These items improve code quality, maintainability, or performance without adding new features.

  
Work Item
Bug
Issues where the system is not working as intended or designed. Bugs represent deviations from expected behavior.

  
Work Item
Epic
Large initiatives that are broken down into multiple smaller work items (user stories, tasks, bugs). Epics represent significant features or projects that span multiple sprints.

  
Work Item
Spike
Time-boxed research, investigation, or proof-of-concept work to reduce uncertainty or explore solutions. Spikes help inform future development decisions.

  
Work Item
Task
Used to break down larger user stories into smaller, manageable implementation pieces. They focus on the specific technical steps needed to deliver the user story's value.

  
Work Item
User Story
User-facing features or functionality that can be framed from a user perspective, describing what someone wants to accomplish and why.

No labels
ai-review-please
Closed As
Duplicate
Closed As
Fixed
Closed As
Invalid
easyfix
freeipa
improvetest
iot
newtest
silverblue
tracker
Backlog Status
Needs Review
Backlog Status
Ready
chore
documentation
points
01
points
02
points
03
points
05
points
08
points
13
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Sprint Status
Blocked
Sprint Status
Done
Sprint Status
In Progress
Sprint Status
Review
Sprint Status
To Do
Technical Debt
Work Item
Bug
Work Item
Epic
Work Item
Spike
Work Item
Task
Work Item
User Story
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Sprint 2
Assignees
Clear assignees
No assignees
adamwill
lruzicka
5 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/os-autoinst-distri-fedora!462
No description provided.