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

x/build/cmd/coordinator: don't stop short when testing packages on release branches #36089

Closed
dmitshur opened this issue Dec 11, 2019 · 4 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

Right now, on release branches, testing is stopped very soon after encountering the first package with a failing test. E.g., from https://build.golang.org/log/b002647c8a5f7c73883916c5327af69cfe44dd3d:

[...]
FAIL	cmd/go/internal/modfetch	17.181s
ok  	cmd/go/internal/modfetch/codehost	5.474s
FAIL
2019/11/07 02:35:34 Failed: exit status 1

Ideally we should not have failing tests on release branches for a long amount of time, but that is not the current state. Until we reach that state, it might be helpful to modify coordinator so that it keeps testing all packages even if one fails, and report the test results in the log. That way, a package that fails early doesn't make it completely impossible to see if other package tests pass or fail.

/cc @bcmills @bradfitz @cagedmantis @toothrot

@dmitshur dmitshur added Builders x/build issues (builders, bots, dashboards) NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Dec 11, 2019
@dmitshur dmitshur added this to the Unreleased milestone Dec 11, 2019
@bcmills
Copy link
Contributor

bcmills commented Dec 11, 2019

This would also be handy for non-release branches: it would at least somewhat mitigate the problem of known test failures masking other regressions.

@dmitshur
Copy link
Contributor Author

dmitshur commented Dec 11, 2019

I limited the scope of this issue to release branches because I thought they would have a much higher benefit-to-cost ratio than other branches. If it goes well, we can consider expanding this to other branches too (but let's discuss it more first).

People seem in favor so I'll move forward to NeedsFix, but really the next step is to investigate this and determine how big or small of a change it'll take.

@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 Dec 11, 2019
@dmitshur
Copy link
Contributor Author

I'm seeing a relevant -k flag in go tool dist test:

$ go tool dist test -help
usage: go tool dist test [options]
[...]
  -k	keep going even when error occurred
[...]

If that's it, then this is a matter of arranging for that flag to be set to an appropriate value in cmd/coordinator when on testing a release branch.

@bradfitz
Copy link
Contributor

Dup of #14305

@golang golang locked and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants