Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forwarding input arguments of #[tokio::test] annotated tests #2388

Closed
rubdos opened this issue Apr 9, 2020 · 2 comments · Fixed by #3691
Closed

Forwarding input arguments of #[tokio::test] annotated tests #2388

rubdos opened this issue Apr 9, 2020 · 2 comments · Fixed by #3691
Labels
A-tokio Area: The main tokio crate A-tokio-macros Area: The tokio-macros crate A-tokio-test Area: The tokio-test crate C-feature-accepted Category: A feature request that has been accepted pending implementation. C-feature-request Category: A feature request. E-help-wanted Call for participation: Help is requested to fix this issue. M-macros Module: macros in the main Tokio crate

Comments

@rubdos
Copy link
Contributor

rubdos commented Apr 9, 2020

Test fixture libraries such as rstest tend to use a proc macro to inject fixtures in tests, based on function parameters of the test function. Example:

#[fixture] fn fix -> u32 {2}
#[rstest] fn foo(fix: u32) { assert_eq!(2, fix) }

Currently, there is an ongoing discussion in rstest on how to best integrate with test-suites other than the default Rust #[test], in particular those that use async, such as #[tokio::test].

In order to transparently integrate and make the #[rstest] and #[tokio::test] attributes "commutative", the function arguments of a test should simply be forwarded by the tokio::test proc macro. Currently, adding function arguments to a test triggers a custom error. As far as I know, Rust itself would generate an error like that, even without that part of code.

This brings me to my main question: dropping that check would allow tighter cooperation with rstest, and I believe with other, future fixture frameworks too. Would Tokio accept a patch, dropping that check, like the one actix-net just accepted? As far as I can tell, everything else is already in place.

@hawkw
Copy link
Member

hawkw commented Apr 9, 2020

Would Tokio accept a patch, dropping that check, like the one actix-net just accepted? As far as I can tell, everything else is already in place.

I think that would be fine. You're right that if a #[tokio::test]-annotated function is used without #[rstest] or another attribute that provides arguments, the #[test] attribute will generate an error anyway, so Tokio doesn't need to have its own error. I'd get input from other Tokio maintainers as well, but this seems reasonable to me.

@Darksonn Darksonn added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. A-tokio-test Area: The tokio-test crate labels Apr 29, 2020
@rubdos
Copy link
Contributor Author

rubdos commented Jun 9, 2020

As discussed in the issue at rstest, this is not required for rstest to work at all. It could, however, be useful for different fixture frameworks, and it makes sense IMO to default to the default Rust error either way, instead of making Tokio's own error.

@Darksonn Darksonn added C-feature-accepted Category: A feature request that has been accepted pending implementation. E-help-wanted Call for participation: Help is requested to fix this issue. A-tokio-macros Area: The tokio-macros crate M-macros Module: macros in the main Tokio crate labels Jul 28, 2020
davidpdrsn added a commit that referenced this issue Apr 10, 2021
Fixes #2388

Previously `#[tokio::test]` would error on functions that took
arguments. That meant other attribute macros couldn't do further
transformations on them. This changes that so arguments are forwarded as
is.

Whatever else might be included on the function is forwarded as well.
For example return type, generics, etc.

Worth noting that this is only for compatibility with other macros.
`#[test]`s that take arguments will still fail to compile.

A bit odd that [trybuild] tests don't fail `#[test]` functions with
arguments which is why the new tests are run with `t.pass(...)`. They do
actually fail if part of a real crate.

[trybuild]: https://crates.io/crates/trybuild
davidpdrsn added a commit that referenced this issue Apr 11, 2021
Fixes #2388

Previously `#[tokio::test]` would error on functions that took
arguments. That meant other attribute macros couldn't do further
transformations on them. This changes that so arguments are forwarded as
is.

Whatever else might be included on the function is forwarded as well.
For example return type, generics, etc.

Worth noting that this is only for compatibility with other macros.
`#[test]`s that take arguments will still fail to compile.

A bit odd that [trybuild] tests don't fail `#[test]` functions with
arguments which is why the new tests are run with `t.pass(...)`. They do
actually fail if part of a real crate.

[trybuild]: https://crates.io/crates/trybuild
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate A-tokio-macros Area: The tokio-macros crate A-tokio-test Area: The tokio-test crate C-feature-accepted Category: A feature request that has been accepted pending implementation. C-feature-request Category: A feature request. E-help-wanted Call for participation: Help is requested to fix this issue. M-macros Module: macros in the main Tokio crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants