-
-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Labels
Description
Problem
The config file discovery has a specific precedence order but no tests verify this behavior works correctly.
Current Precedence Order
From highest to lowest priority:
- File specified with
-configflag ./logwrap.yamlor./logwrap.yml(current directory)~/.config/logwrap/config.yaml~/.logwrap.yaml
Missing Tests
No tests verify:
- Correct file is loaded when multiple exist
- Higher priority files override lower priority
- Fallback works when higher priority files don't exist
- Error when specified config doesn't exist vs. graceful fallback
Proposed Tests
// pkg/config/precedence_test.go
func TestConfigPrecedence(t *testing.T) {
// Create temp directory structure
tmpHome := t.TempDir()
tmpCwd := filepath.Join(tmpHome, "project")
os.MkdirAll(tmpCwd, 0755)
os.MkdirAll(filepath.Join(tmpHome, ".config", "logwrap"), 0755)
// Create config files with different settings
configs := map[string]string{
filepath.Join(tmpCwd, "logwrap.yaml"): "output:\n format: json",
filepath.Join(tmpHome, ".config", "logwrap", "config.yaml"): "output:\n format: text",
filepath.Join(tmpHome, ".logwrap.yaml"): "output:\n format: structured",
}
for path, content := range configs {
err := os.WriteFile(path, []byte(content), 0644)
require.NoError(t, err)
}
// Test precedence
tests := []struct {
name string
cwd string
explicitConfig string
existingFiles []string
expectedFormat string
}{
{
name: "Explicit config has highest priority",
cwd: tmpCwd,
explicitConfig: filepath.Join(tmpHome, ".logwrap.yaml"),
existingFiles: []string{"./logwrap.yaml", "~/.config/logwrap/config.yaml", "~/.logwrap.yaml"},
expectedFormat: "structured", // From ~/.logwrap.yaml
},
{
name: "Current directory overrides home configs",
cwd: tmpCwd,
existingFiles: []string{"./logwrap.yaml", "~/.config/logwrap/config.yaml", "~/.logwrap.yaml"},
expectedFormat: "json", // From ./logwrap.yaml
},
{
name: "XDG config directory before home dotfile",
cwd: tmpHome, // No local config
existingFiles: []string{"~/.config/logwrap/config.yaml", "~/.logwrap.yaml"},
expectedFormat: "text", // From ~/.config/logwrap/config.yaml
},
{
name: "Fallback to home dotfile",
cwd: tmpHome,
existingFiles: []string{"~/.logwrap.yaml"},
expectedFormat: "structured", // From ~/.logwrap.yaml
},
{
name: "Use defaults when no config found",
cwd: tmpHome,
existingFiles: []string{},
expectedFormat: "text", // Default
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Set up environment
os.Chdir(tt.cwd)
os.Setenv("HOME", tmpHome)
// Load config
var cfg *Config
var err error
if tt.explicitConfig != "" {
cfg, err = Load(tt.explicitConfig)
} else {
cfg, err = LoadDefault()
}
require.NoError(t, err)
assert.Equal(t, tt.expectedFormat, cfg.Output.Format)
})
}
}
func TestConfigExplicitNotFound(t *testing.T) {
// Explicit config must exist (no fallback)
_, err := Load("/nonexistent/config.yaml")
assert.Error(t, err)
assert.Contains(t, err.Error(), "no such file or directory")
}
func TestConfigYamlVsYml(t *testing.T) {
tmpDir := t.TempDir()
// Both .yaml and .yml exist - .yaml has priority
yamlPath := filepath.Join(tmpDir, "logwrap.yaml")
ymlPath := filepath.Join(tmpDir, "logwrap.yml")
os.WriteFile(yamlPath, []byte("output:\n format: json"), 0644)
os.WriteFile(ymlPath, []byte("output:\n format: text"), 0644)
os.Chdir(tmpDir)
cfg, err := LoadDefault()
require.NoError(t, err)
assert.Equal(t, "json", cfg.Output.Format, ".yaml should have priority over .yml")
}
func TestConfigMerging(t *testing.T) {
// Test that configs merge correctly (not tested elsewhere)
tmpHome := t.TempDir()
tmpCwd := filepath.Join(tmpHome, "project")
os.MkdirAll(tmpCwd, 0755)
// Home config has some settings
homeConfig := `
output:
format: json
log_level:
default: ERROR
`
os.WriteFile(filepath.Join(tmpHome, ".logwrap.yaml"), []byte(homeConfig), 0644)
// Local config overrides format but not level
localConfig := `
output:
format: text
`
os.WriteFile(filepath.Join(tmpCwd, "logwrap.yaml"), []byte(localConfig), 0644)
os.Chdir(tmpCwd)
os.Setenv("HOME", tmpHome)
cfg, err := LoadDefault()
require.NoError(t, err)
// Local config should override format
assert.Equal(t, "text", cfg.Output.Format)
// But home config's level should still apply
// (or does local config completely replace? Need to clarify behavior)
// assert.Equal(t, "ERROR", cfg.LogLevel.Default)
}Documentation Needed
Document the precedence order clearly:
## Configuration File Discovery
LogWrap searches for configuration files in this order (highest priority first):
1. **Explicit path** via `-config` flag
```bash
logwrap -config /path/to/config.yaml -- command- Error if file doesn't exist
-
Current directory
./logwrap.yamlor./logwrap.yml- Checked when running logwrap from project directory
.yamlextension has priority over.yml
-
XDG config directory
~/.config/logwrap/config.yaml- Follows XDG Base Directory specification
- User-specific configuration
-
Home dotfile
~/.logwrap.yaml- Traditional Unix dotfile location
- Fallback if XDG config not found
-
Built-in defaults
- Used when no config file found
- See example-config.yaml for defaults
Config Merging
Currently, only one config file is loaded (highest priority wins).
Future: Consider merging configs (lower priority provides defaults)?
### Edge Cases to Test
```go
func TestConfigEdgeCases(t *testing.T) {
tests := []struct {
name string
setup func() string
wantErr bool
}{
{
name: "Symlink to config",
setup: func() string {
tmpDir := t.TempDir()
configPath := filepath.Join(tmpDir, "real-config.yaml")
symlinkPath := filepath.Join(tmpDir, "logwrap.yaml")
os.WriteFile(configPath, []byte("output:\n format: json"), 0644)
os.Symlink(configPath, symlinkPath)
return tmpDir
},
wantErr: false,
},
{
name: "Config with no read permissions",
setup: func() string {
tmpDir := t.TempDir()
configPath := filepath.Join(tmpDir, "logwrap.yaml")
os.WriteFile(configPath, []byte("output:\n format: json"), 0000)
return tmpDir
},
wantErr: true,
},
{
name: "Config is directory not file",
setup: func() string {
tmpDir := t.TempDir()
os.MkdirAll(filepath.Join(tmpDir, "logwrap.yaml"), 0755)
return tmpDir
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cwd := tt.setup()
os.Chdir(cwd)
_, err := LoadDefault()
if tt.wantErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
})
}
}
Implementation Checklist
- Add precedence_test.go
- Test explicit config priority
- Test current directory priority
- Test XDG config priority
- Test home dotfile priority
- Test defaults when no config found
- Test .yaml vs .yml priority
- Test error when explicit config not found
- Test edge cases (symlinks, permissions)
- Document precedence in README
- Add example showing precedence
Benefits
Confidence:
- Verify precedence works as documented
- Catch regressions in config loading
- Test edge cases
Documentation:
- Tests serve as examples
- Clear expected behavior
- Easier to explain to users
Related Issues
- Add validation tests for malformed configuration #18 - Validation tests (related test improvements)
- Document configuration validation strategy and improve clarity #19 - Validation documentation (document precedence)
References
- XDG Base Directory: https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
- Go filepath: https://pkg.go.dev/path/filepath
Reactions are currently unavailable