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

proposal: testing: expose -test.shuffle seed in a public function. #66623

Closed
ucirello opened this issue Mar 31, 2024 · 4 comments
Closed

proposal: testing: expose -test.shuffle seed in a public function. #66623

ucirello opened this issue Mar 31, 2024 · 4 comments
Labels
Milestone

Comments

@ucirello
Copy link
Contributor

ucirello commented Mar 31, 2024

Proposal Details

#28592 added support for go test -shuffle off|on|seed.

#61256 debates the need to redo the seed when executing tests -count times.

This proposal would expose to the tests the seed used on invocation so as to enable reproducible tests.

Currently, this workaround is possible:

func TestMain(m *testing.M) {
	flag.Parse()
	shuffleFlag := flag.Lookup("test.shuffle")
	if shuffleFlag != nil {
		shuffle := shuffleFlag.Value.String()
		if shuffle != "on" && shuffle != "off" && shuffle != ""  {
			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)
				os.Exit(2)
				return
			}
			rand.Seed(n)
		}
	}
	os.Exit(m.Run())
}

This workaround has two problems. First problem, it relies on a deprecated interface (math/rand.Seed). Second problem, it will not share the same seed with -test.shuffle on.

Thus this proposal modifies testing to add this call.

// ShuffleSeed reports the environment random seed.
func ShuffleSeed() int64

With this interface, one can use:

$ go test -shuffle 12389761 -run=TestRandomnessSensitive # specific seed that causes a problem to happen

Code:

func TestRandomnessSensitive(t *testing.T) {
        rng := rand.New(rand.NewSource(testing.ShuffleSeed()))
        _ = rng // use RNG that will cause the issue.
}
@gopherbot gopherbot added this to the Proposal milestone Mar 31, 2024
@seankhliao
Copy link
Member

This appears to overload the use of the -shuffle flag for not just randomizing the order of tests, but also some other application specific use cases.
I think applications would be better off defining their own flag for use in seeding their RNG.

@ucirello
Copy link
Contributor Author

ucirello commented Apr 1, 2024

I based my decision on issuing this proposal based off of the help text:

        -shuffle off,on,N
            Randomize the execution order of tests and benchmarks.
            It is off by default. If -shuffle is set to on, then it will seed
            the randomizer using the system clock. If -shuffle is set to an
            integer N, then N will be used as the seed value. In both cases,
            the seed will be reported for reproducibility.

Perhaps I misinterpreted it?

@seankhliao
Copy link
Member

seankhliao commented Apr 1, 2024

The help text clearly states that it is only for shuffling the order of tests, not for any other purposes. and for reproducibility, the seed value is reported.

rng := rand.New(rand.NewSource(n))

@ucirello
Copy link
Contributor Author

ucirello commented Apr 1, 2024

I think I misunderstood of the spirit of the shuffle flag.

I am going to retract this proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants