Skip to content

acceptance: exit cleanly from -update when only regenerating outputs #5424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
janniklasrose wants to merge 1 commit into
base: main
Choose a base branch
from
+22 −14
Draft

acceptance: exit cleanly from -update when only regenerating outputs #5424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Taskfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,8 +799,7 @@ tasks:
generates:
- acceptance/bundle/refschema/out.fields.txt
cmds:
- cmd: go test ./acceptance -run TestAccept/bundle/refschema -update &> /dev/null
ignore_error: true # -update returns non-zero exit code on changes
- go test ./acceptance -run TestAccept/bundle/refschema -update

generate-schema:
desc: Generate bundle JSON schema
Expand Down
33 changes: 21 additions & 12 deletions acceptance/acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,35 +899,44 @@ func doComparison(t *testing.T, repls testdiff.ReplacementsContext, dirRef, dirN
valueNew = repls.Replace(valueNew)
}

// The test did not produce an expected output file.
if okRef && !okNew {
t.Errorf("Missing output file: %s", relPath)
Copy link
Copy Markdown
Contributor

@denik denik Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would not removing t.Errorf be enough? (for cleaner diff)

if testdiff.OverwriteMode {
// In update mode, (re)generating the reference files is the goal, so applying
// a change is success, not failure. Write or remove the reference file to match
// what the test produced, then return without marking the test failed. Genuine
// problems still fail the run: read errors (reported by tryReading above) and
// write errors (via require/testutil below).
if testdiff.OverwriteMode {
switch {
case okRef && !okNew:
// The test no longer produces this output; drop the stale reference.
t.Logf("Removing output file: %s", relPath)
require.NoError(t, os.Remove(pathRef))
case !okRef && okNew:
Copy link
Copy Markdown
Contributor

@denik denik Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also handled below and produces error there

t.Logf("Writing output file: %s", relPath)
testutil.WriteFile(t, pathRef, valueNew)
case valueRef != valueNew:
t.Logf("Overwriting existing output file: %s", relPath)
testutil.WriteFile(t, pathRef, valueNew)
}
return
}

// The test did not produce an expected output file.
if okRef && !okNew {
t.Errorf("Missing output file: %s", relPath)
return
}

// The test produced an unexpected output file.
if !okRef && okNew {
t.Errorf("Unexpected output file: %s\npathRef: %s\npathNew: %s", relPath, pathRef, pathNew)
if shouldShowDiff(pathNew, valueNew) {
testdiff.AssertEqualTexts(t, pathRef, pathNew, valueRef, valueNew)
}
if testdiff.OverwriteMode {
t.Logf("Writing output file: %s", relPath)
testutil.WriteFile(t, pathRef, valueNew)
}
return
}

// Compare the reference and new values.
equal := testdiff.AssertEqualTexts(t, pathRef, pathNew, valueRef, valueNew)
if !equal && testdiff.OverwriteMode {
t.Logf("Overwriting existing output file: %s", relPath)
testutil.WriteFile(t, pathRef, valueNew)
}

if VerboseTest && !equal && printedRepls != nil && !*printedRepls {
*printedRepls = true
Expand Down
Loading