search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.41k stars 17.71k forks source link

doc: introduce a page for CI best practices #42119

Open mvdan opened 4 years ago

mvdan commented 4 years ago

Most CI scripts and defaults do just the basics: go test ./...

There are many more commands that likely make sense for a majority of projects, and some are well known, but we don't have a single place to document and maintain them well. For example:

And potentially staticcheck ./... too, since it's now sort of recommended by gopls as an extension to go vet. Though note that this should be a small set of recommendations that most projects should follow, so it shouldn't include an endless list of linters that might be interesting to some users.

We should include a ready-to-use script containing all of the recommended steps, for example in POSIX Shell. We could document each step directly via script comments, meaning that the entire thing could be copy-pasted and self-documenting.

I'm happy to kickstart this work during the freeze. /cc @bcmills @jayconrod @myitcv @jadekler

mvdan commented 4 years ago

The reason I want this to be a doc on the main site or the main repo is two-fold: so that it's code reviewed (so the wiki doesn't work), and so that it can be maintained and updated over time (so a blog post doesn't work).

bcmills commented 4 years ago

CC @stevetraut

mvdan commented 4 years ago

Friendly bump - who should I get the green light from for this document? I think the freeze is probably the best time to work on docs, so I'd like to start writing this doc before the end of the year.

If no green light is needed I can just start writing and send a CL, but I'd rather not do that if the decision will likely be "we don't want this doc in the website" :)

jayconrod commented 4 years ago

Our tech writer @stevetraut would be the best person to coordinate with. He's in the process of writing a programmer's guide to Go with many different topics. I think CI best practices would fit into that nicely.

stevetraut commented 4 years ago

Sorry for the late response, folks.

@mvdan Thanks for the idea! I think it could be useful content. I'm not sure it's within the "programming with Go" focus that the programmer's guide Jay mentioned has, but content aimed at team development would be useful. Currently, though, the guide is still taking shape and we're not sure yet what's in and what's out.

May I suggest that you add it to the wiki? That would get it in front of folks right away. And I'm sure I'll be collecting other content and ideas from the wiki along the way, and may add a topic on CI.

@rsc

mvdan commented 4 years ago

Thanks for the response. I honestly don't think the wiki is the best place, because it's not code-reviewed and anyone can edit it, so it's not a good place to document best practices. That is the reason why I opened https://github.com/golang/go/issues/34038 last year, though unfortunately the process is taking a long time. That's why I was hoping this new document could be added directly to a git repository or the main site.

I could always start writing it while we decide where it goes, but I still think it has relatively less value (and can't be properly maintained) if it ends up in the wiki.

rsc commented 4 years ago

@mvdan, you're right that the wiki isn't the right long-term place, and we agree. But we have limited attention, and right now this specific page is not at the top of our list. We don't even have the framework set up for where the page would properly go. So if you want to publish something now, the wiki is still the best place. Long term, we'll have a better answer.

mvdan commented 3 years ago

Use go mod tidy and ensure that there aren't changes to go.mod or go.sum (via go mod tidy -mod=readonly maybe?)

Unfortunately go mod tidy -mod=readonly doesn't work, so the current workarounds are either assuming git diff works, or using a bit of cp and diff. So I think https://github.com/golang/go/issues/27005 should be given some priority for the sake of CI best practices.

mvdan commented 3 years ago

Given that we'll get go mod tidy -check, gofmt will remain as the only command that needs extra machinery (e.g. shell code) to do the right thing here. So I've raised https://github.com/golang/go/issues/46289 as an attempt to reconsider some mode for gofmt to tell us, directly, if any files require changes.

mvdan commented 3 years ago

Another interesting point is 1.17's go test -shuffle. I think we should recommend its use in CI, similar to -race.

mvdan commented 3 years ago

Yet another idea, as a bonus: ensure that go test module-path/... works even when the module in question is not the main module. For example, using a temporary go.mod file that simply requires our module and replaces it to the right directory. This would ensure that the module works as a dependency too, i.e. without directives like replace or exclude.

myitcv commented 3 years ago

A question/suggestion (depending whether it's in scope or not): would this guide also provide guidance on how to work with multi-module repositories? In particular, dealing with the fact that go test ./... does not cross module boundaries, and therefore how to work with that?

mvdan commented 3 years ago

Probably, yes, though I think the first version should aim to tackle the common case: a single module.

Ideally all of this would be "best practices for testing a module in CI", but since most CI systems work at the VCS repository level, I think we'll need to bridge the gap a little with extra recommendations.

mvdan commented 2 years ago

Judging by Steve's Gerrit profile, it seems like he's left Google. Who is the next tech writer, or whomever I should coordinate these efforts with? I'd like to contribute documentation which is sorely needed in my opinion, like this thread and https://github.com/golang/go/issues/48539, and for the past few years I haven't been able to find anyone to help make that happen.

mvdan commented 1 year ago

Another one that pops to mind: using -trimpath in CI is probably a good idea as well. Full paths are usually not useful when looking at CI logs, because it's not a filesystem one can typically access directly. And using -trimpath helps with reproducible builds, which should help with keeping build and test caches more reusable between CI runs.

mvdan commented 1 year ago

Another one we figured out with @myitcv: go test -count=1 ./... can be a good way to ensure that all tests are run, without any test caching happening. This can help catch flaky tests quicker, for example if one test only fails 1% of the time.

We wanted a setup where pushes to master ran all the tests, but PRs/CLs being tested would reuse master's test cache to speed up iteration and testing on patches, just like we already did with the build and module caches. In this scenario, using go test -count=1 ./... in master is not quite right, because it disables the test cache entirely - test cache entries are not filled in at all. To ensure that go test ./... in master runs all the tests and fills the test cache, we're now using go clean -testcache && go test ./..., which works well.

mvdan commented 1 year ago

Yet another idea: another command that we want to ensure results in zero changes, like gofmt or go mod tidy, is go fix ./.... I was still carrying // +build lines in a handful of projects that had dropped support for Go 1.16 or older over a year ago.

mvdan commented 1 year ago

One more idea: for monorepos with checked in go.work files, one likely wants to ensure that:

eccles commented 9 months ago

great to know what good practice is. I usually run go mod tidy and go mod verify as part of any PR. Also what is the effect of not keeping go.mod up to date? I recently started working with a repo where the go.mod files did not reflect reality but this appeared to do no harm. Why and when cmds like go mod tidy/verify are executed would be useful

mvdan commented 7 months ago
  • When explicitly testing multiple Go versions via a matrix, set GOTOOLCHAIN=local to ensure old versions do not automatically download and use newer toolchains

I have just added this item, as @myitcv correctly pointed out that https://go.dev/doc/toolchain can be a rather surprising behavior in a CI environment where one explicitly wants to test a matrix of Go versions. If an old version is unexpectedly too old or would break, we want CI to fail and alert us of that, not to automatically download a newer toolchain and silently succeed.

mvdan commented 7 months ago

I also updated the original post about the newly proposed -check flags in go mod tidy and gofmt, which would remove the last places where any shell logic is needed.