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

[Lukas Wagner <l.wagner@proxmox.com>]

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 ;)

> +        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.

-- 
- Lukas