From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 880881FF13B for ; Wed, 22 Apr 2026 13:17:35 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4110D18BAC; Wed, 22 Apr 2026 13:14:44 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage v1 36/54] test: list volumes: introduce new format for test cases Date: Wed, 22 Apr 2026 13:13:02 +0200 Message-ID: <20260422111322.257380-37-m.carrara@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260422111322.257380-1-m.carrara@proxmox.com> References: <20260422111322.257380-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776856354195 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.083 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: G7SU22M4EIDNJIPUFN74EPM46KH7LU56 X-Message-ID-Hash: G7SU22M4EIDNJIPUFN74EPM46KH7LU56 X-MailFrom: m.carrara@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Introduce a new format for test cases in `list_volumes_test.pm`, which allows specifying the `$storeid`, `$scfg` and the list of content types / volume types for the given test. At the same time, get rid of the `files` and `expected` lists in the new format, which made it somewhat tedious to keep track of which file belonged to which hashref in the expected output, or if it even was supposed to show up in the output in the first place. Get rid of the optional `parent` list for the same reason, too. Replace these three lists with a single `cases` list that simply contains hashes that match a given file path with the expected hash in the final output. The `parent` may optionally be specified in the same hash for a given file path as well, which makes the backing file logic a little simpler. Describe the new format in a small POD docstring. Additionally, introduce replacements for the constants declared with the `use constant` pragma. Finally, migrate the first of the old test cases over to the new format and run it through the new machinery added for the new test case format. The reason for introducing this new format here is to make it easier to add more specific test cases. The "legacy" tests always pass all volume types as parameter to `PVE::Storage::Plugin->list_volumes()`, for example. As an additional benefit, we could reuse / recycle the new testing machinery added here for testing other storage (plugin) API methods as well. Signed-off-by: Max R. Carrara --- src/test/list_volumes_test.pm | 399 +++++++++++++++++++++++++--------- 1 file changed, 300 insertions(+), 99 deletions(-) diff --git a/src/test/list_volumes_test.pm b/src/test/list_volumes_test.pm index 2d941b60..c83b782e 100644 --- a/src/test/list_volumes_test.pm +++ b/src/test/list_volumes_test.pm @@ -11,6 +11,7 @@ use PVE::Tools qw(run_command); use Test::More; use Test::MockModule; +use Carp qw(confess); use Cwd; use File::Basename qw(fileparse); use File::Path qw(make_path remove_tree); @@ -22,6 +23,10 @@ use constant DEFAULT_SIZE => 131072; # 128 kiB use constant DEFAULT_USED => 262144; # 256 kiB use constant DEFAULT_CTIME => 1234567890; +my $DEFAULT_SIZE = 128 * 1024; # 128 kiB +my $DEFAULT_USED = 256 * 1024; # 256 kiB +my $DEFAULT_CTIME = 1234567890; + # get_vmlist() return values my $mocked_vmlist = { 'version' => 1, @@ -75,6 +80,23 @@ my $scfg = { }, }; +my $DEFAULT_STOREID = 'local'; +my $DEFAULT_STORAGE_PATH = File::Temp->newdir(); +my $DEFAULT_SCFG = { + type => 'dir', + path => $DEFAULT_STORAGE_PATH, + shared => 0, + content => { + iso => 1, + rootdir => 1, + vztmpl => 1, + images => 1, + snippets => 1, + backup => 1, + import => 1, + }, +}; + my @BACKING_FILE_SUFFIXES = ('qcow2', 'raw', 'vmdk', 'vhdx'); # The test cases are comprised of an arry of hashes with the following keys: @@ -84,103 +106,6 @@ my @BACKING_FILE_SUFFIXES = ('qcow2', 'raw', 'vmdk', 'vhdx'); # expected => returned result hash # (content, ctime, format, parent, size, used, vimd, volid) my @tests = ( - { - description => 'VMID: 16110, VM, qcow2, backup, snippets', - vmid => '16110', - files => [ - "$storage_dir/images/16110/vm-16110-disk-0.qcow2", - "$storage_dir/images/16110/vm-16110-disk-1.raw", - "$storage_dir/images/16110/vm-16110-disk-2.vmdk", - "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz", - "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo", - "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma", - "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst", - "$storage_dir/snippets/userconfig.yaml", - "$storage_dir/snippets/hookscript.pl", - ], - expected => [ - { - 'content' => 'images', - 'ctime' => DEFAULT_CTIME, - 'format' => 'qcow2', - 'parent' => undef, - 'size' => DEFAULT_SIZE, - 'used' => DEFAULT_USED, - 'vmid' => '16110', - 'volid' => 'local:16110/vm-16110-disk-0.qcow2', - }, - { - 'content' => 'images', - 'ctime' => DEFAULT_CTIME, - 'format' => 'raw', - 'parent' => undef, - 'size' => DEFAULT_SIZE, - 'used' => DEFAULT_USED, - 'vmid' => '16110', - 'volid' => 'local:16110/vm-16110-disk-1.raw', - }, - { - 'content' => 'images', - 'ctime' => DEFAULT_CTIME, - 'format' => 'vmdk', - 'parent' => undef, - 'size' => DEFAULT_SIZE, - 'used' => DEFAULT_USED, - 'vmid' => '16110', - 'volid' => 'local:16110/vm-16110-disk-2.vmdk', - }, - { - 'content' => 'backup', - 'ctime' => 1585602700, - 'format' => 'vma.gz', - 'size' => DEFAULT_SIZE, - 'subtype' => 'qemu', - 'vmid' => '16110', - 'volid' => 'local:backup/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz', - }, - { - 'content' => 'backup', - 'ctime' => 1585602765, - 'format' => 'vma.lzo', - 'size' => DEFAULT_SIZE, - 'subtype' => 'qemu', - 'vmid' => '16110', - 'volid' => 'local:backup/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo', - }, - { - 'content' => 'backup', - 'ctime' => 1585602835, - 'format' => 'vma', - 'size' => DEFAULT_SIZE, - 'subtype' => 'qemu', - 'vmid' => '16110', - 'volid' => 'local:backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma', - }, - { - 'content' => 'backup', - 'ctime' => 1585602835, - 'format' => 'vma.zst', - 'size' => DEFAULT_SIZE, - 'subtype' => 'qemu', - 'vmid' => '16110', - 'volid' => 'local:backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst', - }, - { - 'content' => 'snippets', - 'ctime' => DEFAULT_CTIME, - 'format' => 'snippet', - 'size' => DEFAULT_SIZE, - 'volid' => 'local:snippets/hookscript.pl', - }, - { - 'content' => 'snippets', - 'ctime' => DEFAULT_CTIME, - 'format' => 'snippet', - 'size' => DEFAULT_SIZE, - 'volid' => 'local:snippets/userconfig.yaml', - }, - ], - }, { description => 'VMID: 16112, lxc, raw, backup', vmid => '16112', @@ -631,6 +556,189 @@ my @tests = ( }, ); +=head2 TEST CASE FORMAT + +The parameters for individual test cases are hashes with the following +keys: + + { + # Name of the test, also displayed on error. + description => '...', + + # The storage ID for the test. + # Used as parameter when calling list_volumes. + storeid => 'some-storeid', + + # The storage config hash for the test. + # Used as parameter when calling list_volumes. + scfg => { + type => 'dir', + path => '...', + shared => 0, + content => { + iso => 1, + # [...] + }, + # [...] + }, + + # The VMID used as parameter when calling list_volumes. + # May be undef. + vmid => 1234, + + # The list of volume types / content types used as parameter when + # calling list_volumes. + vtypes => [], + + # List of hashes that associate a file with its expected hash in the + # output of list_volumes. + cases => [ + { + # Absolute path to the file. + file => "/tmp/foobar/images/1234/vm-1234-disk-0.qcow2", + + # Relative path to parent (backing file). + parent => '../ssss/base-4321-disk-0.qcow2', + + # Expected hash in the output of list_volumes. + expected => { + content => 'images', + ctime => 1234567890, # inode change time in seconds since the epoc + format => 'qcow2', + parent => '../ssss/base-4321-disk-0.qcow2', + size => 1337, # bytes! + used => 1024, # bytes! + vmid => 1234, + volid => 'local:1234/vm-1234-disk-0.qcow2', + }, + }, + ], + } + +=cut + +my $test_param_list = [ + { + description => 'VMID: 16110, VM, qcow2, backup, snippets', + storeid => $DEFAULT_STOREID, + scfg => $DEFAULT_SCFG, + vmid => 16110, + vtypes => ['images', 'backup', 'snippets'], + cases => [ + { + file => "$DEFAULT_STORAGE_PATH/images/16110/vm-16110-disk-0.qcow2", + expected => { + content => 'images', + ctime => $DEFAULT_CTIME, + format => 'qcow2', + parent => undef, + size => $DEFAULT_SIZE, + used => $DEFAULT_USED, + vmid => '16110', + volid => 'local:16110/vm-16110-disk-0.qcow2', + }, + }, + { + file => "$DEFAULT_STORAGE_PATH/images/16110/vm-16110-disk-1.raw", + expected => { + content => 'images', + ctime => $DEFAULT_CTIME, + format => 'raw', + parent => undef, + size => $DEFAULT_SIZE, + used => $DEFAULT_USED, + vmid => '16110', + volid => 'local:16110/vm-16110-disk-1.raw', + }, + }, + { + file => "$DEFAULT_STORAGE_PATH/images/16110/vm-16110-disk-2.vmdk", + expected => { + content => 'images', + ctime => $DEFAULT_CTIME, + format => 'vmdk', + parent => undef, + size => $DEFAULT_SIZE, + used => $DEFAULT_USED, + vmid => '16110', + volid => 'local:16110/vm-16110-disk-2.vmdk', + }, + }, + { + file => + "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz", + expected => { + content => 'backup', + ctime => 1585602700, + format => 'vma.gz', + size => $DEFAULT_SIZE, + subtype => 'qemu', + vmid => '16110', + volid => 'local:backup/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz', + }, + }, + { + file => + "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo", + expected => { + content => 'backup', + ctime => 1585602765, + format => 'vma.lzo', + size => $DEFAULT_SIZE, + subtype => 'qemu', + vmid => '16110', + volid => 'local:backup/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo', + }, + }, + { + file => "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma", + expected => { + content => 'backup', + ctime => 1585602835, + format => 'vma', + size => $DEFAULT_SIZE, + subtype => 'qemu', + vmid => '16110', + volid => 'local:backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma', + }, + }, + { + file => + "$DEFAULT_STORAGE_PATH/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst", + expected => { + content => 'backup', + ctime => 1585602835, + format => 'vma.zst', + size => $DEFAULT_SIZE, + subtype => 'qemu', + vmid => '16110', + volid => 'local:backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma.zst', + }, + }, + { + file => "$DEFAULT_STORAGE_PATH/snippets/hookscript.pl", + expected => { + content => 'snippets', + ctime => $DEFAULT_CTIME, + format => 'snippet', + size => $DEFAULT_SIZE, + volid => 'local:snippets/hookscript.pl', + }, + }, + { + file => "$DEFAULT_STORAGE_PATH/snippets/userconfig.yaml", + expected => { + content => 'snippets', + ctime => $DEFAULT_CTIME, + format => 'snippet', + size => $DEFAULT_SIZE, + volid => 'local:snippets/userconfig.yaml', + }, + }, + ], + }, +]; + # provide static vmlist for tests my $mock_cluster = Test::MockModule->new('PVE::Cluster', no_auto => 1); $mock_cluster->redefine(get_vmlist => sub { return $mocked_vmlist; }); @@ -715,15 +823,108 @@ my sub run_legacy_tests() { return; } +my sub setup_test_env($test_params) { + my ($storeid, $scfg) = $test_params->@{qw(storeid scfg)}; + + make_path($scfg->{path}, { verbose => 1, mode => 0700 }); + + for my $case ($test_params->{cases}->@*) { + my ($file, $parent) = $case->@{qw(file parent)}; + + confess "\$file = undef" if !defined($file); + + my ($file_name, $directory, $suffix) = fileparse($file, @BACKING_FILE_SUFFIXES); + + make_path($directory, { verbose => 1, mode => 0755 }); + + if ($file_name) { + # using qemu-img to also be able to represent the backing device + my $command = ['/usr/bin/qemu-img', 'create', "$file", $DEFAULT_SIZE]; + + push($command->@*, '-f', $suffix) if $suffix; + push($command->@*, '-u', '-b', $parent) if $parent; + push($command->@*, '-F', $suffix) if $parent && $suffix; + + run_command($command, timeout => 10); + } + } + + return; +} + +my sub teardown_test_env($test_params) { + my $scfg = $test_params->{scfg}; + + remove_tree($scfg->{path}, { verbose => 1 }); + + return; +} + +my sub run_test_for_params($test_params) { + eval { setup_test_env($test_params); }; + if ($@) { + teardown_test_env($test_params); + + fail($test_params->{description}); + diag("Failed to set up test environment: $@"); + + return; + } + + my ($storeid, $scfg, $vmid, $vtypes) = $test_params->@{qw(storeid scfg vmid vtypes)}; + + my $expected = [map { $_->{expected} // () } $test_params->{cases}->@*]; + $expected = [sort cmp_volinfo_by_volid $expected->@*]; + + my $got = eval { + my $volume_list = PVE::Storage::Plugin->list_volumes($storeid, $scfg, $vmid, $vtypes); + return [sort cmp_volinfo_by_volid $volume_list->@*]; + }; + $got = $@ if $@; + + my $desc_vmid = defined($vmid) ? $vmid : 'undef'; + my $desc_vtypes = '[' . join(', ', $vtypes->@*) . ']'; + + my $description = + $test_params->{description} . " - \$vmid = $desc_vmid, \$vtypes = $desc_vtypes"; + + if (!is_deeply($got, $expected, $description)) { + diag("\$got = ", explain($got)); + diag("\$expected = ", explain($expected)); + } + + teardown_test_env($test_params); + + return; +} + +my sub assert_test_params_keys_exist($test_params) { + my $desc = $test_params->{description}; + + confess "Test parameters without description" + if !$desc; + + for my $key (qw(storeid scfg vmid vtypes cases)) { + confess "Key '$key' does not exist in '$desc'" + if !exists($test_params->{$key}); + } + + return; +} + sub main() { - my $plan = scalar @tests; - plan tests => $plan + 1; + plan tests => scalar(@tests) + scalar($test_param_list->@*) + 1; # Keep the original vmlist around in order to check whether it was modified # after running all the tests. See: # https://pve.proxmox.com/pipermail/pve-devel/2020-January/041096.html my $original_vmlist = dclone(PVE::Cluster::get_vmlist()); + for my $test_params ($test_param_list->@*) { + assert_test_params_keys_exist($test_params); + run_test_for_params($test_params); + } + run_legacy_tests(); my $vmlist = PVE::Cluster::get_vmlist(); -- 2.47.3