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

Override steps and checks on a per check basis #35

Closed
pierre-fastly opened this issue Aug 22, 2022 · 7 comments
Closed

Override steps and checks on a per check basis #35

pierre-fastly opened this issue Aug 22, 2022 · 7 comments

Comments

@pierre-fastly
Copy link

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

rapid/engine.go

Line 119 in 110d7a5

func checkTB(tb tb, prop func(*T)) {
. 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
Copy link
Owner

flyingmutant commented Aug 22, 2022

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
Copy link
Author

pierre-fastly commented Aug 22, 2022

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
Copy link
Owner

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
Copy link
Author

Thank you! That's perfect for us.

@flyingmutant
Copy link
Owner

@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
Copy link
Author

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

@flyingmutant
Copy link
Owner

AFAIK until golang/go#48157 is implemented, only via existing global go test -timeout flag.

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

No branches or pull requests

2 participants