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: add Count function to return the test.count value #64883

Closed
purpleidea opened this issue Dec 27, 2023 · 13 comments
Closed

proposal: testing: add Count function to return the test.count value #64883

purpleidea opened this issue Dec 27, 2023 · 13 comments
Labels
Milestone

Comments

@purpleidea
Copy link

Proposal Details

In golang 1.21.0 a new Testing function was added to the "testing" package. I propose a similar and new Count function which along the same lines, returns the test.count value for the test. Implementation is similar to Short shown here:

// Short reports whether the -test.short flag is set.
func Short() bool {
	if short == nil {
		panic("testing: Short called before Init")
	}
	// Catch code that calls this from TestMain without first calling flag.Parse.
	if !flag.Parsed() {
		panic("testing: Short called before Parse")
	}

	return *short
}

This would be very useful to allow a test to perform some more expensive global setup only once if it's going to be run multiple times, for example. Or to know how much longer we expect a test to last. (Eg: time*count)

Thanks!

@gopherbot gopherbot added this to the Proposal milestone Dec 27, 2023
@seankhliao
Copy link
Member

single setup should generally be done by other primitives, like sync.Once.

Having test behaviour change based on the number of times it runs seems like bad practice and defeats the point of running it multiple times.

@purpleidea
Copy link
Author

defeats the point of running it multiple times.

One clear example is a test that depends on global state and can only run once. So for example, the first time it runs, it would proceed as expected, but if testing.Count() was > 1, it would simply return early.

@seankhliao
Copy link
Member

that doesn't really support have a Count() function?
It doesn't help if you didn't know about it, and if you did, it would be better to either write your tests to not depend on global state or write them to setup global state correctly before execution

@purpleidea
Copy link
Author

Another good reason: If you're testing a large project that has init() being used, then these run just once (as expected) when the go test process starts up. But if you use -count 2, they don't re-run. So if you're expecting that you're testing the whole stack, then it doesn't actually happen. A Count() function would allow someone to write a test that fails or prints a warning or something different can be done when Count() > 1.

@seankhliao
Copy link
Member

That doesn't sound like a good reason to have Count, instead it sounds like one to not use init() (or use init only for stuff that really only does need to be initialized once, no matter how tests are run)

@purpleidea
Copy link
Author

purpleidea commented Dec 29, 2023 via email

@seankhliao
Copy link
Member

I still don't see how Count helps, if you're aware of the issue, you can implement workarounds (reexec), if you're not aware, then Count doesn't help you atl all

@G-Rath
Copy link

G-Rath commented Jan 9, 2024

This would be useful for https://github.com/gkampitakis/go-snaps in order to support having multiple snapshots in a single test while still comparing within the same count run.

In order to provide support for multiple snapshots, it uses a sync.Once based registry for tracking the number of calls to MatchSnapshot within each test, and because -count is not currently expressed anywhere (e.g. t.Name is the test name, not "name + count") it ends up counting the same calls as new calls within the same test rather than as repeated calls, and so generates new snapshots rather than comparing to the existing ones.

Exposing the -count value in some way (such as with testing.Count) would enable go-snaps to reset its global registry over each iteration; from what I understand currently there is no way to do this because e.g. TestMain is only called once (since the actual test count handling is done within m.Run).

@rsc
Copy link
Contributor

rsc commented Jan 18, 2024

I don't understand how this helps. Instead of checking whether testing.Count() > 1 you can just count the number of times the code has been run. Also if -count=2 then testing.Count()==2 both times, so you can't distinguish the first.

@purpleidea
Copy link
Author

you can just count the number of times the code has been run

This is a good workaround, and if I count and have testing.Count(), I can know this is iteration N of M.

I would also have just expected the value would be there by symmetry arguments since testing.Short() is, so I don't have to run flag parsing myself.

I also realize it's a niche request.

Thanks for looking!

@rsc rsc changed the title proposal: testing: Add Count function to return the test.count value proposal: testing: add Count function to return the test.count value Jan 24, 2024
@rsc
Copy link
Contributor

rsc commented Jan 26, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Feb 1, 2024

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Feb 8, 2024

No change in consensus, so declined.
— rsc for the proposal review group

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

No branches or pull requests

5 participants