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

Parse test duration correctly for >= go 1.5 #438

Closed

Conversation

sridharv
Copy link

Currently parsing test durations is broken for go 1.5 and above. All test durations are currently reported as 0.00 seconds. This pull request contains a small fix.

func parseTestFunctionDuration(line string) float64 {
line = strings.Replace(line, "(", "", 1)
fields := strings.Split(line, " ")
return parseDurationInSeconds(fields[3]+"s", 2)
timeStr := fields[3]+"s"
if strings.HasSuffix(timeStr, "s)s") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why "s)s" ? wouldn't it be "s)" ?

Copy link
Author

Choose a reason for hiding this comment

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

The < go 1.5 case is treated as the default and so in the previous line we have

timeStr := fields[3]+"s"

For < go 1.5 a line like

--- PASS: TestOldSchool_PassesWithMessage (0.03 seconds)

would have fields[3] == 0.03 and timeStr == 0.03s

For >= go 1.5 with input like

--- PASS: TestOldSchool_PassesWithMessage (0.03s)

we'd have fields[3] == 0.03s) and timeStr == 0.03s)s

riannucci added a commit that referenced this pull request Oct 21, 2021
This updates the tests to include some changes from Pull #438.

I believe this confirms that the pull isn't actually needed.
@riannucci riannucci mentioned this pull request Oct 21, 2021
riannucci added a commit that referenced this pull request Oct 21, 2021
This updates the tests to include some changes from Pull #438.

I believe this confirms that the pull isn't actually needed.
@riannucci
Copy link
Collaborator

Current parser seems to be correct w/ updated tests.

@riannucci riannucci closed this Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants