Support custom async runner

[la10736]

Find a way where the user can choose a custom attribute to mark async tests.

Otherwise they can provide a function or macro to to wrap the calls to async test functions

[rubdos]

I'm currently taking a look at how it could work with actix' runtime (which is Tokio underneath). I think it can be done in a pretty simple way, by letting the original runtime's attibute do the lifting for the async part, and taking over what got produced there.

I currently have this patch for actix:

diff --git a/actix-macros/src/lib.rs b/actix-macros/src/lib.rs
index b6a57b9..a016fef 100644
--- a/actix-macros/src/lib.rs
+++ b/actix-macros/src/lib.rs
@@ -58,6 +58,7 @@ pub fn test(_: TokenStream, item: TokenStream) -> TokenStream {
     let input = syn::parse_macro_input!(item as syn::ItemFn);

     let ret = &input.sig.output;
+    let inputs = &input.sig.inputs;
     let name = &input.sig.ident;
     let body = &input.block;
     let attrs = &input.attrs;
@@ -81,7 +82,7 @@ pub fn test(_: TokenStream, item: TokenStream) -> TokenStream {
     let result = if has_test_attr {
         quote! {
             #(#attrs)*
-            fn #name() #ret {
+            fn #name ( #inputs ) #ret {
                 actix_rt::System::new("test")
                     .block_on(async { #body })
             }
@@ -90,7 +91,7 @@ pub fn test(_: TokenStream, item: TokenStream) -> TokenStream {
         quote! {
             #[test]
             #(#attrs)*
-            fn #name() #ret {
+            fn #name ( #inputs ) #ret {
                 actix_rt::System::new("test")
                     .block_on(async { #body })
             }

which makes it almost work. If I amend the code such that it does not emit the #[test] attribute when #[rstest] is among the attributes, it actually works perfectly.

... which makes me come to a question: would you accept a patch where #[rstest] would "tolerate" duplication of the #[test] attribute? I.e., if rstest would allow code such as

#[test]
#[rstest]
fn foo() {}

I think it would perfectly solve this issue (and it should also reduce the complexity of integrating with async_std: it would probably Just Work™).

The only thing I'm wondering/worrying about is the order of the attributes...

[la10736]

@rubdos

I think that your approach is good should be elaborated a bit. We can assume that custom async test attribue should end by test (last PathSegment is test).... I'll call it async_test_path.

Now in every async test we can search in attributes for the last async_test_path, remove it and use as test_attr.

Your example become:

#[rstest]
#[actix_rt::test]
async fn foo(logger: Logger) {
}

and works also for parametrize and matrix.

In this way we can define a default test_attribute and override it every time we would use a custom test attribute.

What do you think?

[la10736]

In this way you don't need to change actix_rt::test attribute to work with rstest.

[rubdos]

Maybe it's an option to just require some test attribute supplied by the user, and not generate it in #[rstest]? Then, for matrix/parametrize, you just duplicate all the attributes, and no specific logic is required any more (I'd think). I do realize this would be a breaking change though, but it would greatly simplify your code base!

FYI: It seems like tokio doesn't forward arguments either. I agree though that "support" should go bidirectional: actix/tokio shouldn't need a patch to support a specific fixture library, but neither should rstest, in order to support a specific async runtime.

On the other hand: if the async runtime implements simple argument forwarding (such as in my actix patch) and fixture libraries don't impose a test attribute, it should allow any ordering of the #[rstest] and #[*::test] attributes. It does require an effort from both sides to be as general as possible though, so I'm not sure whether it's generally possible with any async crate.

---

Another option would be to separate the #[rstest] attribute (which injects either #[test] or #[async_std::test]) from some kind of an #[inject] attribute (which only transforms the function definition). The former would also inject the latter, but inject only does the bare necessary to call the fixtures.

This approach would, I think, require quite the refactoring in the code.

---

Either way, let me know which option you prefer, and I'll implement it today.

[rubdos]

FYI, actix already merged my patch, which should ease future integration with your and other fixture systems. I've also started the same discussion on the Tokio repository, where the fix should be even simpler. I hope you're okay with how this is progressing!

[la10736]

Maybe it's an option to just require some test attribute supplied by the user, and not generate it in #[rstest]? Then, for matrix/parametrize, you just duplicate all the attributes, and no specific logic is required any more (I'd think). I do realize this would be a breaking change though, but it would greatly simplify your code base!

Your patch can never works with parametrize and matrix. Parametrize and matrix create a single test function for every case and should be annotated by the right attribute but when #[rstest] is processed we can process just the following attributes and not the ones on the upper level.

What I'm trying to explain (and maybe I'm not good enough to get it clear) is that your idea is good but I'm trying to simplify the syntax and unfortunately we should/must relay on attribute ordering because

1. rstest parametrize and matrix create modules that contains test functions and every test run time expect just a function
2. a procedural macro know just the following attributes and don't know nothing about the attributes that are already processed
3. I don't want impose to a test runner to have a specific interface different from the standard test one to works with rstest.

Another option would be to separate the #[rstest] attribute (which injects either #[test] or #[async_std::test]) from some kind of an #[inject] attribute (which only transforms the function definition). The former would also inject the latter, but inject only does the bare necessary to call the fixtures.

This approach would, I think, require quite the refactoring in the code.

Either way, let me know which option you prefer, and I'll implement it today.

This is exactly what I've proposed, I used the convection to get #[*::test] instead of introduce some kind of an #[inject] attribute.

I already tested it yesterday and follow code works

#[rstest(a, b, case (1, 2), case(3, 4))]
#[actix_rt::test]
async fn foo(a: u32, b: u32) {}

This syntax is simple and neat and give a way to use rstest with every runtime without any other change.

Now I'm thinking to use the injected capability enabled for every kind of tests (async or not). And just switch the default attribute for the async case. Later I'll try to find a way to make configurable the async test attribute.

[rubdos]

Maybe it's an option to just require some test attribute supplied by the user, and not generate it in #[rstest]? Then, for matrix/parametrize, you just duplicate all the attributes, and no specific logic is required any more (I'd think). I do realize this would be a breaking change though, but it would greatly simplify your code base!

Your patch can never works with parametrize and matrix. Parametrize and matrix create a single test function for every case and should be annotated by the right attribute but when #[rstest] is processed we can process just the following attributes and not the ones on the upper level.

What I'm trying to explain (and maybe I'm not good enough to get it clear) is that your idea is good but I'm trying to simplify the syntax and unfortunately we should/must relay on attribute ordering because

Ah right, I get it now, we indeed need to rely on attribute ordering because you cannot copy attributes that aren't there. Thank you for explaining! I hadn't used matrix/parametrized fixture-tests before, that's why I glanced over it.

1. `rstest` parametrize and matrix create modules that contains test functions and every test run time expect just a function
2. a procedural macro know just the following attributes and don't know nothing about the attributes that are already processed

ack

3. I don't want impose to a test runner to have a specific interface different from the standard `test` one to works with rstest.

Are you still referring to matrix/parametrize here (i.e., applying #[test] on a module)? If so, I agree. If you're referring to forwarding arguments, I wonder, maybe it's still useful for non-matrix/param tests? It's not like the patch with actix is anything lost, no worries there.

Another option would be to separate the #[rstest] attribute (which injects either #[test] or #[async_std::test]) from some kind of an #[inject] attribute (which only transforms the function definition). The former would also inject the latter, but inject only does the bare necessary to call the fixtures.
This approach would, I think, require quite the refactoring in the code.
Either way, let me know which option you prefer, and I'll implement it today.

This is exactly what I've proposed, I used the convection to get #[*::test] instead of introduce some kind of some kind of an #[inject] attribute.

It's not exactly what I meant by an inject-attribute, but it doesn't make sense now that I consider your comment wrt matrix/param.

I already tested it yesterday and follow code works

#[rstest(a, b, case (1, 2), case(3, 4))]
#[actix_rt::test]
async fn foo(a: u32, b: u32) {}

This syntax is simple and neat and give a way to use rstest with every runtime without any other change.

Cool! Do you have a topic branch that I can have a look at already? One thing that I wonder about, is for things beyond tests, were you would inject fixtures. I was thinking about benchmarks, for example. I suppose that's for future work, nothing really urgent.

Now I'm thinking to use the injected capability enabled for every kind of tests (async or not). And just switch the default attribute for the async case. Later I'll try to find a way to make configurable the async test attribute.

Yes, I like that. It was confusing at first, for me, that #[rstest] wasn't only injecting the fixtures, but that it also added #[test]. I would rather always write them both, I'd think. It should also save you from adding some logic of the form "if !attrs.contains(test) { attr.add(test) }".

[la10736]

I already tested it yesterday and follow code works

#[rstest(a, b, case (1, 2), case(3, 4))]
#[actix_rt::test]
async fn foo(a: u32, b: u32) {}

This syntax is simple and neat and give a way to use rstest with every runtime without any other change.

Cool! Do you have a topic branch that I can have a look at already? One thing that I wonder about, is for things beyond tests, were you would inject fixtures. I was thinking about benchmarks, for example. I suppose that's for future work, nothing really urgent.

I'll commit it on master branch later. After I'll work on unit/end2end tests.

Now I'm thinking to use the injected capability enabled for every kind of tests (async or not). And just switch the default attribute for the async case. Later I'll try to find a way to make configurable the async test attribute.

Yes, I like that. It was confusing at first, for me, that #[rstest] wasn't only injecting the fixtures, but that it also added #[test]. I would rather always write them both, I'd think. It should also save you from adding some logic of the form "if !attrs.contains(test) { attr.add(test) }".

Tests are more about ergonomics and less code to write than logic and coherence. So in name of this I'm looking for simple and concise syntax that works in almost all cases. Moreover I'm strongly influenced by pytest that is an amazing tool.

[la10736]

I've open #91 to implement the proposed behavior and leave this ticket open to find a way of how can the user indicate its default async test attribute instead of async-std.

[rubdos]

I've open #91 to implement the proposed behavior and leave this ticket open to find a way of how can the user indicate its default async test attribute instead of async-std.

One idea might be to create features "async-std", "tokio", "actix", and check which is active when the test attribute is generated? It's quite some manual labour/repetitive code, but it will be like that anyhow, I think.

[la10736]

I've open #91 to implement the proposed behavior and leave this ticket open to find a way of how can the user indicate its default async test attribute instead of async-std.

One idea might be to create features "async-std", "tokio", "actix", and check which is active when the test attribute is generated? It's quite some manual labour/repetitive code, but it will be like that anyhow, I think.

Yes, that's also my idea of how we can implement it for each runtime. But here I'm looking for a way where a user can define a default test attribute (async or not) without change rstest source code.

For instance env variable can works but I'm looking if there is a better way where the user can express in Cargo.toml.

[rubdos]

For instance env variable can works but I'm looking if there is a better way where the user can express in Cargo.toml.

I've been toying a lot with env and Cargo.toml related variable sourcing, and in my experience it's just a liability. Sourcing from Cargo.toml from the main project requires a build.rs in that project; afaik it's not possible to source the dependee's project's Cargo.toml in a dependency such as rstest... I've got that situation going on in something of my own (where I had to revert to env vars).

[la10736]

I'll use env variable to handle this and .env file. Look at sqlx/sqlx-macros/src/query/mod.rs as example.

[Mastermindaxe]

Hey everyone 👋 What's the current state on this?

[la10736]

Today you can use a custom rest runner as described in https://docs.rs/rstest/latest/rstest/attr.rstest.html#inject-test-attribute

This ticket is open just because I would like to fine a good way to define the custom default runner for both sync and async cases.

[la10736]

Today you can use a custom rest runner as described in https://docs.rs/rstest/latest/rstest/attr.rstest.html#inject-test-attribute

This ticket is open just because I would like to fine a good way to define the custom default runner for both sync and async cases.

[the-wondersmith]

I've open #91 to implement the proposed behavior and leave this ticket open to find a way of how can the user indicate its default async test attribute instead of async-std.

One idea might be to create features "async-std", "tokio", "actix", and check which is active when the test attribute is generated? It's quite some manual labour/repetitive code, but it will be like that anyhow, I think.

Yes, that's also my idea of how we can implement it for each runtime. But here I'm looking for a way where a user can define a default test attribute (async or not) without change rstest source code.

For instance env variable can works but I'm looking if there is a better way where the user can express in Cargo.toml.

A Cargo.toml table like [package.metadata.rstest] would be the way to do this (and personally I can't advocate hard enough for it).