fix(config): resolve relative paths in config files against config dir, not cwd#901
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #901 +/- ##
=======================================
Coverage 90.66% 90.67%
=======================================
Files 264 264
Lines 15845 15859 +14
=======================================
+ Hits 14366 14380 +14
Misses 948 948
Partials 531 531
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…r, not cwd Closes #900. Path-valued config keys (err-ignore, warn-ignore, severity-levels, template) referenced from a config file are now resolved relative to the config file's directory, not the process's current working directory. Matches the convention established by ESLint, Prettier, golangci-lint, and similar tools — a config file is self-contained; relative paths in it refer to siblings of the config, not arbitrary files at cwd. Without this fix, --config <subdir>/.oasdiff.yaml silently fails for any path-valued flag because the CLI looks for those files at cwd instead of next to the config. Reproduced in #900. Implementation in internal/viper.go::resolveConfigRelativePaths, called after CLI flags are bound. Per key, the rewrite is skipped if: - The CLI flag was explicitly set by the user (their typed path means what they typed, relative to cwd). - The value is empty or already absolute. Skipped overall when no config file was loaded. Backward compatibility: strict. Existing setups continue to work identically: - Default-cwd-lookup users (.oasdiff.yaml or oasdiff.yaml in cwd): configDir == cwd, so relative paths resolve to <abs cwd>/<file>. Functionally identical to the previous cwd-relative behaviour. - Config-with-absolute-paths users: absolute paths are skipped by the rewrite, behaviour unchanged. - --config <path> users with relative paths in the config file: this case was broken before (the bug from #900) and is now fixed. The only "regression" surface is the theoretical user who relied on --config + cwd-relative-not-config-relative paths, which is a feature that's a few hours old and can't have meaningful adoption. Tests: 6 new cases in internal/viper_test.go cover relative path resolution against config dir, absolute paths unchanged, CLI flag override preserved, all four path keys handled, default lookup in cwd unchanged, and explicit back-compat for the legacy oasdiff.yaml filename in cwd. Plus a `template` field added to the Config struct so validate() recognizes the key. CONFIG-FILES.md documents the new resolution semantics with the convention citation.
028849f to
f9a6d69
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #900.
Summary
Path-valued config keys (
err-ignore,warn-ignore,severity-levels,template) referenced from a config file now resolve relative to the config file's directory, not the process's current working directory. A config file is self-contained: relative paths in it refer to siblings of the config, not arbitrary files at the process's cwd.Why
Without this fix, the workflow that #899 enables (
--config <path>andOASDIFF_CONFIG) silently breaks for path-valued flags as soon as the config file isn't in cwd. The bug as originally reported in #900:$ oasdiff breaking base.yaml revision.yaml --config examples/.oasdiff.yaml Error: can't process err ignore file: open ignore-err-example.txt: no such file or directoryThe config file referenced
ignore-err-example.txt, the file existed atexamples/ignore-err-example.txt, but the CLI looked at<cwd>/ignore-err-example.txt. After this fix, the file is found correctly.What's in the diff
internal/viper.go::resolveConfigRelativePaths— new function called fromRunViperafterbindFlags. For each key inconfigRelativePathKeys(the four path-valued keys), if the value is non-empty, relative, and not user-set on the CLI, it gets rewritten tofilepath.Join(configDir, value).Configstruct gains aTemplate stringfield sovalidate()'sUnmarshalExactrecognises the key. (Previously absent from the struct, which would have caused a "has invalid keys" error in any test that referenced it.)IViperinterface gainsConfigFileUsed,GetString,Set— all already onviper.Viper, so the test mock works unchanged.internal/viper_test.gocovering: relative path resolves against config dir; absolute paths unchanged; CLI flag overrides config and isn't rewritten; all four path keys handled; default cwd lookup unchanged; explicit back-compat for legacyoasdiff.yamlin cwd.docs/CONFIG-FILES.mddocuments the new resolution semantics.Backward compatibility
Strict. Existing setups continue to work identically:
.oasdiff.yamlin cwd, relative path<cwd>/<file><abs cwd>/<file>oasdiff.yamlin cwd, relative path (legacy users)<cwd>/<file><abs cwd>/<file>--err-ignore foo.txton CLI<cwd>/foo.txt<cwd>/foo.txt--config <subdir>/cfg.yaml, relative path insideThe only "regression" surface is the theoretical user who relied on
--config+ cwd-relative-not-config-relative paths.--configis a few hours old (#899 merged today); meaningful adoption hasn't happened.Test plan
go test ./...— all packages greengo test ./internal/ -run TestViper -count=1 -v— 26 tests pass (12 original + 8 from feat(config): rename default config to .oasdiff.*; add --config flag and OASDIFF_CONFIG env var #899 + 6 new)--config examples/.oasdiff.yamlfrom repo root) now succeedsoasdiff.yamlin cwd with relativeerr-ignorestill works