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

Continuously ensure pprof tests pass in Golang builder environments #343

Closed
aalexand opened this issue Mar 23, 2018 · 7 comments
Closed
Assignees

Comments

@aalexand
Copy link
Collaborator

@mvdan @ALTree @hyangah

Over time we've had consistent stream of issues reported by Go team where updating the vendored copy of pprof breaks one or several tests: #342, #328, #300, #251, #146.

We should find a way to continuously ensure that pprof head tests pass in the Go builder environments. Thoughts on how:

  • Maybe Go team could have a continuous build which runs tests for the head of all vendored deps.
  • Or we could figure out how to use Gomote from Travis CI (or Kokoro) to run the tests.
@hyangah
Copy link
Contributor

hyangah commented Mar 23, 2018

@andybons @bradfitz @rsc

@mvdan
Copy link
Contributor

mvdan commented Mar 23, 2018

Is the problem that the tests don't run on all builders often enough, or that we have too many CL reverts due to these failures?

If the problem is the latter, perhaps we should simply have a way to mark CLs that often break the non-trybot builders. That way, they would be run on the entire set of builders before being merged.

@aalexand
Copy link
Collaborator Author

The problem is that pprof CI doesn't catch some issues (e.g. specific to OS like OpenBSD) at the time the changes are made and they are only caught when Go team attempts to vendor the newer version of pprof into Go source tree. From my perspective that creates unnecessary churn - I'd prefer if the issues like that would be caught shortly after the change is made. So I wonder if there is a relatively cheap way to do that.

@mvdan
Copy link
Contributor

mvdan commented Apr 3, 2018

I've sent an e-mail about this to golang-dev, so hopefully we'll have members of the Go team come forward with ideas: https://groups.google.com/forum/#!topic/golang-dev/TzPt6uUZwNM

@aalexand
Copy link
Collaborator Author

aalexand commented Apr 3, 2018

@mvdan Thanks!

@mvdan
Copy link
Contributor

mvdan commented Apr 6, 2018

@aalexand it seems like the consensus will be that, on our side, we will eventually get better at noticing when re-vendoring pprof would break any of the exotic builders. At least, it would be easier for us to notice before we actually merged the update into master.

I think that once that in place, and pprof is re-vendored regularly (say, once a month or so), these issues shouldn't go unnoticed for too long and it shouldn't be a big problem for us.

If you'd like something more continuous, such as either of the two ideas you initially proposed, I'd suggest reaching out to the Go team with a specific proposal. For example, "allow access to the Go builders to all projects vendored by Go".

@aalexand
Copy link
Collaborator Author

aalexand commented Apr 6, 2018

Le'ts say we are good for now. I hope that we've fixed most of the flakiness, so probably not worth it yet to invest further. We are trying to improve the testing on our side as well to match edge cases we learn from failing Go buildbots (e.g. the recent test failure when binutils are not installed on Mac), too.

@aalexand aalexand closed this as completed Apr 6, 2018
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

No branches or pull requests

4 participants