Re: [pve-devel] [PATCH pve-installer v3 2/7] move RAID setup checks to RAID level enum implementations

["Christoph Heiss" <c.heiss@proxmox.com>]

On Tue Jul 8, 2025 at 2:01 PM CEST, Michael Köppl wrote:
> On 7/7/25 13:47, Christoph Heiss wrote:
>>> +
>>> +        for v in zfs_raid_variants {
>>> +            assert!(v.check_disks(&[]).is_err());
>>> +            assert!(v.check_disks(&disks[..v.get_min_disks() - 1]).is_err());
>>> +            assert!(v.check_disks(&disks[..v.get_min_disks()]).is_ok());
>>> +            assert!(v.check_disks(&disks).is_ok());
>>> +        }
>>>      }
>>
>> These unit tests should be moved to `proxmox_installer_common::options`,
>> if the implementation of these methods also resides there.
>
> First of all, thanks for having a look at this! True, I forgot to move
> the tests as well. Will move them for v4. Right now, both sets of unit
> tests (for proxmox_installer_common::options and
> proxmox_installer_common::disk_checks) use the dummy_disks and
> dummy_disk helpers. I think simply copying the implementation of these
> over to the options.rs tests is fine since the implementation is not
> exactly complicated and it seems a bit overkill to introduce some sort
> of test utils for this. What do you think?

As for dummy_disk(), you could potentially make it a (static) method on
the `Disk` type itself, gating it on `#[cfg(test)]`.

That would avoid some extra test utilities somewhere, while still having
the benefit of being able to deduplicate it (even if trivial).
We already use a similar pattern already in the setup code, see e.g.
proxmox_installer_common::setup::ProductConfig::mocked()

dummy_disks() being a one-liner can of course be simply copied over.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel