From b79167293c53eb36c311fad71f2e242a8aec71d9 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 21 Mar 2025 15:04:30 +0200 Subject: [PATCH] chore(Makefile): update golden files as part of make gen (#17039) Updating golden files is an unnecessary extra step in addition to gen that is easily overlooked, leading to the developer noticing the issue in CI leading to lost developer time waiting for tests to complete. --- .github/workflows/ci.yaml | 11 +++----- Makefile | 28 +++++++++++++-------- cli/clitest/golden.go | 6 ++--- coderd/insights_test.go | 8 +++--- coderd/notifications/notifications_test.go | 2 +- enterprise/tailnet/pgcoord_internal_test.go | 6 ++--- provisioner/terraform/cleanup_test.go | 4 +-- tailnet/coordinator_internal_test.go | 6 ++--- 8 files changed, 37 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ee97e675cb..daa4670ea1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -267,18 +267,15 @@ jobs: popd - name: make gen - # no `-j` flag as `make` fails with: - # coderd/rbac/object_gen.go:1:1: syntax error: package statement must be first - run: "make --output-sync -B gen" - - - name: make update-golden-files run: | + # Remove golden files to detect discrepancy in generated files. make clean/golden-files # Notifications require DB, we could start a DB instance here but # let's just restore for now. git checkout -- coderd/notifications/testdata/rendered-templates - # As above, skip `-j` flag. - make --output-sync -B update-golden-files + # no `-j` flag as `make` fails with: + # coderd/rbac/object_gen.go:1:1: syntax error: package statement must be first + make --output-sync -B gen - name: Check for unstaged files run: ./scripts/check_unstaged.sh diff --git a/Makefile b/Makefile index 36b75098e3..2d2d02b5ab 100644 --- a/Makefile +++ b/Makefile @@ -568,12 +568,24 @@ GEN_FILES := \ agent/agentcontainers/dcspec/dcspec_gen.go # all gen targets should be added here and to gen/mark-fresh -gen: gen/db $(GEN_FILES) +gen: gen/db gen/golden-files $(GEN_FILES) .PHONY: gen gen/db: $(DB_GEN_FILES) .PHONY: gen/db +gen/golden-files: \ + cli/testdata/.gen-golden \ + coderd/.gen-golden \ + coderd/notifications/.gen-golden \ + enterprise/cli/testdata/.gen-golden \ + enterprise/tailnet/testdata/.gen-golden \ + helm/coder/tests/testdata/.gen-golden \ + helm/provisioner/tests/testdata/.gen-golden \ + provisioner/terraform/testdata/.gen-golden \ + tailnet/testdata/.gen-golden +.PHONY: gen/golden-files + # Mark all generated files as fresh so make thinks they're up-to-date. This is # used during releases so we don't run generation scripts. gen/mark-fresh: @@ -743,16 +755,10 @@ coderd/apidoc/swagger.json: node_modules/.installed site/node_modules/.installed cd site/ pnpm exec biome format --write ../docs/manifest.json ../coderd/apidoc/swagger.json -update-golden-files: \ - cli/testdata/.gen-golden \ - coderd/.gen-golden \ - coderd/notifications/.gen-golden \ - enterprise/cli/testdata/.gen-golden \ - enterprise/tailnet/testdata/.gen-golden \ - helm/coder/tests/testdata/.gen-golden \ - helm/provisioner/tests/testdata/.gen-golden \ - provisioner/terraform/testdata/.gen-golden \ - tailnet/testdata/.gen-golden +update-golden-files: + echo 'WARNING: This target is deprecated. Use "make gen/golden-files" instead.' 2>&1 + echo 'Running "make gen/golden-files"' 2>&1 + make gen/golden-files .PHONY: update-golden-files clean/golden-files: diff --git a/cli/clitest/golden.go b/cli/clitest/golden.go index 9d82f73f0c..e70e527b66 100644 --- a/cli/clitest/golden.go +++ b/cli/clitest/golden.go @@ -24,7 +24,7 @@ import ( // UpdateGoldenFiles indicates golden files should be updated. // To update the golden files: -// make update-golden-files +// make gen/golden-files var UpdateGoldenFiles = flag.Bool("update", false, "update .golden files") var timestampRegex = regexp.MustCompile(`(?i)\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d+)?(Z|[+-]\d+:\d+)`) @@ -113,12 +113,12 @@ func TestGoldenFile(t *testing.T, fileName string, actual []byte, replacements m } expected, err := os.ReadFile(goldenPath) - require.NoError(t, err, "read golden file, run \"make update-golden-files\" and commit the changes") + require.NoError(t, err, "read golden file, run \"make gen/golden-files\" and commit the changes") expected = normalizeGoldenFile(t, expected) require.Equal( t, string(expected), string(actual), - "golden file mismatch: %s, run \"make update-golden-files\", verify and commit the changes", + "golden file mismatch: %s, run \"make gen/golden-files\", verify and commit the changes", goldenPath, ) } diff --git a/coderd/insights_test.go b/coderd/insights_test.go index 53f70c66df..47a80df528 100644 --- a/coderd/insights_test.go +++ b/coderd/insights_test.go @@ -1295,7 +1295,7 @@ func TestTemplateInsights_Golden(t *testing.T) { } f, err := os.Open(goldenFile) - require.NoError(t, err, "open golden file, run \"make update-golden-files\" and commit the changes") + require.NoError(t, err, "open golden file, run \"make gen/golden-files\" and commit the changes") defer f.Close() var want codersdk.TemplateInsightsResponse err = json.NewDecoder(f).Decode(&want) @@ -1311,7 +1311,7 @@ func TestTemplateInsights_Golden(t *testing.T) { }), } // Use cmp.Diff here because it produces more readable diffs. - assert.Empty(t, cmp.Diff(want, report, cmpOpts...), "golden file mismatch (-want +got): %s, run \"make update-golden-files\", verify and commit the changes", goldenFile) + assert.Empty(t, cmp.Diff(want, report, cmpOpts...), "golden file mismatch (-want +got): %s, run \"make gen/golden-files\", verify and commit the changes", goldenFile) }) } }) @@ -2076,7 +2076,7 @@ func TestUserActivityInsights_Golden(t *testing.T) { } f, err := os.Open(goldenFile) - require.NoError(t, err, "open golden file, run \"make update-golden-files\" and commit the changes") + require.NoError(t, err, "open golden file, run \"make gen/golden-files\" and commit the changes") defer f.Close() var want codersdk.UserActivityInsightsResponse err = json.NewDecoder(f).Decode(&want) @@ -2092,7 +2092,7 @@ func TestUserActivityInsights_Golden(t *testing.T) { }), } // Use cmp.Diff here because it produces more readable diffs. - assert.Empty(t, cmp.Diff(want, report, cmpOpts...), "golden file mismatch (-want +got): %s, run \"make update-golden-files\", verify and commit the changes", goldenFile) + assert.Empty(t, cmp.Diff(want, report, cmpOpts...), "golden file mismatch (-want +got): %s, run \"make gen/golden-files\", verify and commit the changes", goldenFile) }) } }) diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index a823cb117e..d48394771f 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -768,7 +768,7 @@ func TestNotificationTemplates_Golden(t *testing.T) { hello = "localhost" from = "system@coder.com" - hint = "run \"DB=ci make update-golden-files\" and commit the changes" + hint = "run \"DB=ci make gen/golden-files\" and commit the changes" ) tests := []struct { diff --git a/enterprise/tailnet/pgcoord_internal_test.go b/enterprise/tailnet/pgcoord_internal_test.go index dc425c352a..2fed758d74 100644 --- a/enterprise/tailnet/pgcoord_internal_test.go +++ b/enterprise/tailnet/pgcoord_internal_test.go @@ -32,7 +32,7 @@ import ( // UpdateGoldenFiles indicates golden files should be updated. // To update the golden files: -// make update-golden-files +// make gen/golden-files var UpdateGoldenFiles = flag.Bool("update", false, "update .golden files") // TestHeartbeats_Cleanup tests the cleanup loop @@ -316,11 +316,11 @@ func TestDebugTemplate(t *testing.T) { } expected, err := os.ReadFile(goldenPath) - require.NoError(t, err, "read golden file, run \"make update-golden-files\" and commit the changes") + require.NoError(t, err, "read golden file, run \"make gen/golden-files\" and commit the changes") require.Equal( t, string(expected), string(actual), - "golden file mismatch: %s, run \"make update-golden-files\", verify and commit the changes", + "golden file mismatch: %s, run \"make gen/golden-files\", verify and commit the changes", goldenPath, ) } diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index 9fb15c1b13..7d4dd897d8 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -174,8 +174,8 @@ func diffFileSystem(t *testing.T, fs afero.Fs) { } want, err := os.ReadFile(goldenFile) - require.NoError(t, err, "open golden file, run \"make update-golden-files\" and commit the changes") - assert.Empty(t, cmp.Diff(want, actual), "golden file mismatch (-want +got): %s, run \"make update-golden-files\", verify and commit the changes", goldenFile) + require.NoError(t, err, "open golden file, run \"make gen/golden-files\" and commit the changes") + assert.Empty(t, cmp.Diff(want, actual), "golden file mismatch (-want +got): %s, run \"make gen/golden-files\", verify and commit the changes", goldenFile) } func dumpFileSystem(t *testing.T, fs afero.Fs) []byte { diff --git a/tailnet/coordinator_internal_test.go b/tailnet/coordinator_internal_test.go index 2344bf2723..9f5ac7c6a4 100644 --- a/tailnet/coordinator_internal_test.go +++ b/tailnet/coordinator_internal_test.go @@ -15,7 +15,7 @@ import ( // UpdateGoldenFiles indicates golden files should be updated. // To update the golden files: -// make update-golden-files +// make gen/golden-files var UpdateGoldenFiles = flag.Bool("update", false, "update .golden files") func TestDebugTemplate(t *testing.T) { @@ -64,11 +64,11 @@ func TestDebugTemplate(t *testing.T) { } expected, err := os.ReadFile(goldenPath) - require.NoError(t, err, "read golden file, run \"make update-golden-files\" and commit the changes") + require.NoError(t, err, "read golden file, run \"make gen/golden-files\" and commit the changes") require.Equal( t, string(expected), string(actual), - "golden file mismatch: %s, run \"make update-golden-files\", verify and commit the changes", + "golden file mismatch: %s, run \"make gen/golden-files\", verify and commit the changes", goldenPath, ) }