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

testing: panic in AllocsPerRun during parallel test #70464

Open
rsc opened this issue Nov 20, 2024 · 5 comments
Open

testing: panic in AllocsPerRun during parallel test #70464

rsc opened this issue Nov 20, 2024 · 5 comments

Comments

@rsc
Copy link
Contributor

rsc commented Nov 20, 2024

I propose that testing.AllocsPerRun panic if called during a parallel test,
because it cannot give accurate results in that context. Tests that use
the result in that context will be flaky.

For example, flake #70327 was caused by a test that called t.Parallel
and then used AllocsPerRun. The other tests running in parallel
had allocations that were observed by the AllocsPerRun, resulting
in inconsistent results and a flaky failure.

I've implemented this change in CL 630137.

@rsc rsc added this to the Proposal milestone Nov 20, 2024
@gabyhelp
Copy link

Related Code Changes

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@rsc rsc added this to Proposals Nov 20, 2024
@rsc rsc moved this to Incoming in Proposals Nov 20, 2024
@ianlancetaylor
Copy link
Contributor

Personally I think this is a bug fix. I don't think it needs to go through the proposal process. Since it can't work correctly, it's hard for me to believe that anybody depends on the current behavior.

@rsc
Copy link
Contributor Author

rsc commented Nov 20, 2024

I was on the fence about proposal or not. If the committee says not, that's fine with me! 😃

@rsc
Copy link
Contributor Author

rsc commented Dec 4, 2024

Discussed at proposal review and decided on quick accept.

@rsc rsc moved this from Incoming to Accepted in Proposals Dec 4, 2024
@rsc
Copy link
Contributor Author

rsc commented Dec 4, 2024

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The proposal is for testing.AllocsPerRun to panic if called during a parallel test.

@rsc rsc changed the title proposal: testing: panic in AllocsPerRun during parallel test testing: panic in AllocsPerRun during parallel test Dec 4, 2024
@rsc rsc modified the milestones: Proposal, Backlog Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests

4 participants