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
