Git Product home page Git Product logo

Comments (16)

frondeus avatar frondeus commented on August 24, 2024 2

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.

from test-case.

mkpankov avatar mkpankov commented on August 24, 2024 1

assert_debug_snapshot is from insta

from test-case.

frondeus avatar frondeus commented on August 24, 2024 1

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"?

from test-case.

frondeus avatar frondeus commented on August 24, 2024 1

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

from test-case.

frondeus avatar frondeus commented on August 24, 2024

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.

from test-case.

mkpankov avatar mkpankov commented on August 24, 2024

test_case_name!() looks fine to me

from test-case.

mkpankov avatar mkpankov commented on August 24, 2024

I think the last idea is better 🙂

from test-case.

frondeus avatar frondeus commented on August 24, 2024

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?

from test-case.

luke-biel avatar luke-biel commented on August 24, 2024

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

from test-case.

nathan-at-least avatar nathan-at-least commented on August 24, 2024

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.

from test-case.

nathan-at-least avatar nathan-at-least commented on August 24, 2024

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.

from test-case.

luke-biel avatar luke-biel commented on August 24, 2024

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.

from test-case.

nathan-at-least avatar nathan-at-least commented on August 24, 2024

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

from test-case.

huang12zheng avatar huang12zheng commented on August 24, 2024

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

from test-case.

huang12zheng avatar huang12zheng commented on August 24, 2024

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
    }
}

from test-case.

luke-biel avatar luke-biel commented on August 24, 2024

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

from test-case.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.