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: documentation: testing.T.Parallel resets time #12243

Closed
joshlf opened this issue Aug 21, 2015 · 9 comments
Closed

testing: documentation: testing.T.Parallel resets time #12243

joshlf opened this issue Aug 21, 2015 · 9 comments

Comments

@joshlf
Copy link

@joshlf joshlf commented Aug 21, 2015

The testing package's T.Parallel method resets the time of the test so that it's as if the test starts just before the method returns. The reason for doing this is that it's likely the first call made from the testing function. This is reasonable, but I suggest that this behavior be documented. The source is here.

@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Aug 21, 2015
@ianlancetaylor ianlancetaylor changed the title Documentation: testing.T.Parallel resets time testing: documentation: testing.T.Parallel resets time Aug 21, 2015
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 24, 2015

The reason for resetting the time is that the first time t.Parallel does is block the call until all the other non-parallel tests have completed. You don't want to include those in the time.

Perhaps instead it should pause the timer, not reset it.

@joshlf

This comment has been minimized.

Copy link
Author

@joshlf joshlf commented Oct 24, 2015

That would probably be more reasonable, but would it constitute a backwards-breaking change?

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Nov 5, 2015

No, I would not consider s/reset/pause/ a backwards-breaking change. Unless you have a specific scenario in mind?

@joshlf

This comment has been minimized.

Copy link
Author

@joshlf joshlf commented Nov 5, 2015

I was just thinking of any logic that relies on that functionality - technically backwards-breaking, but very unlikely to occur in practice.

@cespare

This comment has been minimized.

Copy link
Contributor

@cespare cespare commented Nov 17, 2015

@joshlf it's not exposed by the testing API, so the only way to rely on the test durations is if you're parsing go test output.

@cespare

This comment has been minimized.

Copy link
Contributor

@cespare cespare commented Nov 17, 2015

I sent a CL to change from resetting the timer to pausing it.

If we do the pause instead, is there any point in documenting it, or does it become sufficiently unsurprising in that case?

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 17, 2015

CL https://golang.org/cl/16989 mentions this issue.

@joshlf

This comment has been minimized.

Copy link
Author

@joshlf joshlf commented Nov 17, 2015

@cespare In that case I agree that it's not an issue.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Nov 24, 2015

Fixed by CL 16989.

@rsc rsc closed this Nov 24, 2015
rsc added a commit that referenced this issue Nov 24, 2015
Before, we reset the timer at the end of T.Parallel, which is okay
assuming that T.Parallel is the first thing in the test.

Snapshot the elapsed time at the beginning of Parallel and include it in
the total duration so that any time spent in the test before calling
Parallel is reported in the test duration as well.

Updates #12243.

Change-Id: Ieca553e1f801e16b9b6416463fa8f7fa65425185
Reviewed-on: https://go-review.googlesource.com/16989
Reviewed-by: Russ Cox <rsc@golang.org>
@golang golang locked and limited conversation to collaborators Nov 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.