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

proptest! macro doesn't play nice with tokio::test #179

Open
znewman01 opened this issue Jan 31, 2020 · 4 comments
Open

proptest! macro doesn't play nice with tokio::test #179

znewman01 opened this issue Jan 31, 2020 · 4 comments

Comments

@znewman01
Copy link

znewman01 commented Jan 31, 2020

Hi! This is purely an ergonomics thing.

$ grep -E 'proptest|tokio' Cargo.toml
tokio = { version = "0.2.11", features = [ "macros", "signal" ] }
proptest = "0.9.4"
$ rustc --version
rustc 1.41.0 (5e1a79984 2020-01-27)

I'm working on some async-heavy code and like to use the tokio::test macro:

#[tokio::test]
async fn test_tokio() {
    assert_eq!(ready(()).await, ());
}

If I try to adapt it to the proptest context:

proptest! {
    #[tokio::test]
    async fn test_prop_tokio(byte in any::<u8>()) {
        assert_eq!(ready(byte).await, byte);
    }
}

I get some inscrutable errors:

error: expected one of `move`, `|`, or `||`, found keyword `fn`
  --> src/lib.rs:59:15
   |
59 |         async fn test_prop_tokio(byte in any::<u8>()) {
   |               ^^ expected one of `move`, `|`, or `||`

The following works around fine:

proptest! {
    #[test]
    fn test_prop(byte in any::<u8>()) {
        Runtime::new().unwrap().block_on(async {
            assert_eq!(ready(byte).await, byte);
        });
     }
}

(Using the proptest! macro with a closure doesn't, because it doesn't handle async closures.)

I think it'd be a little silly to special-case proptest for every test runner, but as I understand, the Tokio test macro pretty much just does my workaround.

I think the proptest! macro is very similar. I wonder if we could make one of the following changes:

  • proptest! would go and expand its code first; the second test macro should pick up the rest. I don't totally get why this doesn't happen currently.
  • add e.g. a #[proptest::test] macro, which would basically do what proptest! does but to one test. The hitch is that tokio::test wants (1) no arguments, so we'd have to put it after #[proptest::test], and (2) no extra #[test], which means we'd need #[proptest::test] to (have an option to) not emit that. We could also maybe address this at the Tokio end.

IIRC custom test runner stuff is pretty in-flux right now, so maybe it's premature to do anything. Or I could be missing something obvious.

If you like any of these ideas or something similar, I might be able to take a stab at implementing, though I'm a little shaky on macros.

The above code assumes the following preamble:

use futures::future::ready;
use proptest::prelude::*;
use tokio::runtime::Runtime;

P.S. Thanks for your work on proptest; it's really helped me write high-assurance software and made testing fun to boot!

@CAD97
Copy link

CAD97 commented Feb 14, 2020

The root issue is that proptest! doesn't allow async fn. Adding async fn cases to the macro should make this "just work".

@johnchildren
Copy link

I would be interested in adding async support to the proptest macro, would it make sense to just add async versions of each case or have a separate proptest_async macro? I suppose it would be nice to be able to mix and match tests in the same usage.

@Jannis
Copy link

Jannis commented Dec 10, 2021

It would be great to have this!

@matthew-russo
Copy link
Member

we'll be looking at async support holisitically in #442

dcherian added a commit to earth-mover/icechunk that referenced this issue Aug 20, 2024
Use the test_strategy::proptest macro for nice async support.

Upstream proptest+tokio issue:
proptest-rs/proptest#179

While there is a workaround there, the `prop_assert!` macro didn't
seem to work properly. The `assert!` macro does work, but then we see
all the output when proptest tries to shrink a failing example.
dcherian added a commit to earth-mover/icechunk that referenced this issue Aug 20, 2024
Use the test_strategy::proptest macro for nice async support.

Upstream proptest+tokio issue:
proptest-rs/proptest#179

While there is a workaround there, the `prop_assert!` macro didn't
seem to work properly. The `assert!` macro does work, but then we see
all the output when proptest tries to shrink a failing example.
dcherian added a commit to earth-mover/icechunk that referenced this issue Aug 20, 2024
Use the test_strategy::proptest macro for nice async support.

Upstream proptest+tokio issue:
proptest-rs/proptest#179

While there is a workaround there, the `prop_assert!` macro didn't
seem to work properly. The `assert!` macro does work, but then we see
all the output when proptest tries to shrink a failing example.
dcherian added a commit to earth-mover/icechunk that referenced this issue Aug 20, 2024
Use the test_strategy::proptest macro for nice async support.

Upstream proptest+tokio issue:
proptest-rs/proptest#179

While there is a workaround there, the `prop_assert!` macro didn't
seem to work properly. The `assert!` macro does work, but then we see
all the output when proptest tries to shrink a failing example.
dcherian added a commit to earth-mover/icechunk that referenced this issue Aug 22, 2024
* Add more property tests.

Use the test_strategy::proptest macro for nice async support.

Upstream proptest+tokio issue:
proptest-rs/proptest#179

While there is a workaround there, the `prop_assert!` macro didn't
seem to work properly. The `assert!` macro does work, but then we see
all the output when proptest tries to shrink a failing example.

* More unwraps to assert

* Add some array tests

* hypothesis style naming

* fix comment

* Better array metadata strategy

* Use Arbitrary

* rename

* Add regression file

* bump version

* Option<DimensionNames>

* More feedbaclk

* Move strategies module under tests

* Make test-strategy dev dep only

* Revert "Make test-strategy dev dep only"

This reverts commit 93f5ed5.

* cleanup

* Tweak one flatmap

* Move strategies to new module

* Align dimensionname type with spec

* Check error types where we can

* Implement fmt::Debug trait on Storage

* update cargo.toml
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 a pull request may close this issue.

5 participants