test2json: associate streaming log output with the originating test #38189

mdwhatcott · 2020-03-31T23:11:17Z

Up until this commit the test2json utility made the assumption that
log output should be associated with the test named in the most recent
line starting with "--- PASS: Test..." or "--- FAIL: Test...".

A consequence of issue #24929 is that test log output is no longer
buffered (it is emitted almost immediately), making the aforementioned
assumption incorrect in the case of tests run in parallel.

This commit introduces a map which stores the names of all tests to
facilitate attributing emitted logs with their originating tests so
that readers of test2json such as IDEs and editors can align log output
containing important details about specific test failures with the
corresponding test name.

Fixes #38050

Up until this commit the test2json utility made the assumption that log output should be associated with the test named in the most recent line starting with "--- PASS: Test..." or "--- FAIL: Test...". A consequence of issue #24929 is that test log output is no longer buffered (it is emitted almost immediately), making the aforementioned assumption incorrect in the case of tests run in parallel. This commit introduces a map which stores the names of all tests to facilitate attributing emitted logs with their originating tests so that readers of test2json such as IDEs and editors can align log output containing important details about specific test failures with the corresponding test name. Fixes #38050

gopherbot · 2020-03-31T23:14:24Z

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

Please visit https://go-review.googlesource.com/c/go/+/226757 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-03-31T23:15:48Z

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

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

gopherbot · 2020-04-15T18:13:54Z

Message from Bryan C. Mills:

Patch Set 1:

(5 comments)

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

Related to #38050

gopherbot · 2020-04-15T22:05:59Z

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

Please visit https://go-review.googlesource.com/c/go/+/226757 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-16T05:35:14Z

Message from Michael Whatcott:

Patch Set 2:

(5 comments)

I've incorporated the suggestions in the comments for patch set 1 in patch set 2. I appreciate the feedback and guidance.

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

gopherbot · 2020-04-18T03:02:24Z

Message from Bryan C. Mills:

Patch Set 2:

Patch Set 2:

(5 comments)

I've incorporated the suggestions in the comments for patch set 1 in patch set 2. I appreciate the feedback and guidance.

I ran out of review bandwith for this week, but plan to do another review pass next week.

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

gopherbot · 2020-04-18T03:16:55Z

Message from Michael Whatcott:

Patch Set 2:

Patch Set 2:

Patch Set 2:

(5 comments)

I've incorporated the suggestions in the comments for patch set 1 in patch set 2. I appreciate the feedback and guidance.

I ran out of review bandwith for this week, but plan to do another review pass next week.

Thanks!

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

gopherbot · 2020-04-22T17:49:36Z

Message from Bryan C. Mills:

Patch Set 2:

(1 comment)

Let's put this on hold until we decide what to do about #38458.

Certainly we should add the new integration test either way, but we will likely be able to back out the changes to test2json proper.

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

gopherbot · 2020-04-22T17:50:54Z

Message from Bryan C. Mills:

Patch Set 2:

(1 comment)

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

Related to the following issues: Fixes #38050 Fixes #38458

gopherbot · 2020-04-23T01:36:06Z

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

Please visit https://go-review.googlesource.com/c/go/+/226757 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-23T02:10:17Z

Message from Michael Whatcott:

Patch Set 3:

(2 comments)

All feedback given to date has been incorporated. Thanks!

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

gopherbot · 2020-05-29T20:12:22Z

Message from Bryan C. Mills:

Patch Set 3:

(2 comments)

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

gopherbot · 2020-05-30T00:57:54Z

Message from Michael Whatcott:

Patch Set 3:

(2 comments)

If CL 229085 resolves the underlying issues in play (#38050 and #38458) than this CL can be abandoned. Thoughts?

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

mdwhatcott · 2020-06-11T17:23:51Z

@bcmills - I'm assuming that the work being done on https://go-review.googlesource.com/c/go/+/229085/ has made this CL/PR unnecessary. Is that true?

gopherbot · 2020-06-11T17:35:29Z

Message from Bryan C. Mills:

Patch Set 3:

Patch Set 3:

(2 comments)

If CL 229085 resolves the underlying issues in play (#38050 and #38458) than this CL can be abandoned. Thoughts?

I think there is still some value in having an integration test for test association in go test -json.

We should be able to revert the changes in test2json.go, and I'm kind of ambivalent about the new test in test2json/testdata, but I would really like to have the new test in cmd/go/testdata/script as a backstop against future regressions.

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

Only retain cmd/go/testdata/script/test_json_issue38050.txt

gopherbot · 2020-06-15T13:55:22Z

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

Please visit https://go-review.googlesource.com/c/go/+/226757 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-06-15T14:02:45Z

Message from Michael Whatcott:

Patch Set 4:

Patch Set 3:

Patch Set 3:

(2 comments)

If CL 229085 resolves the underlying issues in play (#38050 and #38458) than this CL can be abandoned. Thoughts?

I think there is still some value in having an integration test for test association in go test -json.

We should be able to revert the changes in test2json.go, and I'm kind of ambivalent about the new test in test2json/testdata, but I would really like to have the new test in cmd/go/testdata/script as a backstop against future regressions.

As requested, patch set 4 reverts the changes in test2json.go as well as the new tests in test2json/testdata. It retains the test script at cmd/go/testdata/script/test_json_issue38050.txt to prevent regressions in the future.

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

gopherbot · 2020-06-15T15:51:03Z

Message from Bryan C. Mills:

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

(1 comment)

Thanks, this looks great.
Let's plan to merge this when the tree opens for Go 1.16.

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

gopherbot · 2020-06-15T15:57:43Z

Message from Gobot Gobot:

Patch Set 4:

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

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

gopherbot · 2020-06-15T15:57:44Z

Message from Gobot Gobot:

Patch Set 4:

Build is still in progress...
This change failed on linux-amd64:
See https://storage.googleapis.com/go-build-log/3dcabfb1/linux-amd64_18ca093d.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/226757.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-06-15T16:02:19Z

Message from Gobot Gobot:

Patch Set 4: TryBot-Result-1

2 of 20 TryBots failed:
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/3dcabfb1/linux-amd64_18ca093d.log
Failed on js-wasm: https://storage.googleapis.com/go-build-log/3dcabfb1/js-wasm_e93cd39b.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/226757.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-06-16T02:05:11Z

Message from Bryan C. Mills:

Patch Set 4:

Patch Set 4: TryBot-Result-1

2 of 20 TryBots failed:
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/3dcabfb1/linux-amd64_18ca093d.log
Failed on js-wasm: https://storage.googleapis.com/go-build-log/3dcabfb1/js-wasm_e93cd39b.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.

You may need to rebase and/or re-check-out src/*.bat to fix the linux-amd64 failure.

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

gopherbot · 2020-06-16T03:30:26Z

Message from Michael Whatcott:

Patch Set 4:

Patch Set 4:

Patch Set 4: TryBot-Result-1

2 of 20 TryBots failed:
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/3dcabfb1/linux-amd64_18ca093d.log
Failed on js-wasm: https://storage.googleapis.com/go-build-log/3dcabfb1/js-wasm_e93cd39b.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.

You may need to rebase and/or re-check-out src/*.bat to fix the linux-amd64 failure.

By 'rebase', do you mean that I will need to rebase my master branch (github.com/mdwhatcott/go) with the latest master branch of the main repo (github.com/golang/go)?

I've done that locally, but git status reports that I "have 1061 and 5 commits respectively" so I want to be sure before pushing...

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

gopherbot · 2020-06-16T04:30:36Z

Message from Bryan C. Mills:

Patch Set 4:

Patch Set 4:

Patch Set 4:

Patch Set 4: TryBot-Result-1

2 of 20 TryBots failed:
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/3dcabfb1/linux-amd64_18ca093d.log
Failed on js-wasm: https://storage.googleapis.com/go-build-log/3dcabfb1/js-wasm_e93cd39b.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.

You may need to rebase and/or re-check-out src/*.bat to fix the linux-amd64 failure.

By 'rebase', do you mean that I will need to rebase my master branch (github.com/mdwhatcott/go) with the latest master branch of the main repo (github.com/golang/go)?

I've done that locally, but git status reports that I "have 1061 and 5 commits respectively" so I want to be sure before pushing...

I don't know about the GitHub PR workflow, but if you're using git-codereview¹ it would be a git codereview sync and then a git mail.

You ideally want to get to a state where you have strictly more commits than upstream. 😅

¹ https://pkg.go.dev/golang.org/x/review/git-codereview

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

mdwhatcott · 2020-06-16T04:52:12Z

This has turned into lots of overhead for a workaround solution that has now been reduced to a single regression test. I grant permission to anyone that is in a position to insert the test/script contained herein into the Go project.

gopherbot · 2020-06-16T04:52:26Z

Message from Michael Whatcott:

Patch Set 4:

Patch Set 4:

Patch Set 4:

Patch Set 4:

Patch Set 4: TryBot-Result-1

2 of 20 TryBots failed:
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/3dcabfb1/linux-amd64_18ca093d.log
Failed on js-wasm: https://storage.googleapis.com/go-build-log/3dcabfb1/js-wasm_e93cd39b.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.

You may need to rebase and/or re-check-out src/*.bat to fix the linux-amd64 failure.

By 'rebase', do you mean that I will need to rebase my master branch (github.com/mdwhatcott/go) with the latest master branch of the main repo (github.com/golang/go)?

I've done that locally, but git status reports that I "have 1061 and 5 commits respectively" so I want to be sure before pushing...

I don't know about the GitHub PR workflow, but if you're using git-codereview¹ it would be a git codereview sync and then a git mail.

You ideally want to get to a state where you have strictly more commits than upstream. 😅

¹ https://pkg.go.dev/golang.org/x/review/git-codereview

Ugh. This has turned into lots of overhead for a workaround solution that has now been reduced to a single regression test. I grant permission to anyone that is in a position to insert the test/script contained herein into the Go project. I'll close the originating PR (#38189).

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

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 Mar 31, 2020

mdwhatcott mentioned this pull request Apr 10, 2020

cmd/test2json: streamed log output not correctly associated with parallel tests #38050

Closed

mdwhatcott added 2 commits April 15, 2020 16:02

test2json: simplify map lookup and ensure line has expected prefix

ff18a56

Related to #38050

cmd/go/testdata/script: ensure attribution of json test log output

82da4c8

Related to #38050

mdwhatcott mentioned this pull request Apr 18, 2020

testing: streaming output loses parallel subtest associations #38458

Closed

cmd/go/testdata/script: more idiomatic subtests in json test case

5f65fa4

Related to the following issues: Fixes #38050 Fixes #38458

Revert test2json changes.

836bdc9

Only retain cmd/go/testdata/script/test_json_issue38050.txt

mdwhatcott closed this Jun 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test2json: associate streaming log output with the originating test #38189

test2json: associate streaming log output with the originating test #38189

mdwhatcott commented Mar 31, 2020

gopherbot commented Mar 31, 2020

gopherbot commented Mar 31, 2020

gopherbot commented Apr 15, 2020

gopherbot commented Apr 15, 2020

gopherbot commented Apr 16, 2020

gopherbot commented Apr 18, 2020

gopherbot commented Apr 18, 2020

gopherbot commented Apr 22, 2020

gopherbot commented Apr 22, 2020

gopherbot commented Apr 23, 2020

gopherbot commented Apr 23, 2020

gopherbot commented May 29, 2020

gopherbot commented May 30, 2020

mdwhatcott commented Jun 11, 2020

gopherbot commented Jun 11, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 16, 2020

gopherbot commented Jun 16, 2020

gopherbot commented Jun 16, 2020

mdwhatcott commented Jun 16, 2020

gopherbot commented Jun 16, 2020

test2json: associate streaming log output with the originating test #38189

test2json: associate streaming log output with the originating test #38189

Conversation

mdwhatcott commented Mar 31, 2020

gopherbot commented Mar 31, 2020

gopherbot commented Mar 31, 2020

gopherbot commented Apr 15, 2020

gopherbot commented Apr 15, 2020

gopherbot commented Apr 16, 2020

gopherbot commented Apr 18, 2020

gopherbot commented Apr 18, 2020

gopherbot commented Apr 22, 2020

gopherbot commented Apr 22, 2020

gopherbot commented Apr 23, 2020

gopherbot commented Apr 23, 2020

gopherbot commented May 29, 2020

gopherbot commented May 30, 2020

mdwhatcott commented Jun 11, 2020

gopherbot commented Jun 11, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 15, 2020

gopherbot commented Jun 16, 2020

gopherbot commented Jun 16, 2020

gopherbot commented Jun 16, 2020

mdwhatcott commented Jun 16, 2020

gopherbot commented Jun 16, 2020