flyingmutant / rapid

Rapid is a modern Go property-based testing library
https://pkg.go.dev/pgregory.net/rapid
Mozilla Public License 2.0
579 stars 25 forks source link

Override steps and checks on a per check basis #35

Closed pierre-fastly closed 2 years ago

pierre-fastly commented 2 years ago

My use case is I have a very long test case and running it with 100 checks and 100 steps take more than 10 minutes. I'm fine running it every now and then for that long but I'd like to, by default, reduce the number of checks and steps for just this check and leave the default for the other checks.

I'm thinking the best behavior would be command-line flags take precedence over overrides which in turn take precedence over default values.

Is this something you thought about already? Would you be open to adding this feature?

After looking at the source code, we definitely need to make the flags variable local and pass it to checkTB https://github.com/flyingmutant/rapid/blob/110d7a5dd15915adddbb9bf3d43667dbe5933e8d/engine.go#L119. Then the rest of the codebase would use this local variable instead of the global one. This would allows us to customize what's in the flags variable in any way we want.

One way that could make sense to customize the flags variable is to change the signature of Check to take in a variadic argument. This would make the change backwards compatible:

type SetOption func(*cmdline)

func Check(t *testing.T, prop func(*T), options ...SetOption)

And have a few helpers:

func OverrideChecks(checks int) SetOption {
    return func(c *cmdline) {
        c.checks = checks
    }
}

If you're okay to add this feature, I'll submit a PR with a possible implementation.

flyingmutant commented 2 years ago

very long test case and running it with 100 checks and 100 steps take more than 10 minutes

Have you profiled the test? rapid itself should be relatively lighweight, but it may be possible that this hits some bad case where rapid itself is the problem -- would be nice to rule this out.

One way that could make sense to customize the flags variable is to change the signature of Check to take in a variadic argument.

This is definitely an option, but I am vary of changing the Check signature now, as a) there are more directions in which I want to extend Check (for example, add an ability to manually specify the seeds from the past failed runs, to catch possible regressions) and b) given that the prop is usually specified inline, adding options at the end will read a bit backwards.

I was thinking maybe doing something like

rapid.WithExamples(...).WithOptions(...).Check(t, func(t *rapid.T) {
    // ...
})

but am not sure about it yet.

Another option is to use something like testing.Short to either skip the test completely, or for rapid to lower the default checks and/or steps for short test runs automatically. What do you think?

pierre-fastly commented 2 years ago

Have you profiled the test? rapid itself should be relatively lighweight, but it may be possible that this hits some bad case where rapid itself is the problem -- would be nice to rule this out.

Oh yes we know who the culprit is. We just started using rapid's state machine to test our queries to the databases. Indeed, rapid is not the issue here, it's the queries and there's no obvious way to make them faster. For example, an insert takes at best on my laptop 25ms. 100 checks 100 steps 25ms = 250s.

A small example would be:

type User struct {
    // ...
}

// This is the real service doing queries against the database
type UserService struct {
    // holds connection to the database
}

func (UserService) Insert(context.Context, User) error {}
func (UserService) FindByID(context.Context, string) (User, error) {}
func (UserService) FindAll(context.Context) ([]User, error) {}

type UserStateMachine {
    real UserService
    users map[string]User
}

// Sets up a database for the test and creates the stub and real instance
func (UserStateMachine) Init(*rapid.T)

// Inserts a drawn User with a non-existing ID (so we know this should succeed)
func (UserStateMachine) InsertNonexisting(*rapid.T)
// Inserts a drawn User with an existing ID (so we know this should fail)
func (UserStateMachine) InsertExisting(*rapid.T)
// Find a User with a non-existing drawn ID (so we know this should find none)
func (UserStateMachine) FindByIDNonexisting(*rapid.T)
// Find a User with an existing drawn ID (so we know this should find one)
func (UserStateMachine) FindByIDExisting(*rapid.T)

// Check calls FindAll
func (UserStateMachine) Check(*rapid.T)

The nice thing now is we can create a stub of the UserService, and test it against the same state machine, and we know it will behave the same as the real UserService. So for other tests depending on the UserService, we can use the stub and know it behaves like the real deal.

I like the With* methods as this allows users to share a common config for rapid in multiple places in the code base. That being said, I really like the use of Short as a small step towards a full override system. Now, for my use case here, I'd vote on have -short imply reducing the number of checks and steps. But I'd guess some others would want to skip the test completely. Maybe we could have a few helper methods like rapid.SkipOnShort() and rapid.ChecksOnShort(int) and rapid.StepsOnShort(int)?

flyingmutant commented 2 years ago

fa75bd5 lowers checks and steps 5x when -short is specified. To skip an excessively long test completely, just Skip it at the very start, before calling rapid.Check (as testing.Short docs suggest).

I'll wait before adding With* stuff for advanced configuration (probably will do after merging to master the generic code in dev branch).

pierre-fastly commented 2 years ago

Thank you! That's perfect for us.

flyingmutant commented 1 year ago

@pierre-fastly I am a bit vary of implementing full #38 (seems to be a bit out of line with how usual Go tests are configured/run), but I think 4391f11 is a neat trick that should help avoid the need to configure checks or steps manually. Does it look helpful for your use cases?

pierre-fastly commented 1 year ago

I like that indeed! Is there a way to configure the deadlines from the command line?

flyingmutant commented 1 year ago

AFAIK until https://github.com/golang/go/issues/48157 is implemented, only via existing global go test -timeout flag.