Re: [pbs-devel] [PATCH proxmox-backup v5 4/4] tests: check if include/exclude behavior works correctly

[Philipp Hufnagl <p.hufnagl@proxmox.com>]

On 12/19/23 14:23, Lukas Wagner wrote:
> 
> 
> On 12/18/23 16:36, Philipp Hufnagl wrote:
>> diff --git a/tests/sync_jobs.rs b/tests/sync_jobs.rs
>> new file mode 100644
>> index 00000000..83877160
>> --- /dev/null
>> +++ b/tests/sync_jobs.rs
>> @@ -0,0 +1,34 @@
>> +use pbs_api_types::{
>> +    apply_filters, split_by_include_exclude, BackupGroup,
>> BackupType, GroupFilter,
>> +};
>> +use std::str::FromStr;
>> +
>> +#[test]
>> +fn test_group_filters() {
>> +    let group_filters = vec![
>> +        GroupFilter::from_str("exclude:regex:.*10[1-3]").unwrap(),
> 
> Just FIY, since GroupFilter implements FromStr, you can use the .parse
> method on the string:
> 
> "...".parse::<GroupFilter>().unwrap();
> 
> The superfish (`::<Type>`) is probably not needed, since the type can
> be inferred because you pass it to the split function ;)

I'll try that. Thanks
> 
>> +        GroupFilter::from_str("regex:.*10[2-8]").unwrap(),
>> +        GroupFilter::from_str("exclude:regex:.*10[5-7]").unwrap(),
>> +    ];
>> +    let (include_filters, exclude_filters) =
>> split_by_include_exclude(Some(group_filters));
>> +
>> +    let dont_backup = vec![
>> +        "vm/101", "vm/102", "vm/103", "vm/105", "vm/106", "vm/107",
>> "vm/109",
>> +    ];
>> +    for id in dont_backup {
>> +        assert!(!apply_filters(
>> +            &BackupGroup::new(BackupType::Vm, id),
>> +            &include_filters,
>> +            &exclude_filters
>> +        ));
>> +    }
>> +
>> +    let do_backup = vec!["vm/104", "vm/108"];
>> +    for id in do_backup {
>> +        assert!(apply_filters(
>> +            &BackupGroup::new(BackupType::Vm, id),
>> +            &include_filters,
>> +            &exclude_filters
>> +        ));
>> +    }
>> +}
> 
> Including tests is a great idea! I'd also add tests for all four cases:
>   - no filters
>   - only includes
>   - only excludes
>   - both
> 
> Right now, you've only covered the 'both' case.
> 
Yes, that makes the testing way more robust. Ill add this cases.