Skip to content

Commit b791672

Browse files
mafredrimafredri
authored
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.
1 parent de6080c commit b791672

File tree

8 files changed

+37
-34
lines changed

8 files changed

+37
-34
lines changed

‎.github/workflows/ci.yaml

+4-7
Original file line numberDiff line numberDiff line change
@@ -267,18 +267,15 @@ jobs:
267267
popd
268268
269269
- name: make gen
270-
# no `-j` flag as `make` fails with:
271-
# coderd/rbac/object_gen.go:1:1: syntax error: package statement must be first
272-
run: "make --output-sync -B gen"
273-
274-
- name: make update-golden-files
275270
run: |
271+
# Remove golden files to detect discrepancy in generated files.
276272
make clean/golden-files
277273
# Notifications require DB, we could start a DB instance here but
278274
# let's just restore for now.
279275
git checkout -- coderd/notifications/testdata/rendered-templates
280-
# As above, skip `-j` flag.
281-
make --output-sync -B update-golden-files
276+
# no `-j` flag as `make` fails with:
277+
# coderd/rbac/object_gen.go:1:1: syntax error: package statement must be first
278+
make --output-sync -B gen
282279
283280
- name: Check for unstaged files
284281
run: ./scripts/check_unstaged.sh

‎Makefile

+17-11
Original file line numberDiff line numberDiff line change
@@ -568,12 +568,24 @@ GEN_FILES := \
568568
agent/agentcontainers/dcspec/dcspec_gen.go
569569

570570
# all gen targets should be added here and to gen/mark-fresh
571-
gen: gen/db $(GEN_FILES)
571+
gen: gen/db gen/golden-files $(GEN_FILES)
572572
.PHONY: gen
573573

574574
gen/db: $(DB_GEN_FILES)
575575
.PHONY: gen/db
576576

577+
gen/golden-files: \
578+
cli/testdata/.gen-golden \
579+
coderd/.gen-golden \
580+
coderd/notifications/.gen-golden \
581+
enterprise/cli/testdata/.gen-golden \
582+
enterprise/tailnet/testdata/.gen-golden \
583+
helm/coder/tests/testdata/.gen-golden \
584+
helm/provisioner/tests/testdata/.gen-golden \
585+
provisioner/terraform/testdata/.gen-golden \
586+
tailnet/testdata/.gen-golden
587+
.PHONY: gen/golden-files
588+
577589
# Mark all generated files as fresh so make thinks they're up-to-date. This is
578590
# used during releases so we don't run generation scripts.
579591
gen/mark-fresh:
@@ -743,16 +755,10 @@ coderd/apidoc/swagger.json: node_modules/.installed site/node_modules/.installed
743755
cd site/
744756
pnpm exec biome format --write ../docs/manifest.json ../coderd/apidoc/swagger.json
745757

746-
update-golden-files: \
747-
cli/testdata/.gen-golden \
748-
coderd/.gen-golden \
749-
coderd/notifications/.gen-golden \
750-
enterprise/cli/testdata/.gen-golden \
751-
enterprise/tailnet/testdata/.gen-golden \
752-
helm/coder/tests/testdata/.gen-golden \
753-
helm/provisioner/tests/testdata/.gen-golden \
754-
provisioner/terraform/testdata/.gen-golden \
755-
tailnet/testdata/.gen-golden
758+
update-golden-files:
759+
echo 'WARNING: This target is deprecated. Use "make gen/golden-files" instead.' 2>&1
760+
echo 'Running "make gen/golden-files"' 2>&1
761+
make gen/golden-files
756762
.PHONY: update-golden-files
757763

758764
clean/golden-files:

‎cli/clitest/golden.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424

2525
// UpdateGoldenFiles indicates golden files should be updated.
2626
// To update the golden files:
27-
// make update-golden-files
27+
// make gen/golden-files
2828
var UpdateGoldenFiles = flag.Bool("update", false, "update .golden files")
2929

3030
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
113113
}
114114

115115
expected, err := os.ReadFile(goldenPath)
116-
require.NoError(t, err, "read golden file, run \"make update-golden-files\" and commit the changes")
116+
require.NoError(t, err, "read golden file, run \"make gen/golden-files\" and commit the changes")
117117

118118
expected = normalizeGoldenFile(t, expected)
119119
require.Equal(
120120
t, string(expected), string(actual),
121-
"golden file mismatch: %s, run \"make update-golden-files\", verify and commit the changes",
121+
"golden file mismatch: %s, run \"make gen/golden-files\", verify and commit the changes",
122122
goldenPath,
123123
)
124124
}

‎coderd/insights_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -1295,7 +1295,7 @@ func TestTemplateInsights_Golden(t *testing.T) {
12951295
}
12961296

