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.

Sign up for GitHub

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

Jump to bottom

x/build: keep test result logs available after success #34119

Open
dmitshur opened this issue Sep 5, 2019 · 9 comments
Open

x/build: keep test result logs available after success #34119

dmitshur opened this issue Sep 5, 2019 · 9 comments
Labels
Builders x/build issues (builders, bots, dashboards) Community FeatureRequest Issues asking for a new feature that does not need a proposal. NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Unreleased

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Sep 5, 2019 •
edited
Loading

Currently, the x/build testing infrastructure (both trybots and post-submit builders) generally makes test logs available under two conditions:

  • while the test is running, streamed
  • after the test completed, if it failed

After tests complete successfully, the test logs immediately become inaccessible.

I don't know the exact rationale for this design, but my guess is that it done to reduce computational and storage resource use. It was likely deemed logs are less useful if tests passed. /cc @bradfitz Is there more context on this?

Feature Request

This feature request issue is to make them accessible on successful test completion for some amount of time (at least a month). Both on the build dashboard, and for trybot results.

The reasons for wanting this include:

  • it's sometimes helpful to be able to know which tests ran and which tests were skipped, and to be able to confirm a specific test truly passed
  • it's sometimes helpful to know which architectures were tested by trybots (and from that, to know which ones weren't), and having trybot result page with logs makes that much easier
  • pretty much every CI system keeps test logs accessible regardless if tests succeeded or failed, so it has become a baseline expectation

I think this should be worth doing in order to improve the developer experience for people working on the Go project. Feedback from others is welcome.

/cc @golang/osp-team
@dmitshur dmitshur added Testing An issue that has been verified to require only test changes, not just a test failure. Builders x/build issues (builders, bots, dashboards) FeatureRequest Issues asking for a new feature that does not need a proposal. Community labels Sep 5, 2019
@dmitshur dmitshur added this to the Unreleased milestone Sep 5, 2019
@dmitshur dmitshur added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 5, 2019
@toothrot
Copy link
Contributor

toothrot commented Sep 5, 2019 •
edited by dmitshur
Loading

This sounds like it could be related to #13076 and #10313.

Edit by @dmitshur: Another related issue is #12669.

@bradfitz
Copy link
Contributor

bradfitz commented Sep 5, 2019

Agreed. Let's just keep them forever. They're small. We already keep the built artifacts after make.bash forever and they're way bigger.

History is because build.golang.org only had links for failures.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Sep 5, 2019
This was referenced Sep 5, 2019
Closed
Closed
@dmitshur
Copy link
Contributor Author

dmitshur commented Sep 5, 2019 •
edited
Loading

@toothrot Thanks for finding those issues. They support the decision to do this.

Both of those issues are from 2015 and talk about a subset of this problem, so I folded them into this higher level tracking issue. I think it'll be better for us to open new smaller issues as needed.

@bradfitz Thanks for providing background on this. @andybons has expressed his support for this too, so I think we're in favor of doing this.

@dmitshur dmitshur mentioned this issue Sep 17, 2019
Closed
@andybons andybons self-assigned this Oct 1, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/198197 mentions this issue: cmd/coordinator: preserve test logs even if the run fails

Skarlso pushed a commit to Skarlso/build that referenced this issue Oct 2, 2019
@andybons
cmd/coordinator: preserve test logs even if the run fails
6ce37fe 
This includes trybot runs and post-commit build runs.

Fixes golang/go#34119

Change-Id: I1d078632048c473c09a1b8581d16fd1bd65a1305
Reviewed-on: https://go-review.googlesource.com/c/build/+/198197
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@andybons
Copy link
Member

andybons commented Oct 4, 2019

Unless I’m missing something, the CL above fixes this issue. Should have marked it as Fixes instead of Updates.

@andybons andybons closed this as completed Oct 4, 2019
@dmitshur
Copy link
Contributor Author

dmitshur commented Oct 7, 2019

I think coordinator still needs to be deployed with the change; re-opening to track that.

@dmitshur dmitshur reopened this Oct 7, 2019
@dmitshur
Copy link
Contributor Author

dmitshur commented Oct 31, 2019 •
edited
Loading

CL 198197 has made progress on this, but there's more to do to resolve this issue.

Consider a recent CL that ran trybots successfully:

https://go-review.googlesource.com/c/tools/+/204203/3#message-405874684174221493c77cbc2c783a2ac967f80e

The first Gerrit message says:

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

That link points to a page that says:

TryBot result not found (already done, invalid, or not yet discovered from Gerrit). Check Gerrit for results.

The second Gerrit message says:

TryBots are happy.

So it's not yet possible to access test logs when a trybot run completes successfully.

Similarly, at https://build.golang.org, there are many squares with "ok", but none of them are links, so it's not possible to access test logs for successful post-submit builds.

codebien pushed a commit to codebien/build that referenced this issue Nov 13, 2019
@andybons @codebien
cmd/coordinator: preserve test logs even if the run fails
e5c1faa 
This includes trybot runs and post-commit build runs.

Fixes golang/go#34119

Change-Id: I1d078632048c473c09a1b8581d16fd1bd65a1305
Reviewed-on: https://go-review.googlesource.com/c/build/+/198197
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@andybons andybons removed their assignment Nov 18, 2019
@dmitshur dmitshur mentioned this issue Dec 17, 2019
Open
@dmitshur dmitshur self-assigned this Aug 13, 2020
@dmitshur dmitshur mentioned this issue Aug 13, 2020
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/340435 mentions this issue: cmd/coordinator: link "ok" results to build logs

gopherbot pushed a commit to golang/build that referenced this issue Aug 10, 2021
@dmitshur
cmd/coordinator: link "ok" results to build logs
2cf5c44 
Every now and then, it's highly desirable to be able to view build
logs for a build even if it was successful. Those logs are kept as
of CL 198197, but they just haven't been linked. At long last, add
those links to the build dashboard(s)! 🎉

Keep the visual style of "ok" cells the same, aside that the mouse
cursor becomes a hand with an index finger, rather than a text caret.
This means this feature isn't very discoverable, but we can address
that in the future. (I've experimented with giving the "ok" links
an underline, but even a faint underline quickly leads to a large
amount of overwhelming visual noise when there are hundreds of "ok"
cells on many pages.)

Apply the same change to the work-in-progress dashboard V2 page,
to keep it in sync. (There are additional CSS changes to make it
look more consistent with the current build.golang.org dashboard.)

For golang/go#34119.
Updates golang/go#34744.

Change-Id: I27530e4103bbfd10e8cf09ae1a1cfce06c9bd748
Reviewed-on: https://go-review.googlesource.com/c/build/+/340435
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
@dmitshur
Copy link
Contributor Author

Similarly, at https://build.golang.org, there are many squares with "ok", but none of them are links, so it's not possible to access test logs for successful post-submit builds.

This part is resolved via CL 340435 now.

All that should be left in this issue is an ability to access test logs when a trybot run completes successfully.

@dmitshur dmitshur removed their assignment Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) Community FeatureRequest Issues asking for a new feature that does not need a proposal. NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@bradfitz @toothrot @andybons @dmitshur @gopherbot