Re: [pve-devel] [PATCH common 4/4] section config: add tests for separated property lists

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Tue, Nov 14, 2023 at 11:33:39AM +0100, Dominik Csapak wrote:
> more or less a copy from the normal section config test, but now with
> properties defined multiple times as well as conflicting options
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  test/Makefile                         |   1 +
>  test/section_config_separated_test.pl | 486 ++++++++++++++++++++++++++
>  2 files changed, 487 insertions(+)
>  create mode 100755 test/section_config_separated_test.pl
> 
> diff --git a/test/Makefile b/test/Makefile
> index 82f40ab..3e9fef2 100644
> --- a/test/Makefile
> +++ b/test/Makefile
> @@ -6,6 +6,7 @@ TESTS = lock_file.test			\
>  	format_test.test		\
>  	section_config_test.test	\
>  	api_parameter_test.test		\
> +	section_config_separated_test.test\
>  
>  all:
>  
> diff --git a/test/section_config_separated_test.pl b/test/section_config_separated_test.pl
> new file mode 100755
> index 0000000..234f444
> --- /dev/null
> +++ b/test/section_config_separated_test.pl
> @@ -0,0 +1,486 @@
> +#!/usr/bin/perl
> +
> +use lib '../src';
> +
> +package Conf;
> +use strict;
> +use warnings;
> +
> +use Test::More;
> +
> +use base qw(PVE::SectionConfig);
> +
> +my $defaultData = {
> +    propertyList => {
> +	type => { description => "Section type." },
> +	id => {
> +	    description => "ID",
> +	    type => 'string',
> +	    format => 'pve-configid',
> +	    maxLength => 64,
> +	},
> +	common => {
> +	    type => 'string',
> +	    description => 'common value',
> +	    maxLength => 512,
> +	},
> +    },
> +    options => {
> +	id => {},
> +	type => {},
> +    },
> +};
> +
> +sub private {
> +    return $defaultData;
> +}
> +
> +sub expect_success {
> +    my ($class, $filename, $expected, $raw, $allow_unknown) = @_;
> +
> +    my $res = $class->parse_config($filename, $raw, $allow_unknown);
> +    delete $res->{digest};
> +
> +    is_deeply($res, $expected, $filename);
> +
> +    my $written = $class->write_config($filename, $res, $allow_unknown);
> +    my $res2 = $class->parse_config($filename, $written, $allow_unknown);
> +    delete $res2->{digest};
> +
> +    is_deeply($res, $res2, "$filename - verify rewritten data");
> +}
> +
> +sub expect_fail {
> +    my ($class, $filename, $expected, $raw) = @_;
> +
> +    eval { $class->parse_config($filename, $raw) };
> +    die "test '$filename' succeeded unexpectedly\n" if !$@;
> +    ok(1, "$filename should fail to parse");
> +}
> +
> +package Conf::One;
> +use strict;
> +use warnings;
> +
> +use base 'Conf';
> +
> +sub type {
> +    return 'one';
> +}
> +
> +sub properties {
> +    return {
> +	field1 => {
> +	    description => 'Field One',
> +	    type => 'integer',
> +	    minimum => 3,
> +	    maximum => 9,
> +	},
> +	another => {
> +	    description => 'Another field',
> +	    type => 'string',
> +	},
> +	field2 => {
> +	    description => 'Field Two',
> +	    type => 'integer',
> +	    minimum => 10,
> +	    maximum => 19,
> +	}
> +    };
> +}
> +
> +sub options {
> +    return {
> +	common => { optional => 1 },
> +	field1 => {},
> +	field2 => {},
> +	another => { optional => 1 },
> +	arrayfield => { optional => 1, type => 'two' },

Oh...
*That's* how you use `type` here...
Can we not?
I'd *really* prefer to just have `arrayfield` copied into the properties
right here.

Changing an existing property currently requires us to consider all the
types it appears in.
I thought we could now have *distinct* schemas for them without *also*
having to worry about potential reuse from somewhere else :S