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

Add flag -testify.shuffle for suites #1413

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

stamm
Copy link

@stamm stamm commented Jul 9, 2023

Summary

Add flag -testify.shuffle=(off|on|N) to randomize execution of a suite's tests

Changes

Add flag -testify.shuffle=(off|on|N)

Motivation

If go 1.17 was added ability to run test in random order.
I'd like to randomize tests inside suite.

Example usage (if applicable)

go test -testify.shuffle=on ./...

@dolmen dolmen changed the title add flag testify.shuffle Add flag -testify.shuffle for suites Jul 11, 2023
suite/suite.go Outdated
} else {
n, err = strconv.ParseInt(*shuffle, 10, 64)
if err != nil {
fmt.Fprintln(os.Stderr, `testing: -shuffle should be "off", "on", or a valid integer:`, err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use t.Fatal or t.Skip.

suite/suite.go Outdated
return
}
}
fmt.Println("-testify.shuffle", n)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use t.Log.

suite/suite.go Outdated
@@ -17,6 +19,7 @@ import (

var allTestsFilter = func(_, _ string) (bool, error) { return true, nil }
var matchMethod = flag.String("testify.m", "", "regular expression to select tests of the testify suite to run")
var shuffle = flag.String("testify.shuffle", "off", "randomize the execution order of tests")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a reference to -test.shuffle:

randomize the execution order of tests in testify suites, like -test.shuffle

suite/suite.go Show resolved Hide resolved
suite/suite.go Show resolved Hide resolved

func TestShuffleOrderSuiteShuffleOn(t *testing.T) {
// To test this with testify.shuffle on we launch it in its own process
cmd := exec.Command("go", "test", "-v", "-race", "-run", "TestSuiteShuffleOrder", "-testify.shuffle", "2")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment mentionning that the test reliability relies on the algorithm used by package math/rand. If that algorithm changes the seed value may have to be adapted.


func TestShuffleOrderSuiteShuffleOn(t *testing.T) {
// To test this with testify.shuffle on we launch it in its own process
cmd := exec.Command("go", "test", "-v", "-race", "-run", "TestSuiteShuffleOrder", "-testify.shuffle", "2")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test doesn't have to run in -race mode which is slower.

@dolmen dolmen added the pkg-suite Change related to package testify/suite label Jul 11, 2023
@dolmen
Copy link
Collaborator

dolmen commented Jul 11, 2023

This seems a good feature to add.

@stamm
Copy link
Author

stamm commented Jul 11, 2023

@dolmen thanks for review! I have fixed all the issues, if you have time, could you review it again?

@stamm
Copy link
Author

stamm commented Jul 17, 2023

@dolmen I'm sorry to bother you, but any chance to be reviewed again?

Copy link
Collaborator

@dolmen dolmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation requires users of package suite to use a specific kind of Suite. This is inconsistent with the shuffle flag of go test.

I think it would be better to instead change the default behavior of Suite. It would be appreciated if you could submit that alternative implementation as a separate PRs to let us choose the one to merge.

@dolmen
Copy link
Collaborator

dolmen commented Oct 16, 2023

This patch conflicts with #1471 which will soon be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement hacktoberfest-accepted Hacktoberfest pkg-suite Change related to package testify/suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants