cmd/test2json: attribute output to the correct test #34419

dnephin · 2019-09-19T23:57:06Z

When printing regular test output check the indentation of the output, and use
the report stack to find the appropriate test name for that output.

This change includes a whitespace change to some golden test files. The
indentation of tests was changed in CL 113177
from tabs to spaces. The golden files have been updated to match the new
output format. The tabs in the golden files cause problems because the indentation check
looks for 4 spaces.

Fixes #29755
Updates #25369

gopherbot · 2019-09-20T00:12:58Z

This PR (HEAD: 271d398) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/196617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2019-09-20T17:01:47Z

Message from Bryan C. Mills:

Patch Set 1:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2019-09-20T17:15:37Z

Message from Bryan C. Mills:

Patch Set 1: Run-TryBot+1

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2019-09-20T17:15:38Z

Message from Gobot Gobot:

Patch Set 1:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=f79bbdd6

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2019-09-20T17:25:15Z

Message from Gobot Gobot:

Patch Set 1: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2019-09-20T19:53:54Z

This PR (HEAD: d29e9e4) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/196617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2019-09-20T19:58:14Z

Message from Daniel Nephin:

Patch Set 4:

(5 comments)

I believe I have addressed all the review comments.

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

dnephin · 2019-12-03T00:39:42Z

Hi, anything I can do to help move this along? Is there anything you need from me?

gopherbot · 2019-12-03T01:36:28Z

Message from Ian Lance Taylor:

Patch Set 4:

Is this still needed now that https://golang.org/cl/127120 has been submitted?

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2019-12-03T05:41:52Z

Message from Daniel Nephin:

Patch Set 4:

Patch Set 4:

Is this still needed now that https://golang.org/cl/127120 has been submitted?

I believe so, yes.
I tried the tests from this PR on master (386b1a4) and got the following failure:

TestGolden/issue29755: test2json_test.go:227: output mismatch for Test="TestOutputWithSubtest/sub_test/sub2":
    have "        --- PASS: TestOutputWithSubtest/sub_test/sub2 (0.00s)\n            foo_test.go:14: output from sub2 test\n    foo_test.go:22: output from root test\n    foo_test.go:27: output from root test\n"
    want "        --- PASS: TestOutputWithSubtest/sub_test/sub2 (0.00s)\n            foo_test.go:14: output from sub2 test\n"
TestGolden/issue29755: test2json_test.go:207: have:
    	{"Action":"output","Test":"TestOutputWithSubtest/sub_test/sub2","Output":"    foo_test.go:22: output from root test\n"}
    	{"Action":"output","Test":"TestOutputWithSubtest/sub_test/sub2","Output":"    foo_test.go:27: output from root test\n"}
    	» {"Action":"pass","Test":"TestOutputWithSubtest/sub_test/sub2"}
    	{"Action":"pass","Test":"TestOutputWithSubtest/sub_test"}
    	{"Action":"output","Test":"TestOutputWithSubtest/sub_test2","Output":"    --- PASS: TestOutputWithSubtest/sub_test2 (0.00s)\n"}
    	{"Action":"output","Test":"TestOutputWithSubtest/sub_test2","Output":"        foo_test.go:21: output from sub test2\n"}
    	{"Action":"output","Test":"TestOutputWithSubtest/sub_test2","Output":"        foo_test.go:23: more output from sub test2\n"}
    want:
    	{"Action":"output","Test":"TestOutputWithSubtest/sub_test/sub2","Output":"        --- PASS: TestOutputWithSubtest/sub_test/sub2 (0.00s)\n"}
    	{"Action":"output","Test":"TestOutputWithSubtest/sub_test/sub2","Output":"            foo_test.go:14: output from sub2 test\n"}
    	» {"Action":"output","Test":"TestOutputWithSubtest","Output":"    foo_test.go:22: output from root test\n"}
    	{"Action":"output","Test":"TestOutputWithSubtest","Output":"    foo_test.go:27: output from root test\n"}
    	{"Action":"pass","Test":"TestOutputWithSubtest/sub_test/sub2"}
    	{"Action":"pass","Test":"TestOutputWithSubtest/sub_test"}
    	{"Action":"output","Test":"TestOutputWithSubtest/sub_test2","Output":"    --- PASS: TestOutputWithSubtest/sub_test2 (0.00s)\n"}

