[Testing]: Improve the test setup experience with rstest

[pitoniak32]

Related Problems?

I notice there is some setup required for tests that would like to test multiple cases with slightly different inputs.

Extra setup can lead to tests not being updated as easily, and being neglected more often than if its trivial to add a case.

I personally also find the fixture based approach easier to read / reason about, and it takes out the extra logic needed to setup a test fixture.

Describe the solution you'd like:

Try out using rstest crate as a dev dependency to improve the test writing ergonomics.

• https://github.com/la10736/rstest?tab=readme-ov-file#fixture-based-test-framework-for-rust

Example:

#[test]
fn merge_resource_schema_url() {
    // if both resources contains key value pairs
    let test_cases = vec![
        (Some("http://schema/a"), None, Some("http://schema/a")),
        (Some("http://schema/a"), Some("http://schema/b"), None),
        (None, Some("http://schema/b"), Some("http://schema/b")),
        (
            Some("http://schema/a"),
            Some("http://schema/a"),
            Some("http://schema/a"),
        ),
        (None, None, None),
    ];

    for (schema_url_a, schema_url_b, expected_schema_url) in test_cases.into_iter() {
        let resource_a = Resource::from_schema_url(
            vec![KeyValue::new("key", "")],
            schema_url_a.unwrap_or(""),
        );
        let resource_b = Resource::from_schema_url(
            vec![KeyValue::new("key", "")],
            schema_url_b.unwrap_or(""),
        );

        let merged_resource = resource_a.merge(&resource_b);
        let result_schema_url = merged_resource.schema_url();

        assert_eq!(
            result_schema_url.map(|s| s as &str),
            expected_schema_url,
            "Merging schema_url_a {:?} with schema_url_b {:?} did not yield expected result {:?}",
            schema_url_a, schema_url_b, expected_schema_url
        );
    }
}

vs

#[rstest]
#[case(Some("http://schema/a"), None, Some("http://schema/a"))]
#[case(Some("http://schema/a"), Some("http://schema/b"), None)]
#[case(None, Some("http://schema/b"), Some("http://schema/b"))]
#[case(
    Some("http://schema/a"),
    Some("http://schema/a"),
    Some("http://schema/a")
)]
#[case(None, None, None)]
fn merge_resource_schema_url(
    #[case] schema_url_a: Option<&'static str>,
    #[case] schema_url_b: Option<&'static str>,
    #[case] expected_schema_url: Option<&'static str>,
) {
    let resource_a =
        Resource::from_schema_url(vec![KeyValue::new("key", "")], schema_url_a.unwrap_or(""));
    let resource_b =
        Resource::from_schema_url(vec![KeyValue::new("key", "")], schema_url_b.unwrap_or(""));

    let merged_resource = resource_a.merge(&resource_b);
    let result_schema_url = merged_resource.schema_url();

    assert_eq!(
        result_schema_url.map(|s| s as &str),
        expected_schema_url,
        "Merging schema_url_a {:?} with schema_url_b {:?} did not yield expected result {:?}",
        schema_url_a,
        schema_url_b,
        expected_schema_url
    );
}

Considered Alternatives

No response

Additional Context

Some considerations might include if this will negatively impact the speed at which tests build if used frequently

[lalitb]

Probably related : #1552

[pitoniak32]

yeah this seems related, this would help to get that ball rolling. It seems that issue has been passed around a few times. If there is interest in introducing rstest I can take a look at moving that issue forward.

[cijothomas]

@pitoniak32 Lets give it a try. From the description, it looks very promising. Please start with one file and based on the feedback, we can expand.