Skip to content

Conversation

@drmorr0
Copy link

@drmorr0 drmorr0 commented May 8, 2025

Description

This adds support for using rstest with test_log, by simply wrapping the #[rstest] macro in the same way that test_log wraps the #[test] macro. The syntax for rstest is a little different, but not unreasonably so. We just need to desugar this:

#[rstest(tokio::test)]
#[case(...)]
#[case(...)]
async fn test_blah(#[case] ...) { ... }

into this:

#[rstest]
#[case(...)]
#[case(...)]
#[tokio::test]
async fn test_blah(#[case] ...) { ... }

I put everything behind a feature flag so that folks who aren't using rstest don't have to worry about it.

Testing done

  • Updated the integration tests
  • Replaced all the tests in my project with test_log and it works

@d-e-s-o
Copy link
Owner

d-e-s-o commented May 9, 2025

Thanks for the pull request. I am not eager to integrate this functionality: it has nothing to do with logging and really the two crates should just compose, similar to how it works (or is supposed to work) with test-case. Special casing like this is not desireable from my point of view.

@drmorr0
Copy link
Author

drmorr0 commented May 9, 2025

I understand the hesistation; sadly this starts to become a lot of boilerplate for every test; especially for async tests, having to write something like

#[test(rstest)]
#[tokio::test]

starts to feel pretty cumbersome, not to mention that I have to replace every instance of #[rstest] in my code with #[test_log::test(rstest)]. It'd be slightly nicer if I could instead say #[test_log::test(rstest, tokio::test)] (which isn't possible right now) but I still think the story around being able to use test_log::test (or use test_log::rstest in this case) and not change any test function annotations is pretty compelling.

Up to you though, feel free to close if you disagree.

@d-e-s-o
Copy link
Owner

d-e-s-o commented May 9, 2025

especially for async tests, having to write something like

#[test(rstest)]
#[tokio::test]

I think the ideal would be listing each test "layer", ala

#[test_log::test]
#[rstest::rstest]
#[tokio::test]

(in whatever order necessary). I think #46 may get us somewhere there. Yes, still boilerplate, but the declarative and obvious-to-understand-what-is-happening kind, if there is such a thing.
I could equally well argue that rstest provide an rstest_async attribute which makes things async (or a rstest_w_logging that initializes logging). They probably won't do it for similar reasons as this crate won't have test_async: it would hard code the runtime choice in their crate, which limits flexibility, enforces versioning constraints, and just generally is a pretty bad idea for something supposedly generic.

@d-e-s-o d-e-s-o closed this May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants