Add WithUpdateEnv option to protect fixtures from blanket updates#219
Add WithUpdateEnv option to protect fixtures from blanket updates#219abiduke612 wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a configurable environment variable name for golden fixture updates so tests can opt out of blanket UPDATE-driven fixture rewrites.
Changes:
- Add
UpdateEnvtotestutil.CompareWithFixtureoptions, defaulting toUPDATE. - Introduce
WithUpdateEnv(...)option to override which env var triggers fixture updates.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
abiduke612
force-pushed
the
abbyduke/protect-golden-fixtures
branch
from
50d3c4f to
19e98e3
Compare
abiduke612
force-pushed
the
abbyduke/protect-golden-fixtures
branch
from
19e98e3 to
a8ee25d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| t.Fatalf("failed to get absolute path to testdata file: %v", err) | ||
| } | ||
| if os.Getenv("UPDATE") != "" { | ||
| if os.Getenv(options.UpdateEnv) != "" { |
There was a problem hiding this comment.
CompareWithFixture now checks os.Getenv(options.UpdateEnv) for updating fixtures, but the diff failure hint later in the function still instructs users to run with UPDATE=true .... This becomes misleading when WithUpdateEnv(...) is used; format the hint using the resolved options.UpdateEnv value (and consider using a non-empty example value like =1 since the check is != "").
| func WithUpdateEnv(env string) option { | ||
| return func(opts *options) { | ||
| opts.UpdateEnv = env | ||
| } | ||
| } |
There was a problem hiding this comment.
This PR introduces WithUpdateEnv, but the CompareWithFixture doc comment still says fixtures are updated by setting the UPDATE env var. Update the comment to reflect that the env var name is configurable (defaulting to UPDATE) so callers know to use WithUpdateEnv when needed.
CompareWithFixtureto use the resolvedupdateEnvvariable instead of hardcodedUPDATE=trueWithUpdateEnv