[pve-devel] [PATCH storage v3 5/7] test: path_to_volume_id

[Fabian Ebner]

Two comments inline.

On 08.04.20 12:26, Alwin Antreich wrote:
> Test to reduce the potential for accidental breakage on regex changes.
> 
> Signed-off-by: Alwin Antreich <a.antreich at proxmox.com>
> ---
>   test/run_parser_tests.pl       |   2 +-
>   test/test_path_to_volume_id.pm | 102 +++++++++++++++++++++++++++++++++
>   2 files changed, 103 insertions(+), 1 deletion(-)
>   create mode 100644 test/test_path_to_volume_id.pm
> 
> diff --git a/test/run_parser_tests.pl b/test/run_parser_tests.pl
> index 4b1c003..635b59d 100755
> --- a/test/run_parser_tests.pl
> +++ b/test/run_parser_tests.pl
> @@ -10,7 +10,7 @@ my $res = $harness->runtests(
>       "test_archive_info.pm",
>       "test_parse_volname.pm",
>       "test_list_volumes.pm",
> +    "test_path_to_volume_id.pm",
>   );
>   
>   exit -1 if !$res || $res->{failed} || $res->{parse_errors};
> -
> diff --git a/test/test_path_to_volume_id.pm b/test/test_path_to_volume_id.pm
> new file mode 100644
> index 0000000..e693974
> --- /dev/null
> +++ b/test/test_path_to_volume_id.pm
> @@ -0,0 +1,102 @@
> +package PVE::Storage::TestPathToVolumeId;
> +
> +use strict;
> +use warnings;
> +
> +use lib qw(..);
> +
> +use PVE::Storage;
> +
> +use Test::More;
> +
> +use Cwd;
> +use File::Basename;
> +use File::Path qw(make_path remove_tree);
> +
> +my $storage_dir = getcwd() . '/test_path_to_volume_id';
> +my $scfg = {
> +    'digest' => 'd29306346b8b25b90a4a96165f1e8f52d1af1eda',
> +    'ids' => {
> +	'local' => {
> +	    'shared' => 0,
> +	    'path' => "$storage_dir",
> +	    'type' => 'dir',
> +	    'content' => {
> +		'snippets' => 1,
> +		'rootdir' => 1,
> +		'images' => 1,
> +		'iso' => 1,
> +		'backup' => 1,
> +		'vztmpl' => 1
> +	    },
> +	    'maxfiles' => 0
> +	}
> +    },
> +    'order' => {
> +	'local' => 1
> +    }
> +};
> +
> +my @tests = (
> +	[ "$storage_dir/images/16110/vm-16110-disk-0.qcow2", ['images', 'local:16110/vm-16110-disk-0.qcow2'], 'Image, qcow2' ],
> +	[ "$storage_dir/images/16112/vm-16112-disk-0.raw",   ['images', 'local:16112/vm-16112-disk-0.raw'],   'Image, raw' ],
> +	[ "$storage_dir/images/9004/base-9004-disk-0.qcow2", ['images', 'local:9004/base-9004-disk-0.qcow2'], 'Image template, qcow2' ],
> +
> +	[ "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz",  ['iso', 'local:backup/vzdump-qemu-16110-2020_03_30-21_11_40.vma.gz'],  'Backup, vma.gz' ],
> +	[ "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo", ['iso', 'local:backup/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo'], 'Backup, vma.lzo' ],
> +	[ "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_13_55.vma",     ['iso', 'local:backup/vzdump-qemu-16110-2020_03_30-21_13_55.vma'],     'Backup, vma' ],
> +	[ "$storage_dir/dump/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo",  ['iso', 'local:backup/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo'],  'Backup, tar.lzo' ],
> +

Here it shouldn't be 'iso', but 'backup' (compare with parse_volname in 
Storage/Plugin.pm). This is actually a bug in the implementation in 
Storage.pm.

> +	[ "$storage_dir/template/iso/yet-again-a-installation-disk.iso",          ['iso', 'local:iso/yet-again-a-installation-disk.iso'],              'ISO file' ],
> +	[ "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.gz", ['vztmpl', 'local:vztmpl/debian-10.0-standard_10.0-1_amd64.tar.gz'], 'CT template, tar.gz' ],
> +
> +	[ "$storage_dir/private/1234/",                          ['rootdir', 'local:rootdir/1234'],                  'Rootdir' ], # fileparse needs / at the end
> +	[ "$storage_dir/images/1234/subvol-1234-disk-0.subvol/", ['images', 'local:1234/subvol-1234-disk-0.subvol'], 'Rootdir, folder subvol' ], # fileparse needs / at the end
> +
> +	# no matches
> +	[ "$storage_dir/snippets/userconfig.yaml",                                [''], 'Snippets, yaml' ],
> +	[ "$storage_dir/snippets/hookscript.pl",                                  [''], 'Snippets, hookscript' ],

Again I'd say this is a bug in Storage.pm, as snippets are a valid 
content type and should be recognized.

> +	[ "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.xz", [''], 'CT template, tar.xz' ],
> +
> +	# no matches, path or files with failures
> +	[ "$storage_dir/images/ssss/base-4321-disk-0.raw",                         [''], 'Base template, string as vmid in folder name' ],
> +	[ "$storage_dir/template/iso/yet-again-a-installation-disk.dvd",           [''], 'ISO file, wrong ending' ],
> +	[ "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.zip.gz",  [''], 'CT template, wrong ending, zip.gz' ],
> +	[ "$storage_dir/template/cache/debian-10.0-standard_10.0-1_amd64.tar.bz2", [''], 'CT template, wrong ending, tar bz2' ],
> +	[ "$storage_dir/private/subvol-19254-disk-0/",                             [''], 'Rootdir as subvol, wrong path' ],
> +	[ "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tar.bz2",     [''], 'Backup, wrong ending, openvz, tar.bz2' ],
> +	[ "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.zip.gz",      [''], 'Backup, wrong format, openvz, zip.gz' ],
> +	[ "$storage_dir/dump/vzdump-openvz-16112-2020_03_30-21_39_30.tgz.lzo",     [''], 'Backup, wrong format, openvz, tgz.lzo' ],
> +	[ "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vma.xz",        [''], 'Backup, wrong ending, qemu, vma.xz' ],
> +	[ "$storage_dir/dump/vzdump-qemu-16110-2020_03_30-21_12_40.vms.gz",        [''], 'Backup, wrong format, qemu, vms.gz' ],
> +	[ "$storage_dir/images/ssss/vm-1234-disk-0.qcow2",                         [''], 'Image, string as vmid in folder name' ],
> +);
> +
> +plan tests => scalar @tests;
> +
> +foreach my $tt (@tests) {
> +    my ($file, $expected, $description) = @$tt;
> +
> +    # 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);
> +    }
> +
> +    # run tests
> +    my $got;
> +    eval { $got = [ PVE::Storage::path_to_volume_id($scfg, $file) ] };
> +    $got = $@ if $@;
> +
> +    is_deeply($got, $expected, $description) || diag(explain($got));
> +
> +    #cleanup
> +    remove_tree($storage_dir, { verbose => 1 });
> +}
> +
> +done_testing();
> +
> +1;
>