[PATCH pve-storage v1 45/54] test: volume id: modernize code

Max R. Carrara
2026-04-22T11:20:51Z

Modernize the existing code in `path_to_volume_id_test.pm` while
leaving the tests unchanged otherwise.

In particular,
- move the test execution code into its own subroutine
- remove needless `diag(explain(...))` call
- suppress warnings when calling `path_to_volume_id()` to make test
output less noisy
- add and call a `main()` subroutine
- declare `use v5.36;` instead of `use strict;` and `use warnings;`
- capitalize constants
- move the comment regarding `File::Temp->newdir()` unlinking on exit
to where the method is actually called
- adapt the code style to fit our more modern style guide

Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/test/path_to_volume_id_test.pm | 149 ++++++++++++++++-------------
1 file changed, 82 insertions(+), 67 deletions(-)

diff --git a/src/test/path_to_volume_id_test.pm b/src/test/path_to_volume_id_test.pm
index e7e36037..72644482 100644
--- a/src/test/path_to_volume_id_test.pm
+++ b/src/test/path_to_volume_id_test.pm
@@ -1,7 +1,6 @@
package PVE::Storage::TestPathToVolumeId;

-use strict;
-use warnings;
+use v5.36;

use lib qw(..);

@@ -13,17 +12,17 @@ use PVE::Storage::Common qw(
use Test::More;

use Cwd;
-use File::Basename;
+use File::Basename qw(fileparse);
use File::Path qw(make_path remove_tree);
use File::Temp;

-my $storage_dir = File::Temp->newdir();
-my $scfg = {
+my $DEFAULT_STORAGE_DIR = File::Temp->newdir(); # unlinks on exit
+my $DEFAULT_CFG = {
'digest' => 'd29306346b8b25b90a4a96165f1e8f52d1af1eda',
'ids' => {
'local' => {
'shared' => 0,
- 'path' => "$storage_dir",
+ 'path' => "$DEFAULT_STORAGE_DIR",
'type' => 'dir',
'content' => {
'snippets' => 1,
@@ -44,24 +43,24 @@ my $scfg = {
# description => to identify the test case
# volname => to create the test file
# expected => the result that path_to_volume_id should return
-my @tests = (
+my $tests = [
{
description => 'Image, qcow2',
- volname => "$storage_dir/images/16110/vm-16110-disk-0.qcow2",
+ volname => "$DEFAULT_STORAGE_DIR/images/16110/vm-16110-disk-0.qcow2",
expected => [
'images', 'local:16110/vm-16110-disk-0.qcow2',
],
},
{
description => 'Image, raw',
- volname => "$storage_dir/images/16112/vm-16112-disk-0.raw",
+ volname => "$DEFAULT_STORAGE_DIR/images/16112/vm-16112-disk-0.raw",
expected => [
'images', 'local:16112/vm-16112-disk-0.raw',
],
},
{
description => 'Image template, qcow2',
- volname => "$storage_dir/images/9004/base-9004-disk-0.qcow2",
+ volname => "$DEFAULT_STORAGE_DIR/images/9004/base-9004-disk-0.qcow2",
expected => [
'images', 'local:9004/base-9004-disk-0.qcow2',
],
@@ -69,49 +68,49 @@ my @tests = (

{
description => 'Backup, vma.gz',
- volname => "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz",
expected => [
'backup', 'local:backup/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz',
],
},
{
description => 'Backup, vma.lzo',
- volname => "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo",
expected => [
'backup', 'local:backup/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo',
],
},
{
description => 'Backup, vma',
- volname => "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma",
expected => [
'backup', 'local:backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma',
],
},
{
description => 'Backup, tar.lzo',
- volname => "$storage_dir/dump/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo",
expected => [
'backup', 'local:backup/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo',
],
},
{
description => 'Backup, vma.zst',
- volname => "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst",
expected => [
'backup', 'local:backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst',
],
},
{
description => 'Backup, tar.zst',
- volname => "$storage_dir/dump/vzdump-lxc-16112-2020_03_30-21_39_30.tar.zst",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-lxc-16112-2020_03_30-21_39_30.tar.zst",
expected => [
'backup', 'local:backup/vzdump-lxc-16112-2020_03_30-21_39_30.tar.zst',
],
},
{
description => 'Backup, tar.bz2',
- volname => "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2",
expected => [
'backup', 'local:backup/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2',
],
@@ -119,21 +118,23 @@ my @tests = (

{
description => 'ISO file',
- volname => "$storage_dir/template/iso/yet-again-a-installation-disk.iso",
+ volname => "$DEFAULT_STORAGE_DIR/template/iso/yet-again-a-installation-disk.iso",
expected => [
'iso', 'local:iso/yet-again-a-installation-disk.iso',
],
},
{
description => 'CT template, tar.gz',
- volname => "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.gz",
+ volname =>
+ "$DEFAULT_STORAGE_DIR/template/cache/debian-10.0-standard_10.0-1_amd64.tar.gz",
expected => [
'vztmpl', 'local:vztmpl/debian-10.0-standard_10.0-1_amd64.tar.gz',
],
},
{
description => 'CT template, wrong ending, tar bz2',
- volname => "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.bz2",
+ volname =>
+ "$DEFAULT_STORAGE_DIR/template/cache/debian-10.0-standard_10.0-1_amd64.tar.bz2",
expected => [
'vztmpl', 'local:vztmpl/debian-10.0-standard_10.0-1_amd64.tar.bz2',
],
@@ -141,49 +142,50 @@ my @tests = (

{
description => 'Rootdir, folder subvol, legacy naming',
- volname => "$storage_dir/images/1234/subvol-1234-disk-0.subvol/", # fileparse needs / at the end
+ volname => "$DEFAULT_STORAGE_DIR/images/1234/subvol-1234-disk-0.subvol/", # fileparse needs / at the end
expected => [
'images', 'local:1234/subvol-1234-disk-0.subvol',
],
},
{
description => 'Rootdir, folder subvol',
- volname => "$storage_dir/images/1234/subvol-1234-disk-0.subvol/", # fileparse needs / at the end
+ volname => "$DEFAULT_STORAGE_DIR/images/1234/subvol-1234-disk-0.subvol/", # fileparse needs / at the end
expected => [
'images', 'local:1234/subvol-1234-disk-0.subvol',
],
},
{
description => 'Snippets, yaml',
- volname => "$storage_dir/snippets/userconfig.yaml",
+ volname => "$DEFAULT_STORAGE_DIR/snippets/userconfig.yaml",
expected => [
'snippets', 'local:snippets/userconfig.yaml',
],
},
{
description => 'Snippets, hookscript',
- volname => "$storage_dir/snippets/hookscript.pl",
+ volname => "$DEFAULT_STORAGE_DIR/snippets/hookscript.pl",
expected => [
'snippets', 'local:snippets/hookscript.pl',
],
},
{
description => 'CT template, tar.xz',
- volname => "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.xz",
+ volname =>
+ "$DEFAULT_STORAGE_DIR/template/cache/debian-10.0-standard_10.0-1_amd64.tar.xz",
expected => [
'vztmpl', 'local:vztmpl/debian-10.0-standard_10.0-1_amd64.tar.xz',
],
},
{
description => 'Import, ova',
- volname => "$storage_dir/import/import.ova",
+ volname => "$DEFAULT_STORAGE_DIR/import/import.ova",
expected => [
'import', 'local:import/import.ova',
],
},
{
description => 'Import, ovf',
- volname => "$storage_dir/import/import.ovf",
+ volname => "$DEFAULT_STORAGE_DIR/import/import.ovf",
expected => [
'import', 'local:import/import.ovf',
],
@@ -192,91 +194,104 @@ my @tests = (
# no matches, path or files with failures
{
description => 'Base template, string as vmid in folder name',
- volname => "$storage_dir/images/ssss/base-4321-disk-0.raw",
+ volname => "$DEFAULT_STORAGE_DIR/images/ssss/base-4321-disk-0.raw",
expected => [''],
},
{
description => 'ISO file, wrong ending',
- volname => "$storage_dir/template/iso/yet-again-a-installation-disk.dvd",
+ volname => "$DEFAULT_STORAGE_DIR/template/iso/yet-again-a-installation-disk.dvd",
expected => [''],
},
{
description => 'CT template, wrong ending, zip.gz',
- volname => "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.zip.gz",
+ volname =>
+ "$DEFAULT_STORAGE_DIR/template/cache/debian-10.0-standard_10.0-1_amd64.zip.gz",
expected => [''],
},
{
description => 'Backup, wrong format, openvz, zip.gz',
- volname => "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.zip.gz",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-openvz-16112-2020_03_30-21_39_30.zip.gz",
expected => [''],
},
{
description => 'Backup, wrong format, openvz, tgz.lzo',
- volname => "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tgz.lzo",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tgz.lzo",
expected => [''],
},
{
description => 'Backup, wrong ending, qemu, vma.xz',
- volname => "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vma.xz",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vma.xz",
expected => [''],
},
{
description => 'Backup, wrong format, qemu, vms.gz',
- volname => "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vms.gz",
+ volname => "$DEFAULT_STORAGE_DIR/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vms.gz",
expected => [''],
},
{
description => 'Image, string as vmid in folder name',
- volname => "$storage_dir/images/ssss/vm-1234-disk-0.qcow2",
+ volname => "$DEFAULT_STORAGE_DIR/images/ssss/vm-1234-disk-0.qcow2",
expected => [''],
},
{
description => 'Import, non ova/ovf/disk image in import dir',
- volname => "$storage_dir/import/test.foo",
+ volname => "$DEFAULT_STORAGE_DIR/import/test.foo",
expected => [''],
},
-);
+];

-plan tests => scalar @tests + 1;
+my sub run_tests($tests) {
+ my $seen_vtype;
+ my $vtype_subdirs = { map { $_ => 1 } keys plugin_get_default_vtype_subdirs()->%* };

-my $seen_vtype;
-my $vtype_subdirs =
- { map { $_ => 1 } keys %{ plugin_get_default_vtype_subdirs() } };
+ for my $t ($tests->@*) {
+ my $file = $t->{volname};
+ my $expected = $t->{expected};
+ my $description = $t->{description};

-foreach my $tt (@tests) {
- my $file = $tt->{volname};
- my $expected = $tt->{expected};
- my $description = $tt->{description};
+ # prepare environment
+ my ($name, $dir, $suffix) = fileparse($file);
+ make_path($dir, { verbose => 1, mode => 0755 });

- # prepare environment
- my ($name, $dir, $suffix) = fileparse($file);
- make_path($dir, { verbose => 1, mode => 0755 });
+ if ($name) {
+ open(my $fh, ">>", "$file") || die "Error open file: $!";
+ close($fh);
+ }

- if ($name) {
- open(my $fh, ">>", "$file") || die "Error open file: $!";
- close($fh);
+ my $got;
+ eval {
+ # Suppress warnings here to make output less noisy for certain tests
+ local $SIG{__WARN__} = sub { };
+
+ $got = [PVE::Storage::path_to_volume_id($DEFAULT_CFG, $file)];
+ };
+ $got = $@ if $@;
+
+ is_deeply($got, $expected, $description);
+
+ $seen_vtype->{ $expected->[0] } = 1
+ if ($expected->[0] ne '' && scalar($expected->@*) > 1);
}

- # run tests
- my $got;
- eval { $got = [PVE::Storage::path_to_volume_id($scfg, $file)] };
- $got = $@ if $@;
+ # to check if all $vtype_subdirs are defined in path_to_volume_id
+ # or have a test
+ # FIXME re-enable after vtype split changes
+ #is_deeply($seen_vtype, $vtype_subdirs, "vtype_subdir check");
+ is_deeply({}, {}, "vtype_subdir check");

- is_deeply($got, $expected, $description) || diag(explain($got));
-
- $seen_vtype->{ @$expected[0] } = 1
- if (@$expected[0] ne '' && scalar @$expected > 1);
+ return;
}

-# to check if all $vtype_subdirs are defined in path_to_volume_id
-# or have a test
-# FIXME re-enable after vtype split changes
-#is_deeply($seen_vtype, $vtype_subdirs, "vtype_subdir check");
-is_deeply({}, {}, "vtype_subdir check");
+my sub main() {
+ plan tests => scalar($tests->@*) + 1;

-#cleanup
-# File::Temp unlinks tempdir on exit
+ run_tests($tests);

-done_testing();
+ done_testing();
+
+ return;
+}
+
+main();

1;
--
2.47.3