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

["Max R. Carrara" <m.carrara@proxmox.com>]

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