From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 852951FF13B for ; Wed, 22 Apr 2026 13:15:32 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7E26E17A06; Wed, 22 Apr 2026 13:14:25 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage v1 23/54] common: test: set up parser testing code, add tests for 'iso' vtype Date: Wed, 22 Apr 2026 13:12:49 +0200 Message-ID: <20260422111322.257380-24-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: 1776856339935 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.085 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: 2RU3BMDARRKRF5D2K7MAXIUHLW4KMFNM X-Message-ID-Hash: 2RU3BMDARRKRF5D2K7MAXIUHLW4KMFNM 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: Set up the necessary scaffolding for testing the new parser functions in `PVE::Storage::Common::Parse` and add tests for the 'iso' vtype. This also adds `libtest-differences-perl` to `Build-Depends`. Since the volid parsing functions are built on top of the volname parsing functions, their tests are built in a similar way; that is, the volname parser test cases are taken and adapted for the volid parsers. This is primarily done to ensure consistency between the two "families" of parsing functions; for example, the `parse_path_as_volid_parts()` function should always return the same hash as the `parse_path_as_volname_parts()` function, except that it also contains a 'volid' key. Signed-off-by: Max R. Carrara --- debian/control | 1 + src/PVE/Makefile | 1 + src/PVE/Storage/Common/Makefile | 4 + src/PVE/Storage/Common/test/Makefile | 6 + src/PVE/Storage/Common/test/parser_tests.pl | 298 ++++++++++++++++++++ src/PVE/Storage/Common/test/run_tests.pl | 25 ++ src/PVE/Storage/Makefile | 4 + 7 files changed, 339 insertions(+) create mode 100644 src/PVE/Storage/Common/test/Makefile create mode 100755 src/PVE/Storage/Common/test/parser_tests.pl create mode 100755 src/PVE/Storage/Common/test/run_tests.pl diff --git a/debian/control b/debian/control index 0e0593af..ab30df06 100644 --- a/debian/control +++ b/debian/control @@ -10,6 +10,7 @@ Build-Depends: debhelper-compat (= 13), libpve-common-perl (>= 9.1.10), librados2-perl, libtest-mockmodule-perl, + libtest-differences-perl, libxml-libxml-perl, lintian, perl, diff --git a/src/PVE/Makefile b/src/PVE/Makefile index 9e9f6aac..b8fc6325 100644 --- a/src/PVE/Makefile +++ b/src/PVE/Makefile @@ -14,6 +14,7 @@ install: .PHONY: test test: + $(MAKE) -C Storage test $(MAKE) -C test test clean: diff --git a/src/PVE/Storage/Common/Makefile b/src/PVE/Storage/Common/Makefile index 0d9b1be1..7a13371d 100644 --- a/src/PVE/Storage/Common/Makefile +++ b/src/PVE/Storage/Common/Makefile @@ -5,3 +5,7 @@ SOURCES = \ .PHONY: install install: for i in ${SOURCES}; do install -D -m 0644 $$i ${DESTDIR}${PERLDIR}/PVE/Storage/Common/$$i; done + +.PHONY: test +test: + make -C test test diff --git a/src/PVE/Storage/Common/test/Makefile b/src/PVE/Storage/Common/test/Makefile new file mode 100644 index 00000000..c4b2c0db --- /dev/null +++ b/src/PVE/Storage/Common/test/Makefile @@ -0,0 +1,6 @@ +all: test + +.PHONY: test +test: run_tests.pl + ./run_tests.pl + diff --git a/src/PVE/Storage/Common/test/parser_tests.pl b/src/PVE/Storage/Common/test/parser_tests.pl new file mode 100755 index 00000000..0167ca74 --- /dev/null +++ b/src/PVE/Storage/Common/test/parser_tests.pl @@ -0,0 +1,298 @@ +#!/usr/bin/perl + +use v5.36; + +use lib q(../../../..); + +use File::Temp; +use Storable qw(dclone); + +use PVE::Storage::Common qw( + plugin_get_vtype_subdir +); +use PVE::Storage::Common::Parse qw( + parse_path_as_volname_parts + parse_path_as_volname + parse_volname_as_parts + + parse_path_as_volid_parts + parse_path_as_volid + parse_volid_as_parts +); + +use Test::More qw(no_plan); +use Test::Differences; + +my $volname_tests = []; + +my $volname_cases_iso_valid = [ + # plain paths + { + path => 'custom-debian.iso', + expected => { + file => 'custom-debian.iso', + ext => 'iso', + 'disk-path' => 'custom-debian.iso', + path => 'custom-debian.iso', + vtype => 'iso', + volname => 'iso/custom-debian.iso', + }, + }, + { + path => 'archlinux.img', + expected => { + file => 'archlinux.img', + ext => 'img', + 'disk-path' => 'archlinux.img', + path => 'archlinux.img', + vtype => 'iso', + volname => 'iso/archlinux.img', + }, + }, + + # case insensitive file extensions + { + path => 'Debian 13 Trixie.ISO', + expected => { + file => 'Debian 13 Trixie.ISO', + ext => 'ISO', + 'disk-path' => 'Debian 13 Trixie.ISO', + path => 'Debian 13 Trixie.ISO', + vtype => 'iso', + volname => 'iso/Debian 13 Trixie.ISO', + }, + }, + { + path => 'Fedora.Img', + expected => { + file => 'Fedora.Img', + ext => 'Img', + 'disk-path' => 'Fedora.Img', + path => 'Fedora.Img', + vtype => 'iso', + volname => 'iso/Fedora.Img', + }, + }, +]; + +my $volname_cases_iso_invalid = [ + { + description => "Invalid file extension (iso)", + args => { + path => 'MacOS-kittycat.dmg', + vtype => 'iso', + }, + expected => undef, + }, +]; + +my $cases_valid_all = [ + $volname_cases_iso_valid, +]; + +my $cases_invalid_all = [ + $volname_cases_iso_invalid, +]; + +{ + for my $cases_list ($cases_valid_all->@*) { + for my $case ($cases_list->@*) { + my $path = $case->{path}; + my $vtype = $case->{expected}->{vtype}; + + push( + $volname_tests->@*, + { + description => "Valid path ($vtype) \"$path\"", + args => { + path => $path, + vtype => $vtype, + }, + expected => $case->{expected}, + }, + ); + } + } + + for my $cases_list ($cases_invalid_all->@*) { + push($volname_tests->@*, $cases_list->@*); + } +} + +my sub run_volname_parsing_tests : prototype($) ($volname_tests) { + for my $case ($volname_tests->@*) { + subtest $case->{description} => sub() { + my ($path, $vtype) = $case->{args}->@{qw(path vtype)}; + + my $got_volname_parts = parse_path_as_volname_parts($path, $vtype); + my $got_volname = parse_path_as_volname($path, $vtype); + + if (defined($case->{expected})) { + eq_or_diff( + $got_volname_parts, + $case->{expected}, + 'parse_path_as_volname_parts() returns expected hashref', + { context => 50000 }, + ); + + is( + $got_volname, + $case->{expected}->{volname}, + 'parse_path_as_volname() returns expected volname', + ); + + my $got_volname_parts_from_volname = parse_volname_as_parts($got_volname); + + eq_or_diff( + $got_volname_parts_from_volname, + $case->{expected}, + 'parse_volname_as_parts() returns expected hashref from parsed volname', + ); + } else { + is($got_volname_parts, undef, 'parse_path_as_volname_parts() returns undef'); + is($got_volname, undef, 'parse_path_as_volname() returns undef'); + } + + }; + } + + return; +} + +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 $ALT_STOREID = 'dir-store'; +my $ALT_STORAGE_PATH = File::Temp->newdir(); +my $ALT_SCFG = { + type => 'dir', + path => $ALT_STORAGE_PATH, + shared => 0, + content => { $DEFAULT_SCFG->{content}->%* }, + 'content-dirs' => { + iso => 'pve/volumes/iso', + vztmpl => 'pve/dumps/vz', + images => 'pve/guest/images', + snippets => 'pve/misc/snippets', + backup => 'pve/dumps/bak', + import => 'archive/import', + }, +}; + +my sub format_volname_case_to_volid_case($storeid, $scfg, $volname_case) { + my $volid_case = dclone($volname_case); + + my ($path, $vtype) = $volid_case->{args}->@{qw(path vtype)}; + + my $vtype_subdir = plugin_get_vtype_subdir($scfg, $vtype); + + $volid_case->{args}->{path} = $vtype_subdir . '/' . $path; + $volid_case->{args}->{storeid} = $storeid; + $volid_case->{args}->{scfg} = $scfg; + + if (defined($volid_case->{expected})) { + my $volname = $volid_case->{expected}->{volname}; + $volid_case->{expected}->{volid} = $storeid . ':' . $volname; + } + + return $volid_case; +} + +my $volid_tests = [ + { + description => "volid parsers build on volname parsers' behaviors (1)", + storeid => $DEFAULT_STOREID, + scfg => $DEFAULT_SCFG, + cases => [ + map { format_volname_case_to_volid_case($DEFAULT_STOREID, $DEFAULT_SCFG, $_) } + $volname_tests->@* + ], + }, + { + description => "volid parsers build on volname parsers' behaviors (2)", + storeid => $ALT_STOREID, + scfg => $ALT_SCFG, + cases => [ + map { format_volname_case_to_volid_case($ALT_STOREID, $ALT_SCFG, $_) } + $volname_tests->@* + ], + }, +]; + +my sub run_volid_parsing_tests : prototype($) ($volid_tests) { + for my $test ($volid_tests->@*) { + subtest $test->{description} => sub() { + for my $case ($test->{cases}->@*) { + my ($storeid, $scfg, $path, $vtype) = + $case->{args}->@{qw(storeid scfg path vtype)}; + + my $got_volid_parts = parse_path_as_volid_parts($storeid, $scfg, $path, $vtype); + my $got_volid = parse_path_as_volid($storeid, $scfg, $path, $vtype); + + note("Running test case based on:"); + note($case->{description}); + + if (defined($case->{expected})) { + eq_or_diff( + $got_volid_parts, + $case->{expected}, + 'parse_path_as_volid_parts() returns expected hashref', + { context => 50000 }, + ); + + is( + $got_volid, + $case->{expected}->{volid}, + 'parse_path_as_volid() returns expected volid', + ); + + my $expected_volid_parts = { + volid => $case->{expected}->{volid}, + storeid => $case->{args}->{storeid}, + volname => $case->{expected}->{volname}, + }; + + my $got_volid_parts_from_volid = parse_volid_as_parts($got_volid); + + eq_or_diff( + $got_volid_parts_from_volid, + $expected_volid_parts, + 'parse_volid_as_parts() returns expected hashref from parsed volid', + ); + } else { + is($got_volid_parts, undef, 'parse_path_as_volid_parts() returns undef'); + is($got_volid, undef, 'parse_path_as_volid() returns undef'); + } + } + }; + } + + return; +} + +my sub main() { + unified_diff(); + + run_volname_parsing_tests($volname_tests); + run_volid_parsing_tests($volid_tests); + + done_testing(); + + return; +} + +main(); diff --git a/src/PVE/Storage/Common/test/run_tests.pl b/src/PVE/Storage/Common/test/run_tests.pl new file mode 100755 index 00000000..ed8b1164 --- /dev/null +++ b/src/PVE/Storage/Common/test/run_tests.pl @@ -0,0 +1,25 @@ +#!/usr/bin/perl + +use v5.36; + +use TAP::Harness; + +my $MAX_JOBS = 4; + +my $nproc = eval { int(`nproc`) } || $MAX_JOBS; + +my $jobs = $nproc > $MAX_JOBS ? $MAX_JOBS : $nproc; + +my sub main() { + my $harness = TAP::Harness->new({ verbosity => -1, jobs => $jobs }); + + my $res = $harness->runtests( + "parser_tests.pl", + ); + + exit -1 if !$res || $res->{failed} || $res->{parse_errors}; + + return; +} + +main(); diff --git a/src/PVE/Storage/Makefile b/src/PVE/Storage/Makefile index a67dc25f..1c8d364b 100644 --- a/src/PVE/Storage/Makefile +++ b/src/PVE/Storage/Makefile @@ -21,3 +21,7 @@ install: make -C Common install for i in ${SOURCES}; do install -D -m 0644 $$i ${DESTDIR}${PERLDIR}/PVE/Storage/$$i; done make -C LunCmd install + +.PHONY: test +test: + make -C Common test -- 2.47.3