internal/ci: go generate after go test

Some packages like ./encoding/jsonschema use `go generate` to produce
testdata files, which is an entirely reasonable use case.
Because these files get overwritten every time, getting new modtimes,
this causes `go test ./...` to treat the test input files as changed,
and so CI never reuses a cached test run of the jsonschema package.

For this reason, as well as to avoid confusion with developers looking
at CI failures (which has happened twice already), test and check first,
and generate after. In the happy case, where code generation is clean,
this results in no change in behavior other than more test cache hits.
In the case where the user forgot to re-run `go generate`,
CI will still fail, just a little bit later, which is fine.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: I2635095eec950d392f5ccb6da2147e20f7c83584
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1201440
Reviewed-by: Roger Peppe <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

.github/workflows/trybot.yaml

@@ -110,9 +110,6 @@ jobs:
       - name: Early git and code sanity checks
         if: (matrix.go-version == '1.23.x' && matrix.runner == 'ubuntu-22.04')
         run: go run ./internal/ci/checks
-      - name: Generate
-        run: go generate ./...
-        if: (matrix.go-version == '1.23.x' && matrix.runner == 'ubuntu-22.04')
       - name: Test
         if: |-
           ((github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/heads/release-branch.')) && (! (contains(github.event.head_commit.message, '
@@ -169,6 +166,9 @@ jobs:
           	echo "Did you forget about refs/attic branches? https://github.com/cue-lang/cue/wiki/Notes-for-project-maintainers"
           	exit 1
           fi
+      - name: Generate
+        run: go generate ./...
+        if: (matrix.go-version == '1.23.x' && matrix.runner == 'ubuntu-22.04')
       - name: Check that git is clean at the end of the job
         if: always()
         run: test -z "$(git status --porcelain)" || (git status; git diff; false)

internal/ci/github/trybot.cue

@@ -67,7 +67,6 @@ workflows: trybot: _repo.bashWorkflow & {
 					// so we only need to run them on one of the matrix jobs.
 					if: _isLatestLinux
 				},
-				_goGenerate,
 				_goTest & {
 					if: "\(_repo.isProtectedBranch) || !\(_isLatestLinux)"
 				},
@@ -78,6 +77,12 @@ workflows: trybot: _repo.bashWorkflow & {
 				for v in _e2eTestSteps {v},
 				_goCheck,
 				_checkTags,
+				// Run code generation towards the very end, to ensure it succeeds and makes no changes.
+				// Note that doing this before any Go tests or checks may lead to test cache misses,
+				// as Go uses modtimes to approximate whether files have been modified.
+				// Moveover, Go test failures on CI due to changed generated code are very confusing
+				// as the user might not notice that checkGitClean is also failing towards the end.
+				_goGenerate,
 				_repo.checkGitClean,
 			]
 		}