12971297
f, err := os.Open(goldenFile)
1298-
require.NoError(t, err, "open golden file, run \"make update-golden-files\" and commit the changes")
1298+
require.NoError(t, err, "open golden file, run \"make gen/golden-files\" and commit the changes")
12991299
defer f.Close()
13001300
var want codersdk.TemplateInsightsResponse
13011301
err = json.NewDecoder(f).Decode(&want)
@@ -1311,7 +1311,7 @@ func TestTemplateInsights_Golden(t *testing.T) {
13111311
}),
13121312
}
13131313
// Use cmp.Diff here because it produces more readable diffs.
1314-
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)
1314+
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)
13151315
})
13161316
}
13171317
})
@@ -2076,7 +2076,7 @@ func TestUserActivityInsights_Golden(t *testing.T) {
20762076
}
20772077

20782078
f, err := os.Open(goldenFile)
2079-
require.NoError(t, err, "open golden file, run \"make update-golden-files\" and commit the changes")
2079+
require.NoError(t, err, "open golden file, run \"make gen/golden-files\" and commit the changes")
20802080
defer f.Close()
20812081
var want codersdk.UserActivityInsightsResponse
20822082
err = json.NewDecoder(f).Decode(&want)
@@ -2092,7 +2092,7 @@ func TestUserActivityInsights_Golden(t *testing.T) {
20922092
}),
20932093
}
20942094
// Use cmp.Diff here because it produces more readable diffs.
2095-
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)
2095+
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)
20962096
})
20972097
}
20982098
})

‎coderd/notifications/notifications_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,7 @@ func TestNotificationTemplates_Golden(t *testing.T) {
768768
hello = "localhost"
769769

770770
from = "system@coder.com"
771-
hint = "run \"DB=ci make update-golden-files\" and commit the changes"
771+
hint = "run \"DB=ci make gen/golden-files\" and commit the changes"
772772
)
773773

774774
tests := []struct {

‎enterprise/tailnet/pgcoord_internal_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import (
3232

3333
// UpdateGoldenFiles indicates golden files should be updated.
3434
// To update the golden files:
35-
// make update-golden-files
35+
// make gen/golden-files
3636
var UpdateGoldenFiles = flag.Bool("update", false, "update .golden files")
3737

3838
// TestHeartbeats_Cleanup tests the cleanup loop
@@ -316,11 +316,11 @@ func TestDebugTemplate(t *testing.T) {
316316
}
317317

318318
expected, err := os.ReadFile(goldenPath)
319-
require.NoError(t, err, "read golden file, run \"make update-golden-files\" and commit the changes")
319+
require.NoError(t, err, "read golden file, run \"make gen/golden-files\" and commit the changes")
320320

321321
require.Equal(
322322
t, string(expected), string(actual),
323-
"golden file mismatch: %s, run \"make update-golden-files\", verify and commit the changes",
323+
"golden file mismatch: %s, run \"make gen/golden-files\", verify and commit the changes",
324324
goldenPath,
325325
)
326326
}

‎provisioner/terraform/cleanup_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,8 @@ func diffFileSystem(t *testing.T, fs afero.Fs) {
174174
}
175175

176176
want, err := os.ReadFile(goldenFile)
177-
require.NoError(t, err, "open golden file, run \"make update-golden-files\" and commit the changes")
178-
assert.Empty(t, cmp.Diff(want, actual), "golden file mismatch (-want +got): %s, run \"make update-golden-files\", verify and commit the changes", goldenFile)
177+
require.NoError(t, err, "open golden file, run \"make gen/golden-files\" and commit the changes")
178+
assert.Empty(t, cmp.Diff(want, actual), "golden file mismatch (-want +got): %s, run \"make gen/golden-files\", verify and commit the changes", goldenFile)
179179
}
180180

181181
func dumpFileSystem(t *testing.T, fs afero.Fs) []byte {

‎tailnet/coordinator_internal_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515

1616
// UpdateGoldenFiles indicates golden files should be updated.
1717
// To update the golden files:
18-
// make update-golden-files
18+
// make gen/golden-files
1919
var UpdateGoldenFiles = flag.Bool("update", false, "update .golden files")
2020

2121
func TestDebugTemplate(t *testing.T) {
@@ -64,11 +64,11 @@ func TestDebugTemplate(t *testing.T) {
6464
}
6565

6666
expected, err := os.ReadFile(goldenPath)
67-
require.NoError(t, err, "read golden file, run \"make update-golden-files\" and commit the changes")
67+
require.NoError(t, err, "read golden file, run \"make gen/golden-files\" and commit the changes")
6868

6969
require.Equal(
7070
t, string(expected), string(actual),
71-
"golden file mismatch: %s, run \"make update-golden-files\", verify and commit the changes",
71+
"golden file mismatch: %s, run \"make gen/golden-files\", verify and commit the changes",
7272
goldenPath,
7373
)
7474
}

0 commit comments

Comments
 (0)