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] Attribute Macro #153

Open
davidbarsky opened this issue Jul 1, 2019 · 10 comments
Open

#[proptest] Attribute Macro #153

davidbarsky opened this issue Jul 1, 2019 · 10 comments
Labels
feature-request This issue is requesting new functionality

Comments

@davidbarsky
Copy link

Hi! I wrote a kinda rough implemenation of a #[proptest] attribute macro in awslabs/aws-lambda-rust-runtime#111. It accepts an impl Strategy<Value = T>, and I wanted to see if there's interest in me sending a pull request to introduce it here. It looks a bit like this:

pub fn gen_function_arn() -> impl Strategy<Value = FunctionArn> {
    let expr = "arn:aws:lambda:us-east-1:[0-9]{12}:function:custom-runtime";
    let arn = string_regex(expr).unwrap();
    arn.prop_map(FunctionArn)
}

#[proptest(gen_function_arn())]
fn function_arn(arn: FunctionArn) {
    let mut headers = HeaderMap::new();
    headers.typed_insert(arn.clone());
    prop_assert_eq!(headers.typed_get::<FunctionArn>(), Some(arn));
}
@Centril
Copy link
Collaborator

Centril commented Jul 1, 2019

cc #149 and #97 (comment).

A #[proptest] attribute would be neat I think.


We'll need to think about the details like:

  • how to most ergonomically provide custom strategies
  • how configurations should be passed
  • how function return types should be dealt with in terms of Termination or some other trait
  • whether closure style invocation can be supported,

Also, if you just write:

#[proptest]
fn addition_is_commutative(x: u8, y: u8) {
   ... 
}

then presumably any::<u8>() will be used for both.


#[proptest(gen_function_arn())]

The notation here for passing custom strategies seems suboptimal but serviceable in the interim.

It could be improved with rust-lang/rust#60406 but that isn't stable yet (tho one some bugs are fixed it could be stabilized relatively quickly after baking for 6 weeks). This would allow e.g.:

