Access test case name from test case body

[mkpankov]

Example code

    #[test_case(vec![(1,1),(2,2),(3,3)].iter().collect(), vec![(1,1),(2,2),(3,3)].iter().collect() ; "test")]
    fn get_changes_values<
        'a,
        'b,
        K: Eq + Ord + Clone + fmt::Debug,
        V: Eq + fmt::Debug,
    >(
        old: &'b BTreeMap<&'a K, V>,
        new: &'b BTreeMap<&'a K, V>,
    ) {
        let changes = super::get_changes_values(old, new);
        // test_case => "test"
        assert_debug_snapshot(test_case, changes);
    }

[mkpankov]

assert_debug_snapshot is from insta

[frondeus]

I thought about it. In my opinion yes, there should be a way to retrieve test case name, however right now I'm not sure how to achieve it without creating variable "from thin air".

What I mean if you are not aware that #[test_case(...)] macro defines variable test_case then you may wonder what this variable comes from.

[frondeus]

I thought either to "hide it" inside some macro like test_case_name!() (similar to module_path!() )
and then let #[test_case(...)] parse function body and replace test_case_name!()

Another idea is to have explicit argument in function definition with some extra attribute.

[mkpankov]

test_case_name!() looks fine to me

[frondeus]

Okay, I feel macro may be quite cumbersome to achieve.

However, I was thinking a bit about the issue and what about something like this:

#[test_case("".into(), 0)]
#[test_case("bar".into(), 1)]
fn foo(arg_1: String, arg_2: usize) {
    dbg!(&arg_1);
    dbg!(&arg_2);
    dbg!(&self.name); // <- Note self.name where `self` would contain test case data
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=f86338cc8428b7c315afb08e54a910e9

Isn't it going to be to "magical"?

[frondeus]

Hmm okay, but I think I can simply modify this idea with TestCase as a struct and go further with macros:

#[test_case("".into(), 0)]
#[test_case("bar".into(), 1)]
fn foo(arg_1: String, arg_2: usize) {
    dbg!(&arg_1);
    dbg!(&arg_2);
    dbg!(test_case_context!().name);
}

I was thinking about test_case_name but ... I feel we could store in TestCase more than just name and one macro could be enough. One macro to remember :)

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=027f16900091693eee3f9cbb524b6245

[mkpankov]

I think the last idea is better 🙂

[frondeus]

Okay, I have some extra notes and edge cases we need to consider but for now, it seems to be a good way forward.

It requires major refactoring in macro but technically I believe we can deliver every existing feature with this approach + async await support would be quite easy.

The biggest issue I see is to transform impl Trait in argument into T1: Trait.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=2356b4c04b4693c4c558febe4f444341

@luke-biel - what do you think?

[luke-biel]

@frondeus
While I like the resulting macro, I have doubts about the refactor. I don't say no, but we'd have to think a little deeper.
You are proposing final output of test_case, making it more concise and cleaner, however what should be considered there is how we are generating the TokenStream in first place. Correct me if I'm wrong.
I think that how the lib is currently structured makes it problematic when it comes to extending it. We should definitely tackle that, but output structuring should be superseeded by lib architecture itself. We probably should introduce some builder-ish pattern, move code around, make it easier to change parts of a generator independently to parser changes.
However that's a discussion for a separate thread.

I'd say we introduce the macro as is, with easiest implementation possible. It seems that we agree that test_case_context!() is a way to go, and we can populate it with just name field. This is public API but it should not be a subject to change due to us tinkering with the code.

[nathan-at-least]

Hi, big-time test-case fan here. ;-)

One thought experiment: what if test-case could "compose" with the function_name attribute?

The design and implementation is thorny but the idea would be something like this would do the right thing:

use function_name::named;
use test_case::test_case;

[named]
[test_case(…; "case a")]
[test_case(…; "case b")]
fn my_test(…) {
    // do normal test case testing here and you also have access to `function_name!`, ie:
    println!("my function name is {}", function_name!());
}

I looked into this a bit while trying to get my project target-test-dir to compose with test-case. The current composition in this integration test "works" but doesn't do the right thing: test-case-composition.rs.

The design issue is that test-case currently propagates any attributes to the "work function", such as my_test above, so the expansion looks similar to:

#[named]
fn my_test(…) { … }

mod my_test {
  #[test]
  fn case_a() {
    my_test(…);
  }

  #[test]
  fn case_b() {
    my_test(…);
  }
}

For the composition of named that I'm brainstorming, there are two problems. First the expansion would need to apply to the test functions, not the work functions, like this:

fn my_test(…) { … }

mod my_test {
 #[test]
 #[named]
 fn case_a() {
   my_test(…);
 }