with the patch applied the tests pass.

I don't think https://golang.org/cl/127120 addresses the problem.

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-31T19:08:26Z

Message from Daniel Nephin:

Patch Set 4:

Hello,

This bug is still present in go1.14.1. I would really like to get this fixed as having test output hidden is causing lots of problems.

How can I help move this along?

Thank you!
Daniel

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-31T19:57:40Z

Message from Bryan C. Mills:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

The behaviour of 'go test' on go1.13 is to use 4 spaces for this output. The indentation character is significant because test2json uses it to determine which test produced the output.

gopherbot · 2020-04-11T22:00:30Z

This PR (HEAD: feafbf6) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/196617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

If a test prints output after a subtest has started it was being attributed to the last subtest. This change uses the identation level of the output to determine which test the output comes from.

gopherbot · 2020-04-11T22:09:09Z

This PR (HEAD: 03b1b73) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/196617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2020-04-11T22:11:44Z

Message from Daniel Nephin:

Patch Set 6:

(1 comment)

Patchset 6 should address the last issue. Thank you!

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-04-11T23:17:35Z

Message from Daniel Nephin:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-04-19T18:21:59Z

This PR (HEAD: 898827f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/196617 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2020-05-16T21:20:57Z

Message from Daniel Nephin:

Patch Set 7:

Checking in again to see if I can help move this along. I added a much longer comment to explain why the second condition is necessary.

This continues to be a major problem for anyone using test2json with sub tests. Output is attributed to the wrong test, making it very difficult to debug failures.

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-05-29T21:04:49Z

Message from Bryan C. Mills:

Patch Set 8: Patch Set 7 was rebased

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-05-29T21:04:49Z

Message from Bryan C. Mills:

Patch Set 8: Run-TryBot+1 Code-Review+2

(1 comment)

Sorry for the delay; I've had a bit of trouble keeping pace with code reviews since April.

This change overall looks good to me, although I'm not sure whether it should go in during the code freeze.

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-05-29T21:04:50Z

Message from Gobot Gobot:

Patch Set 8:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=99b7928c

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-05-29T21:11:40Z

Message from Gobot Gobot:

Patch Set 8:

Build is still in progress...
This change failed on windows-amd64-2016:
See https://storage.googleapis.com/go-build-log/99b7928c/windows-amd64-2016_d12c86f9.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-05-29T21:16:53Z

Message from Gobot Gobot:

Patch Set 8: TryBot-Result-1

1 of 20 TryBots failed:
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/99b7928c/windows-amd64-2016_d12c86f9.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-06-01T14:46:56Z

Message from Bryan C. Mills:

Patch Set 9: Patch Set 8 was rebased

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-06-01T14:46:57Z

Message from Bryan C. Mills:

Patch Set 9: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-06-01T14:46:58Z

Message from Gobot Gobot:

Patch Set 9:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=4eed6fa0

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-06-01T14:58:08Z

Message from Gobot Gobot:

Patch Set 9: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-06-01T15:50:31Z

Message from Dmitri Shuralyov:

Patch Set 9:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-06-01T16:09:49Z

Message from Bryan C. Mills:

Patch Set 10: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

When printing regular test output check the indentation of the output, and use the report stack to find the appropriate test name for that output. This change includes a whitespace change to some golden test files. The indentation of tests was changed in CL 113177 from tabs to spaces. The golden files have been updated to match the new output format. The tabs in the golden files cause problems because the indentation check looks for 4 spaces. Fixes #29755 Updates #25369 Change-Id: Iebab51816a9755168083a7a665b41497e9dfd85f GitHub-Last-Rev: 898827f GitHub-Pull-Request: #34419 Reviewed-on: https://go-review.googlesource.com/c/go/+/196617 Reviewed-by: Bryan C. Mills <bcmills@google.com>

gopherbot · 2020-06-01T16:14:29Z

Message from Bryan C. Mills:

Patch Set 12:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-06-01T16:16:07Z

Message from Bryan C. Mills:

Patch Set 12:

This is a bug-fix and doesn't affect anything outside of cmd/test2json, so ok for 1.15.

Merging based on TryBot +1 from PS9. (Only change since then is to the commit messsage).

Please don’t reply on this GitHub thread. Visit golang.org/cl/196617.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-06-01T16:20:36Z

This PR is being closed because golang.org/cl/196617 has been merged.

googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Sep 19, 2019

dnephin force-pushed the subtest-output-test2json branch from 271d398 to d29e9e4 Compare September 20, 2019 19:53

gopherbot force-pushed the master branch 3 times, most recently from 53bd915 to 6139019 Compare October 3, 2019 21:09

gopherbot force-pushed the master branch from 07b4abd to 19a7490 Compare October 9, 2019 22:40

dnephin mentioned this pull request Oct 15, 2019

test2json incorrectly attributes output to a subtest, resulting in missing test failure output gotestyourself/gotestsum#66

Closed

gopherbot force-pushed the master branch from 03fb1f6 to a813d3c Compare October 22, 2019 19:40

gopherbot force-pushed the master branch 11 times, most recently from 4a7ed1f to 0f992b9 Compare November 5, 2019 02:56

Convert tabs to spaces in golden files

264aed2

The behaviour of 'go test' on go1.13 is to use 4 spaces for this output. The indentation character is significant because test2json uses it to determine which test produced the output.

dnephin force-pushed the subtest-output-test2json branch from d29e9e4 to feafbf6 Compare April 11, 2020 21:59

test2json: use indentation to determine source of test output

03b1b73

If a test prints output after a subtest has started it was being attributed to the last subtest. This change uses the identation level of the output to determine which test the output comes from.

dnephin force-pushed the subtest-output-test2json branch from feafbf6 to 03b1b73 Compare April 11, 2020 22:08

Improve the testcase for issue29755

898827f

dnephin mentioned this pull request Apr 19, 2020

mitigate test2json bug causing missing test output gotestyourself/gotestsum#102

Merged

gopherbot closed this Jun 1, 2020

dnephin deleted the subtest-output-test2json branch June 1, 2020 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/test2json: attribute output to the correct test #34419

cmd/test2json: attribute output to the correct test #34419

dnephin commented Sep 19, 2019 •

edited by bcmills

Loading

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

dnephin commented Dec 3, 2019

gopherbot commented Dec 3, 2019

gopherbot commented Dec 3, 2019

gopherbot commented Mar 31, 2020

gopherbot commented Mar 31, 2020

gopherbot commented Apr 11, 2020

gopherbot commented Apr 11, 2020

gopherbot commented Apr 11, 2020

gopherbot commented Apr 11, 2020

gopherbot commented Apr 19, 2020

gopherbot commented May 16, 2020

gopherbot commented May 29, 2020

gopherbot commented May 29, 2020

gopherbot commented May 29, 2020

gopherbot commented May 29, 2020

gopherbot commented May 29, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

cmd/test2json: attribute output to the correct test #34419

cmd/test2json: attribute output to the correct test #34419

Conversation

dnephin commented Sep 19, 2019 • edited by bcmills Loading

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

gopherbot commented Sep 20, 2019

dnephin commented Dec 3, 2019

gopherbot commented Dec 3, 2019

gopherbot commented Dec 3, 2019

gopherbot commented Mar 31, 2020

gopherbot commented Mar 31, 2020

gopherbot commented Apr 11, 2020

gopherbot commented Apr 11, 2020

gopherbot commented Apr 11, 2020

gopherbot commented Apr 11, 2020

gopherbot commented Apr 19, 2020

gopherbot commented May 16, 2020

gopherbot commented May 29, 2020

gopherbot commented May 29, 2020

gopherbot commented May 29, 2020

gopherbot commented May 29, 2020

gopherbot commented May 29, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

gopherbot commented Jun 1, 2020

dnephin commented Sep 19, 2019 •

edited by bcmills

Loading