exercism / go-test-runner

GNU Affero General Public License v3.0
15 stars 17 forks source link

Make 1.20 fast #103

Closed andrerfcsantos closed 1 year ago

andrerfcsantos commented 1 year ago

Follow up of: https://forum.exercism.org/t/tests-failing-with-an-error-occurred/4616/11


After trying several things (which I included in the forum thread), removing the special user and the GOCACHE env var fixed the problem. Both these changes seem to be necessary, also tried one without the other, and the problem persists.

I'm still not sure why this fixes it. My best guess is these two bits of the 1.20 Release Notes:

The packages in the standard library that use cgo are net, os/user, and plugin. On macOS, the net and os/user packages have been rewritten not to use cgo: the same code is now used for cgo and non-cgo builds as well as cross-compiled builds. On Windows, the net and os/user packages have never used cgo. On other systems, builds with cgo disabled will use a pure Go version of these packages.

My guess here is that running the test runner as a special user inside the container makes Go call the os/user package. Setting CGO_ENABLED=1 to explicitly enable cgo also seems to do nothing in this regard.

(In module mode, compiled packages are stored in the build cache only, but a bug had caused the GOPATH install targets to unexpectedly remain in effect.)

We do compile the user solution in module mode, which uses the build cache set by GOCACHE, but it seems using /tmp causes slowness, and letting the default Go uses is best.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 4500226192


Totals Coverage Status
Change from base Build 4447602566: 0.6%
Covered Lines: 404
Relevant Lines: 622

๐Ÿ’› - Coveralls
iHiD commented 1 year ago

โค๏ธ ๐Ÿงก ๐Ÿ’› ๐Ÿ’š ๐Ÿ’™ ๐Ÿ’œ ๐Ÿ–ค

andrerfcsantos commented 1 year ago

It seems this is an issue with the build cache (see discussion in https://github.com/golang/go/issues/59146).

For that reason, I included a command to populate the build cache with the std lib and other packages related with go commands. This makes the initial build of the container a bit slower, but ensures the build cache is populated, meaning we should see more consistent builds.

I plan on merging tomorrow when I have more time to monitor the test runs.

ErikSchierboom commented 1 year ago

For that reason, I included a command to populate the build cache with the std lib and other packages related with go commands. This makes the initial build of the container a bit slower, but ensures the build cache is populated, meaning we should see more consistent builds.

We use the same approach in several other test runners.

ErikSchierboom commented 1 year ago

I tested use one of my Go solutions and that looked fine performance wise

andrerfcsantos commented 1 year ago

Same here. Tested in a variety of exercises and looks great.

Maybe even faster than before?

ErikSchierboom commented 1 year ago

๐Ÿš€