 #[test]
 #[named]
 fn case_b() {
   my_test(…);
 }
}

(I'm not actually certain if the order shown for #[named] vs #[test] is correct or the other order is.)

-next is the bigger problem is that #[named] injects a function-specific macro function_name!() into the body of the annotated function, so this approach breaks down because within my_func the expansion of function_name!() would be undefined.

So… thanks for coming along this journey with me, but I don't see a way yet to make these two proc-macros compose cleanly.

It's a shame because I personally would prefer the ability to mix and match a variety of proc-macros, rather than having a single proc-macro that does everything.

[nathan-at-least]

BTW, the composition tests test-case-composition.rs in the target-test-dir project work!

The problem is that function_name!() (which target-test-dir uses) is for the work function, whereas for the design goal of target-test-dir is to provide a unique directory for each unique test. With that composition, the same directory is used for every test case, violating a clean independent repeatability goal for tests.

[luke-biel]

I see your issue and I must say I have no idea how to get #named to work without significant changes to test-case. In theory we get other attributes, like #[tokio::test], to work by copying them to child functions, but #[named] isn't isolated to that child function. It looks like this:

fn test_fn() {
    println!("Call me: {}", function_name!());
}

mod test_fn {
    #[test]
    #[named]
    fn test() {
        let res = super::test_fn();
        res
    }

So the test_fn::test get's a name, but it's the ::test_fn who's trying to call it :/

Other note is, that just by using a custom test harness, suddenly a lot of test-case's workarounds stop being a problem.

[nathan-at-least]

@luke-biel Yep, I don't see any clean way to make #[named] and #[test-case] compose well, so it's probably cleaner to add the feature directly into test-case.

[huang12zheng]

Are there current best practices for assert debug snapshot integration?
@luke-biel lub

I was suddenly in that would closures like FnOnce be useful?

---

more

#[cfg(test)]
macro_rules! set_snapshot_suffix {
    ($($expr:expr),*) => {
        let mut settings = insta::Settings::clone_current();
        settings.set_snapshot_suffix(format!($($expr,)*));
        let _guard = settings.bind_to_scope();
    }
}
#[cfg(test)]
fn suffix(name: &str) {
    set_snapshot_suffix!("{}", name);
}

let _result = stringify!(phone);
dbg!(&_result);
// suffix(_result);  // can't work
set_snapshot_suffix!("{}", _result); // work

• get file_name
  test_pattern__create_login_json@phone.snap

[huang12zheng]

https://github.com/huang12zheng/test-case/tree/prehook

#[case(("+118150".to_string(), "".to_string());"phone" => ! set_snapshot_suffix)]

use crate::expr::TestCaseExpression;

#[derive(Debug)]
pub struct TestCaseComment {
    _semicolon: Token![;],
    pub comment: LitStr,
    pub expression: Option<TestCaseExpression>,
}

impl Parse for TestCaseComment {
    fn parse(input: ParseStream) -> syn::Result<Self> {
        Ok(Self {
            _semicolon: input.parse()?,
            comment: input.parse()?,
            expression: input.parse().ok(),
        })
    }
}

pub fn test_case_name_prehook(&self) -> TokenStream2 {
        if let Some(comment) = self.comment.as_ref() {
            if let Some(expr) = comment.expression.as_ref() {
                return match expr.result {
                    TestCaseResult::Panicking(_) => TokenStream2::new(),
                    TestCaseResult::CommentMacro(_) => {
                        let assertion = expr.assertion();
                        let test_case_name: LitStr = comment.comment.clone();

                        return quote!(
                            #assertion!(#test_case_name);
                        );
                    }
                    _ => return quote!(expr.assertion()),
                };
            }
        }
        return quote! {};
    }

quote! {
    #(#attrs)*
    #signature {
        #test_case_name_prehook
        #body
        #expected
    }
}

[luke-biel]

@huang12zheng Afaik best solution in this case would be to use insta::with_settings, while also setting a CONST that'd represent group of tests (in this case for use with insta suffix), eg.:

const SUFFIX: &str = "phone";

#[case(1)]
#[case(2)]
fn a_test(i: i32) {
    with_settings!({snapshot_suffix = SUFFIX}, { /* do the snapshot comparison */ })
}

Or if you need a suffix per test, passing the suffix as an argument:

#[case(1, "test_1")]
#[case(2, "test_2")]
fn a_test(i: i32, suffix: &str) {
    with_settings!({snapshot_suffix = suffix}, {/* do the snapshot comparison */ })
}

I don't see us putting extra macros into comment part, the attribute body is anyway extremely bloated at times, and the goal for test case was to be rather a simple tool.

As for accessing the test name from the body (which appears to be more and more needed feature), I'm gonna prioritize at least designing it.