#[proptest]
fn function_arn(
    #[strategy = gen_function_arn()]
    arn: FunctionArn
) {

We also cannot support the fn foo(pat in expr) { notation either with #[proptest] as parsing is done before attributes are expanded and the type must always be provided lest we allow pat ::= ... | pat ":" type ; into Rust's pattern grammar (which could possibly be done as part of rust-lang/rfcs#2522).

@davidbarsky
Copy link
Author

We'll need to think about the details like:

  • how to most ergonomically provide custom strategies
  • how configurations should be passed
  • how function return types should be dealt with in terms of Termination or some other trait
  • whether closure style invocation can be supported,

I'd be interested in hearing your thoughts on these! I've only used a minimal portion of proptest, so beyond what is in the Lambda runtime I linked, I don't really know how those other features of proptest ought be supported.

Also, if you just write:

#[proptest]
fn addition_is_commutative(x: u8, y: u8) {
   ... 
}

then presumably any::() will be used for both.

I like that approach!

#[proptest(gen_function_arn())]
The notation here for passing custom strategies seems suboptimal but serviceable in the interim.

Yeah, I'm not a fan of my current approach either. I think the correct solution/notation is, as you said, rust-lang/rust#60406. Would you be okay with a minimal, off-by-default feature flag that allows users to create & pass a custom strategy in the serviceable form I described above?

We also cannot support the fn foo(pat in expr) { } notation

I don't really have a strong opinion on tackling this, as I wasn't a big fan of the pat in expr notation. The creation of the underlying strategy felt a bit too opaque for my liking/aesthetic sensibilities.

@Centril
Copy link
Collaborator

Centril commented Jul 1, 2019

I'd be interested in hearing your thoughts on these! I've only used a minimal portion of proptest, so beyond what is in the Lambda runtime I linked, I don't really know how those other features of proptest ought be supported.

Here are my thoughts:

How to most ergonomically provide custom strategies

This is mostly the discussion re. pat in expr and rust-lang/rust#60406 ;)

How configurations should be passed

I figure we can take this design decision straight from proptest! in the form of #[proptest_config...], e.g.:

#[proptest]
#[proptest_config(ProptestConfig { cases: 99, .. ProptestConfig::default() })]
fn foo(...) { ... }

Tho we could make this "more first class":

#[proptest]
#[proptest::cases = 99]
fn foo(...) { ... }

or:

#[proptest]
#[proptest_config(cases = 99)]
fn foo(...) { ... }

How function return types should be dealt with in terms of Termination or some other trait

I think you can largely do this by taking the approach of normalizing fn foo(...) { to fn foo() -> () {, then parse the return type with syn and then we could take the approach of quickcheck::Testable which would let us have e.g. -> bool { ... } which is typical in property based testing environments.

One thing to think about here is whether we want the original function fn foo to remain in place?
E.g. given:

#[proptest]
fn foo(x: u8) -> bool { ... }

we could expand to:

fn foo(x: u8) -> bool { ... }

#[test]
fn foo_test() {
    // 1. Setup test runner and strategies:
    // 2. Invoke the runner on strategies, and:
    // 2. a) Call `foo(..)` in the runner and store the result to `res`.
    // 2. a) Convert `res` into `TestCaseResult` through a trait.
}

But we could also do this later and iterate on this (don't have to reach perfection at once!)

I like that approach!

Great :)
(The proptest! macro already does this for you as well.)

Would you be okay with a minimal, off-by-default feature flag that allows users to create & pass a custom strategy in the serviceable form I described above?

Sure. Alternatively, we could leave it on-by-default and then make a breaking change as needed when Rust has the features we need.

I don't really have a strong opinion on tackling this, as I wasn't a big fan of the pat in expr notation. The creation of the underlying strategy felt a bit too opaque for my liking/aesthetic sensibilities.

Hehe ;) I don't think we can tackle it even if we wanted to as nothing in Rust's grammar will allow pat ::= .. | pat OP expr ; (see syntax::ast::PatKind).

Whether closure style invocation can be supported

No idea (for now) -- let's punt on this :)

@dcreager
Copy link

dcreager commented Jul 9, 2019

There's another possibility for how to provide custom strategies, which would be to encode them at the type level. Right now something like:

#[proptest] 
fn test_foo(x: u8) {
  assert!(x > 0);
}

is nice because it's syntactically a valid function (which is needed if you want to attach an attribute macro to it), but it only works if the type implements Arbitrary.

If I wanted to use a custom strategy that only produces even numbers, what if we could aim for a syntax like:

#[proptest] 
fn test_foo(x: Even<u8>) {
  assert!(x > 0);
}

Here we encode the strategy as the hypothetical Even<u8> type, instead of as the result of the even_integer function as shown in the book.

What would be wonky here is that inside the body of the function, x would still be a u8, even though it seems like it should be an Even<u8> if you just look at the parameter list. That might be initially confusing — though on the other hand, it means that you've always got a (syntactically) valid function body, that rustfmt and friends will know what to do with. And if you squint, you could interpret x: Even<u8> as "x is a u8 that happens to be Even".

Another nice feature is that this is something I think we could do today with Rust stable: you'd just remove the requirement in Arbitrary that Self::Strategy::Value == Self. So then the macro machinery would pick up the Arbitrary implementation for Even<u8> to get the actual strategy, and grab that strategy's Value type to figure out the actual type of value to pass into the test function. (And for this example, <Even<u8> as Arbitrary>::Strategy::Value would be u8.)

Thoughts?

@Centril
Copy link
Collaborator

Centril commented Jul 9, 2019

Here we encode the strategy as the hypothetical Even<u8> type, instead of as the result of the even_integer function as shown in the book.

This is standard practice in the Haskell community with QuickCheck; e.g.

is_even :: Even -> bool
is_even (Even x) = x `mod` 2 == 0

but here standard pattern matching is used to extract out the x from data Even = Even Integer.

What would be wonky here is that inside the body of the function, x would still be a u8, even though it seems like it should be an Even<u8> if you just look at the parameter list. That might be initially confusing — though on the other hand, it means that you've always got a (syntactically) valid function body, that rustfmt and friends will know what to do with. And if you squint, you could interpret x: Even<u8> as "x is a u8 that happens to be Even".

I do think this is surprising, not just initially, in that it breaks some basic things you expect about Rust. Consider also that someone not familiar with the codebase might be thrown into it.

Another nice feature is that this is something I think we could do today with Rust stable: you'd just remove the requirement in Arbitrary that Self::Strategy::Value == Self.

This was a pretty intentional design choice. I expect that if something says T: Arbitrary then I can use that implementation to generate arbitrary Ts -- this is basically the whole idea of Arbitrary: it provides a canonical way to construct arbitrary instances of the implementing type. Also, by removing this equality constraint the type inferencer can no longer assume it and so if you work with generic cases you will have to impose these constraints yourself.

Instead, taking inspiration from Haskell's QuickCheck, you can do something like:

#[proptest]
fn is_even(Even(x): Even<u8>) {
    assert!(x % 2 == 0);
}

or if the older version of rust-lang/rfcs#2522 was implemented you could also write:

#[proptest]
fn is_even(Even(x: u8)) {
    assert!(x % 2 == 0);
}

(this would only require syntactic support from Rust, semantic restrictions can still be in place)

(It should also be relatively easy to #[derive(Arbitrary)] for Even and rust-lang/rust#29661 should make it simpler in the cases where you need a manual implementation)

@CAD97
Copy link

CAD97 commented Feb 14, 2020

Just a few notes on syntax that I ran accross when considering this. All of the below is syntactically valid today (i.e. it will work under #[cfg(FALSE)]).

It's possible to (ab)use const generics syntax to get something close to the current prop in strategy syntax:

#[proptest]
fn my_test(bytes: In<{vec(any::<u8>(), 0..10_000)}>) {}

Having the strategy as an argument attribute is also clean, and makes the actual type of the parameter obvious:

#[proptest]
fn my_test(
    #[proptest(strategy = vec(any::<u8>(), 0..10_000))]
    bytes: Vec<u8>,
) {}

Side note: alternate test runners (e.g. #179) aren't too hard to support with an attribute:

#[proptest(tokio::test)]
async fn my_test(
    #[proptest(strategy = vec(any::<u8>(), 0..10_000))]
    bytes: Vec<u8>,
) {}

@Andlon
Copy link
Contributor

Andlon commented Jan 4, 2021

I just came across the test-strategy crate, by @frozenlib. Quite frankly, it looks great, and it would be amazing to have this kind of functionality in proptest directly. I opened an issue there for discussing a possible integration.

@matthew-russo matthew-russo added the feature-request This issue is requesting new functionality label Jan 26, 2023
@cameron1024
Copy link
Member

I use test-strategy extensively - I significantly prefer the attribute macro to proptest! {} for a few reasons:

  • it removes a level of indentation
  • it looks like "#[test] but with parameters", which is the right mental model IMO
  • anecdotally, it seems to play nicer with autocomplete/rust-analyzer
  • in very large proptest! { } blocks, it can be easy to forget that you're inside one, and seeing fn foo(x in any::<Something>()) {} can be a bit jarring until you remember "oh wait, it's proptest"

As for syntax, I'd like to see something like:

#[proptest]
fn simple(x: i32) { ... } 

#[proptest(
  cases = 50,
  // any other field of `ProptestConfig`
  async = tokio::test,
)]
async fn complex(
  #[strategy = foo_strategy()]
  foo: Foo,
  #[strategy = bar_strategy()]
  bar: Bar,
) { ... }

Of course, exact details are up for bikeshedding

@orsinium
Copy link

The feature also would let rstest to integrate with proptest, see la10736/rstest#30 (comment)

@cameron1024
Copy link
Member

I've begun work on this, there's a WIP PR, I'll have a look at that issue, but if there's any specifics that you we should bear in mind while implementing it, that's the best place to do it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request This issue is requesting new functionality
Projects
None yet
Development

No branches or pull requests

8 participants