Skip to content

Add test coverage for daemon configuration loader#3

Merged
rmax merged 3 commits into
codex/continue-ongoing-taskfrom
copilot/sub-pr-1
Feb 5, 2026
Merged

Add test coverage for daemon configuration loader#3
rmax merged 3 commits into
codex/continue-ongoing-taskfrom
copilot/sub-pr-1

Conversation

Copilot AI commented Feb 4, 2026

Copy link
Copy Markdown

The config loader in cmd/ratelord-d/config.go lacked test coverage despite other packages having comprehensive tests.

Changes

  • Test coverage for LoadConfig: Environment variable parsing, flag precedence, validation errors, path resolution
  • Error case validation: Invalid poll intervals, empty addresses, missing web-dir when web-assets=fs, invalid web-assets modes
  • Helper function tests: resolvePath, normalizeWebAssetsMode, envOrDefaultWithFallback, addrFromEnv
  • Environment variable fallback behavior: Tests for RATELORD_CONFIG_PATHRATELORD_POLICY_PATH and RATELORD_PORTRATELORD_ADDR precedence

Coverage

  • LoadConfig: 87%
  • Helper functions: 100%
  • Overall package: 35.8%

Example test pattern:

func TestLoadConfig_FlagOverridesEnv(t *testing.T) {
    os.Setenv("RATELORD_ADDR", "env-addr:8080")
    defer os.Unsetenv("RATELORD_ADDR")
    
    config, err := LoadConfig([]string{"-addr", "flag-addr:9090"})
    if err != nil {
        t.Fatalf("LoadConfig failed: %v", err)
    }
    
    if config.Addr != "flag-addr:9090" {
        t.Errorf("expected flag to override env for addr, got %s", config.Addr)
    }
}

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits February 4, 2026 21:48
- Test default values and path resolution
- Test env var parsing and precedence
- Test flag overrides env vars
- Test validation errors (invalid poll intervals, empty addr, missing web-dir)
- Test web-assets mode validation and normalization
- Test env var fallbacks (RATELORD_CONFIG_PATH, RATELORD_PORT)
- Test helper functions (resolvePath, normalizeWebAssetsMode)

Addresses PR feedback requesting test coverage for config.go

Co-authored-by: rmax <26015+rmax@users.noreply.github.com>
- Add proper error checking for os.Getwd() and os.Chdir()
- Fix variable declaration issues in error test cases
- All tests still pass

Co-authored-by: rmax <26015+rmax@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback on daemon configuration loader implementation Add test coverage for daemon configuration loader Feb 4, 2026
Copilot AI requested a review from rmax February 4, 2026 21:51
@rmax rmax marked this pull request as ready for review February 5, 2026 07:59
Copilot AI review requested due to automatic review settings February 5, 2026 07:59
@rmax rmax merged commit cd31c22 into codex/continue-ongoing-task Feb 5, 2026
3 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c935d94038

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +402 to +406
// Test that RATELORD_CONFIG_PATH is used as fallback for RATELORD_POLICY_PATH
os.Setenv("RATELORD_CONFIG_PATH", "/fallback/config.json")
defer os.Unsetenv("RATELORD_CONFIG_PATH")

config, err := LoadConfig([]string{})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Unset RATELORD_POLICY_PATH before fallback test

This test assumes RATELORD_POLICY_PATH is unset so that RATELORD_CONFIG_PATH acts as the fallback, but LoadConfig checks RATELORD_POLICY_PATH first. If a developer/CI environment already sets RATELORD_POLICY_PATH, the test will read that value and fail even though the code is correct. Consider explicitly clearing it (e.g., t.Setenv("RATELORD_POLICY_PATH", "") or os.Unsetenv) to make the test deterministic.

Useful? React with 👍 / 👎.

Comment on lines +348 to +352
os.Setenv("RATELORD_PORT", "3000")
defer os.Unsetenv("RATELORD_PORT")

config, err := LoadConfig([]string{})
if err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear RATELORD_ADDR in EnvPortOnly test

addrFromEnv prioritizes RATELORD_ADDR over RATELORD_PORT. This test only sets RATELORD_PORT, so if RATELORD_ADDR is already defined in the environment, the returned address will come from RATELORD_ADDR and the test will fail. Clearing or overriding RATELORD_ADDR in the test would avoid environment-dependent failures.

Useful? React with 👍 / 👎.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds comprehensive test coverage for the daemon configuration loader in cmd/ratelord-d/config.go, addressing a gap where this package lacked tests while other packages had them.

Changes:

  • Added 21 test functions covering environment variable parsing, flag precedence, validation errors, and path resolution
  • Implemented table-driven tests for helper functions normalizeWebAssetsMode and resolvePath
  • Verified error handling for invalid configurations including poll intervals, addresses, and web assets modes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +11 to +567
func TestLoadConfig_Defaults(t *testing.T) {
// Setup: Create temp dir and cd into it
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

// Verify defaults
if config.Addr != defaultAddr {
t.Errorf("expected default addr %s, got %s", defaultAddr, config.Addr)
}
if config.PollInterval != defaultPollInterval {
t.Errorf("expected default poll interval %v, got %v", defaultPollInterval, config.PollInterval)
}
if config.WebAssetsMode != defaultWebAssetsMode {
t.Errorf("expected default web assets mode %s, got %s", defaultWebAssetsMode, config.WebAssetsMode)
}
if !strings.HasSuffix(config.DBPath, "ratelord.db") {
t.Errorf("expected DBPath to end with ratelord.db, got %s", config.DBPath)
}
if !strings.HasSuffix(config.PolicyPath, "policy.json") {
t.Errorf("expected PolicyPath to end with policy.json, got %s", config.PolicyPath)
}
}

func TestLoadConfig_EnvVars(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Set env vars
os.Setenv("RATELORD_DB_PATH", "/custom/db.db")
os.Setenv("RATELORD_POLICY_PATH", "/custom/policy.json")
os.Setenv("RATELORD_ADDR", "0.0.0.0:9000")
os.Setenv("RATELORD_POLL_INTERVAL", "30s")
os.Setenv("RATELORD_WEB_ASSETS_MODE", "off")
defer func() {
os.Unsetenv("RATELORD_DB_PATH")
os.Unsetenv("RATELORD_POLICY_PATH")
os.Unsetenv("RATELORD_ADDR")
os.Unsetenv("RATELORD_POLL_INTERVAL")
os.Unsetenv("RATELORD_WEB_ASSETS_MODE")
}()

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.DBPath != "/custom/db.db" {
t.Errorf("expected DBPath from env /custom/db.db, got %s", config.DBPath)
}
if config.PolicyPath != "/custom/policy.json" {
t.Errorf("expected PolicyPath from env /custom/policy.json, got %s", config.PolicyPath)
}
if config.Addr != "0.0.0.0:9000" {
t.Errorf("expected Addr from env 0.0.0.0:9000, got %s", config.Addr)
}
if config.PollInterval != 30*time.Second {
t.Errorf("expected PollInterval from env 30s, got %v", config.PollInterval)
}
if config.WebAssetsMode != "off" {
t.Errorf("expected WebAssetsMode from env off, got %s", config.WebAssetsMode)
}
}

func TestLoadConfig_FlagOverridesEnv(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Set env vars
os.Setenv("RATELORD_ADDR", "env-addr:8080")
os.Setenv("RATELORD_POLL_INTERVAL", "15s")
defer func() {
os.Unsetenv("RATELORD_ADDR")
os.Unsetenv("RATELORD_POLL_INTERVAL")
}()

// Flags should override env
config, err := LoadConfig([]string{
"-addr", "flag-addr:9090",
"-poll-interval", "25s",
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.Addr != "flag-addr:9090" {
t.Errorf("expected flag to override env for addr, got %s", config.Addr)
}
if config.PollInterval != 25*time.Second {
t.Errorf("expected flag to override env for poll-interval, got %v", config.PollInterval)
}
}

func TestLoadConfig_InvalidPollInterval_Env(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

os.Setenv("RATELORD_POLL_INTERVAL", "invalid")
defer os.Unsetenv("RATELORD_POLL_INTERVAL")

_, err = LoadConfig([]string{})
if err == nil {
t.Fatal("expected error for invalid RATELORD_POLL_INTERVAL, got nil")
}
if !strings.Contains(err.Error(), "invalid RATELORD_POLL_INTERVAL") {
t.Errorf("expected error message to contain 'invalid RATELORD_POLL_INTERVAL', got: %v", err)
}
}

func TestLoadConfig_InvalidPollInterval_Flag(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-poll-interval", "not-a-duration"})
if err == nil {
t.Fatal("expected error for invalid poll-interval flag, got nil")
}
if !strings.Contains(err.Error(), "invalid poll interval") {
t.Errorf("expected error message to contain 'invalid poll interval', got: %v", err)
}
}

func TestLoadConfig_EmptyAddr(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-addr", ""})
if err == nil {
t.Fatal("expected error for empty addr, got nil")
}
if !strings.Contains(err.Error(), "addr cannot be empty") {
t.Errorf("expected error about empty addr, got: %v", err)
}
}

func TestLoadConfig_EmptyAddr_Whitespace(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-addr", " "})
if err == nil {
t.Fatal("expected error for whitespace-only addr, got nil")
}
if !strings.Contains(err.Error(), "addr cannot be empty") {
t.Errorf("expected error about empty addr, got: %v", err)
}
}

func TestLoadConfig_WebAssets_FS_MissingWebDir(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-web-assets", "fs"})
if err == nil {
t.Fatal("expected error when web-assets=fs but web-dir is missing, got nil")
}
if !strings.Contains(err.Error(), "web-assets=fs requires web-dir") {
t.Errorf("expected error about missing web-dir, got: %v", err)
}
}

func TestLoadConfig_WebAssets_FS_WithWebDir(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

webDir := filepath.Join(tmpDir, "web")
config, err := LoadConfig([]string{"-web-assets", "fs", "-web-dir", webDir})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.WebAssetsMode != "fs" {
t.Errorf("expected WebAssetsMode fs, got %s", config.WebAssetsMode)
}
if config.WebDir != webDir {
t.Errorf("expected WebDir %s, got %s", webDir, config.WebDir)
}
}

func TestLoadConfig_InvalidWebAssetsMode(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-web-assets", "invalid-mode"})
if err == nil {
t.Fatal("expected error for invalid web-assets mode, got nil")
}
if !strings.Contains(err.Error(), "unsupported web-assets mode") {
t.Errorf("expected error about unsupported web-assets mode, got: %v", err)
}
}

func TestLoadConfig_PathResolution_Relative(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{
"-db", "data/ratelord.db",
"-policy", "config/policy.json",
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

expectedDB := filepath.Join(tmpDir, "data/ratelord.db")
expectedPolicy := filepath.Join(tmpDir, "config/policy.json")

if config.DBPath != expectedDB {
t.Errorf("expected DBPath %s, got %s", expectedDB, config.DBPath)
}
if config.PolicyPath != expectedPolicy {
t.Errorf("expected PolicyPath %s, got %s", expectedPolicy, config.PolicyPath)
}
}

func TestLoadConfig_PathResolution_Absolute(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

absDBPath := "/absolute/path/to/db.db"
absPolicyPath := "/absolute/path/to/policy.json"

config, err := LoadConfig([]string{
"-db", absDBPath,
"-policy", absPolicyPath,
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.DBPath != absDBPath {
t.Errorf("expected DBPath %s, got %s", absDBPath, config.DBPath)
}
if config.PolicyPath != absPolicyPath {
t.Errorf("expected PolicyPath %s, got %s", absPolicyPath, config.PolicyPath)
}
}

func TestLoadConfig_EnvPortOnly(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

os.Setenv("RATELORD_PORT", "3000")
defer os.Unsetenv("RATELORD_PORT")

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

expected := "127.0.0.1:3000"
if config.Addr != expected {
t.Errorf("expected addr %s from RATELORD_PORT, got %s", expected, config.Addr)
}
}

func TestLoadConfig_EnvAddrOverridesPort(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

os.Setenv("RATELORD_ADDR", "0.0.0.0:8080")
os.Setenv("RATELORD_PORT", "3000")
defer func() {
os.Unsetenv("RATELORD_ADDR")
os.Unsetenv("RATELORD_PORT")
}()

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

// RATELORD_ADDR should take precedence over RATELORD_PORT
if config.Addr != "0.0.0.0:8080" {
t.Errorf("expected RATELORD_ADDR to override RATELORD_PORT, got %s", config.Addr)
}
}

func TestLoadConfig_PolicyPathFallback(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Test that RATELORD_CONFIG_PATH is used as fallback for RATELORD_POLICY_PATH
os.Setenv("RATELORD_CONFIG_PATH", "/fallback/config.json")
defer os.Unsetenv("RATELORD_CONFIG_PATH")

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.PolicyPath != "/fallback/config.json" {
t.Errorf("expected PolicyPath from RATELORD_CONFIG_PATH fallback, got %s", config.PolicyPath)
}
}

func TestLoadConfig_PolicyPathPrimaryOverFallback(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Test that RATELORD_POLICY_PATH takes precedence over RATELORD_CONFIG_PATH
os.Setenv("RATELORD_POLICY_PATH", "/primary/policy.json")
os.Setenv("RATELORD_CONFIG_PATH", "/fallback/config.json")
defer func() {
os.Unsetenv("RATELORD_POLICY_PATH")
os.Unsetenv("RATELORD_CONFIG_PATH")
}()

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.PolicyPath != "/primary/policy.json" {
t.Errorf("expected PolicyPath from RATELORD_POLICY_PATH, got %s", config.PolicyPath)
}
}

func TestNormalizeWebAssetsMode(t *testing.T) {
tests := []struct {
input string
expected string
}{
{"", "embedded"},
{"embedded", "embedded"},
{"EMBEDDED", "embedded"},
{" embedded ", "embedded"},
{"fs", "fs"},
{"FS", "fs"},
{"dir", "fs"},
{"directory", "fs"},
{"off", "off"},
{"OFF", "off"},
{"disabled", "off"},
{"none", "off"},
{"unknown", "unknown"},
}

for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
result := normalizeWebAssetsMode(tt.input)
if result != tt.expected {
t.Errorf("normalizeWebAssetsMode(%q) = %q, want %q", tt.input, result, tt.expected)
}
})
}
}

func TestResolvePath(t *testing.T) {
cwd := "/test/cwd"

tests := []struct {
name string
input string
expected string
}{
{"empty", "", ""},
{"whitespace", " ", ""},
{"absolute", "/absolute/path", "/absolute/path"},
{"relative", "relative/path", "/test/cwd/relative/path"},
{"with_whitespace", " relative/path ", "/test/cwd/relative/path"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := resolvePath(tt.input, cwd)
if result != tt.expected {
t.Errorf("resolvePath(%q, %q) = %q, want %q", tt.input, cwd, result, tt.expected)
}
})
}
}

func TestLoadConfig_WebDirResolution(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Test that web-dir gets resolved when web-assets=fs
config, err := LoadConfig([]string{
"-web-assets", "fs",
"-web-dir", "relative/web",
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

expectedWebDir := filepath.Join(tmpDir, "relative/web")
if config.WebDir != expectedWebDir {
t.Errorf("expected WebDir to be resolved to %s, got %s", expectedWebDir, config.WebDir)
}
}

func TestLoadConfig_WebAssets_Embedded(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{"-web-assets", "embedded"})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.WebAssetsMode != "embedded" {
t.Errorf("expected WebAssetsMode embedded, got %s", config.WebAssetsMode)
}
// WebDir can be empty for embedded mode
}

func TestLoadConfig_WebAssets_Off(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{"-web-assets", "off"})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.WebAssetsMode != "off" {
t.Errorf("expected WebAssetsMode off, got %s", config.WebAssetsMode)
}
}

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

Every test function contains identical setup code for directory creation and change that could be extracted into a helper function. This duplicated pattern (lines 13-21, 47-55, 94-102, etc.) appears in nearly every test function. Consider creating a test helper function to reduce duplication and improve maintainability. For example:

func setupTestDir(t *testing.T) (cleanup func()) {
    tmpDir := t.TempDir()
    oldWd, err := os.Getwd()
    if err != nil {
        t.Fatalf("failed to get working directory: %v", err)
    }
    if err := os.Chdir(tmpDir); err != nil {
        t.Fatalf("failed to change directory: %v", err)
    }
    return func() { os.Chdir(oldWd) }
}

Then each test could simply call: defer setupTestDir(t)()

Copilot uses AI. Check for mistakes.
Comment on lines +445 to +498
func TestNormalizeWebAssetsMode(t *testing.T) {
tests := []struct {
input string
expected string
}{
{"", "embedded"},
{"embedded", "embedded"},
{"EMBEDDED", "embedded"},
{" embedded ", "embedded"},
{"fs", "fs"},
{"FS", "fs"},
{"dir", "fs"},
{"directory", "fs"},
{"off", "off"},
{"OFF", "off"},
{"disabled", "off"},
{"none", "off"},
{"unknown", "unknown"},
}

for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
result := normalizeWebAssetsMode(tt.input)
if result != tt.expected {
t.Errorf("normalizeWebAssetsMode(%q) = %q, want %q", tt.input, result, tt.expected)
}
})
}
}

func TestResolvePath(t *testing.T) {
cwd := "/test/cwd"

tests := []struct {
name string
input string
expected string
}{
{"empty", "", ""},
{"whitespace", " ", ""},
{"absolute", "/absolute/path", "/absolute/path"},
{"relative", "relative/path", "/test/cwd/relative/path"},
{"with_whitespace", " relative/path ", "/test/cwd/relative/path"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := resolvePath(tt.input, cwd)
if result != tt.expected {
t.Errorf("resolvePath(%q, %q) = %q, want %q", tt.input, cwd, result, tt.expected)
}
})
}
}

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

The PR description claims "Helper function tests: resolvePath, normalizeWebAssetsMode, envOrDefaultWithFallback, addrFromEnv" but direct unit tests only exist for resolvePath and normalizeWebAssetsMode. The functions envOrDefaultWithFallback and addrFromEnv are only tested indirectly through LoadConfig integration tests (e.g., TestLoadConfig_PolicyPathFallback and TestLoadConfig_EnvPortOnly). While integration testing provides coverage, the PR description should accurately reflect what's implemented, or direct unit tests should be added for consistency with the claimed coverage.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +567
package main

import (
"os"
"path/filepath"
"strings"
"testing"
"time"
)

func TestLoadConfig_Defaults(t *testing.T) {
// Setup: Create temp dir and cd into it
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

// Verify defaults
if config.Addr != defaultAddr {
t.Errorf("expected default addr %s, got %s", defaultAddr, config.Addr)
}
if config.PollInterval != defaultPollInterval {
t.Errorf("expected default poll interval %v, got %v", defaultPollInterval, config.PollInterval)
}
if config.WebAssetsMode != defaultWebAssetsMode {
t.Errorf("expected default web assets mode %s, got %s", defaultWebAssetsMode, config.WebAssetsMode)
}
if !strings.HasSuffix(config.DBPath, "ratelord.db") {
t.Errorf("expected DBPath to end with ratelord.db, got %s", config.DBPath)
}
if !strings.HasSuffix(config.PolicyPath, "policy.json") {
t.Errorf("expected PolicyPath to end with policy.json, got %s", config.PolicyPath)
}
}

func TestLoadConfig_EnvVars(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Set env vars
os.Setenv("RATELORD_DB_PATH", "/custom/db.db")
os.Setenv("RATELORD_POLICY_PATH", "/custom/policy.json")
os.Setenv("RATELORD_ADDR", "0.0.0.0:9000")
os.Setenv("RATELORD_POLL_INTERVAL", "30s")
os.Setenv("RATELORD_WEB_ASSETS_MODE", "off")
defer func() {
os.Unsetenv("RATELORD_DB_PATH")
os.Unsetenv("RATELORD_POLICY_PATH")
os.Unsetenv("RATELORD_ADDR")
os.Unsetenv("RATELORD_POLL_INTERVAL")
os.Unsetenv("RATELORD_WEB_ASSETS_MODE")
}()

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.DBPath != "/custom/db.db" {
t.Errorf("expected DBPath from env /custom/db.db, got %s", config.DBPath)
}
if config.PolicyPath != "/custom/policy.json" {
t.Errorf("expected PolicyPath from env /custom/policy.json, got %s", config.PolicyPath)
}
if config.Addr != "0.0.0.0:9000" {
t.Errorf("expected Addr from env 0.0.0.0:9000, got %s", config.Addr)
}
if config.PollInterval != 30*time.Second {
t.Errorf("expected PollInterval from env 30s, got %v", config.PollInterval)
}
if config.WebAssetsMode != "off" {
t.Errorf("expected WebAssetsMode from env off, got %s", config.WebAssetsMode)
}
}

func TestLoadConfig_FlagOverridesEnv(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Set env vars
os.Setenv("RATELORD_ADDR", "env-addr:8080")
os.Setenv("RATELORD_POLL_INTERVAL", "15s")
defer func() {
os.Unsetenv("RATELORD_ADDR")
os.Unsetenv("RATELORD_POLL_INTERVAL")
}()

// Flags should override env
config, err := LoadConfig([]string{
"-addr", "flag-addr:9090",
"-poll-interval", "25s",
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.Addr != "flag-addr:9090" {
t.Errorf("expected flag to override env for addr, got %s", config.Addr)
}
if config.PollInterval != 25*time.Second {
t.Errorf("expected flag to override env for poll-interval, got %v", config.PollInterval)
}
}

func TestLoadConfig_InvalidPollInterval_Env(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

os.Setenv("RATELORD_POLL_INTERVAL", "invalid")
defer os.Unsetenv("RATELORD_POLL_INTERVAL")

_, err = LoadConfig([]string{})
if err == nil {
t.Fatal("expected error for invalid RATELORD_POLL_INTERVAL, got nil")
}
if !strings.Contains(err.Error(), "invalid RATELORD_POLL_INTERVAL") {
t.Errorf("expected error message to contain 'invalid RATELORD_POLL_INTERVAL', got: %v", err)
}
}

func TestLoadConfig_InvalidPollInterval_Flag(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-poll-interval", "not-a-duration"})
if err == nil {
t.Fatal("expected error for invalid poll-interval flag, got nil")
}
if !strings.Contains(err.Error(), "invalid poll interval") {
t.Errorf("expected error message to contain 'invalid poll interval', got: %v", err)
}
}

func TestLoadConfig_EmptyAddr(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-addr", ""})
if err == nil {
t.Fatal("expected error for empty addr, got nil")
}
if !strings.Contains(err.Error(), "addr cannot be empty") {
t.Errorf("expected error about empty addr, got: %v", err)
}
}

func TestLoadConfig_EmptyAddr_Whitespace(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-addr", " "})
if err == nil {
t.Fatal("expected error for whitespace-only addr, got nil")
}
if !strings.Contains(err.Error(), "addr cannot be empty") {
t.Errorf("expected error about empty addr, got: %v", err)
}
}

func TestLoadConfig_WebAssets_FS_MissingWebDir(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-web-assets", "fs"})
if err == nil {
t.Fatal("expected error when web-assets=fs but web-dir is missing, got nil")
}
if !strings.Contains(err.Error(), "web-assets=fs requires web-dir") {
t.Errorf("expected error about missing web-dir, got: %v", err)
}
}

func TestLoadConfig_WebAssets_FS_WithWebDir(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

webDir := filepath.Join(tmpDir, "web")
config, err := LoadConfig([]string{"-web-assets", "fs", "-web-dir", webDir})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.WebAssetsMode != "fs" {
t.Errorf("expected WebAssetsMode fs, got %s", config.WebAssetsMode)
}
if config.WebDir != webDir {
t.Errorf("expected WebDir %s, got %s", webDir, config.WebDir)
}
}

func TestLoadConfig_InvalidWebAssetsMode(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-web-assets", "invalid-mode"})
if err == nil {
t.Fatal("expected error for invalid web-assets mode, got nil")
}
if !strings.Contains(err.Error(), "unsupported web-assets mode") {
t.Errorf("expected error about unsupported web-assets mode, got: %v", err)
}
}

func TestLoadConfig_PathResolution_Relative(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{
"-db", "data/ratelord.db",
"-policy", "config/policy.json",
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

expectedDB := filepath.Join(tmpDir, "data/ratelord.db")
expectedPolicy := filepath.Join(tmpDir, "config/policy.json")

if config.DBPath != expectedDB {
t.Errorf("expected DBPath %s, got %s", expectedDB, config.DBPath)
}
if config.PolicyPath != expectedPolicy {
t.Errorf("expected PolicyPath %s, got %s", expectedPolicy, config.PolicyPath)
}
}

func TestLoadConfig_PathResolution_Absolute(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

absDBPath := "/absolute/path/to/db.db"
absPolicyPath := "/absolute/path/to/policy.json"

config, err := LoadConfig([]string{
"-db", absDBPath,
"-policy", absPolicyPath,
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.DBPath != absDBPath {
t.Errorf("expected DBPath %s, got %s", absDBPath, config.DBPath)
}
if config.PolicyPath != absPolicyPath {
t.Errorf("expected PolicyPath %s, got %s", absPolicyPath, config.PolicyPath)
}
}

func TestLoadConfig_EnvPortOnly(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

os.Setenv("RATELORD_PORT", "3000")
defer os.Unsetenv("RATELORD_PORT")

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

expected := "127.0.0.1:3000"
if config.Addr != expected {
t.Errorf("expected addr %s from RATELORD_PORT, got %s", expected, config.Addr)
}
}

func TestLoadConfig_EnvAddrOverridesPort(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

os.Setenv("RATELORD_ADDR", "0.0.0.0:8080")
os.Setenv("RATELORD_PORT", "3000")
defer func() {
os.Unsetenv("RATELORD_ADDR")
os.Unsetenv("RATELORD_PORT")
}()

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

// RATELORD_ADDR should take precedence over RATELORD_PORT
if config.Addr != "0.0.0.0:8080" {
t.Errorf("expected RATELORD_ADDR to override RATELORD_PORT, got %s", config.Addr)
}
}

func TestLoadConfig_PolicyPathFallback(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Test that RATELORD_CONFIG_PATH is used as fallback for RATELORD_POLICY_PATH
os.Setenv("RATELORD_CONFIG_PATH", "/fallback/config.json")
defer os.Unsetenv("RATELORD_CONFIG_PATH")

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.PolicyPath != "/fallback/config.json" {
t.Errorf("expected PolicyPath from RATELORD_CONFIG_PATH fallback, got %s", config.PolicyPath)
}
}

func TestLoadConfig_PolicyPathPrimaryOverFallback(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Test that RATELORD_POLICY_PATH takes precedence over RATELORD_CONFIG_PATH
os.Setenv("RATELORD_POLICY_PATH", "/primary/policy.json")
os.Setenv("RATELORD_CONFIG_PATH", "/fallback/config.json")
defer func() {
os.Unsetenv("RATELORD_POLICY_PATH")
os.Unsetenv("RATELORD_CONFIG_PATH")
}()

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.PolicyPath != "/primary/policy.json" {
t.Errorf("expected PolicyPath from RATELORD_POLICY_PATH, got %s", config.PolicyPath)
}
}

func TestNormalizeWebAssetsMode(t *testing.T) {
tests := []struct {
input string
expected string
}{
{"", "embedded"},
{"embedded", "embedded"},
{"EMBEDDED", "embedded"},
{" embedded ", "embedded"},
{"fs", "fs"},
{"FS", "fs"},
{"dir", "fs"},
{"directory", "fs"},
{"off", "off"},
{"OFF", "off"},
{"disabled", "off"},
{"none", "off"},
{"unknown", "unknown"},
}

for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
result := normalizeWebAssetsMode(tt.input)
if result != tt.expected {
t.Errorf("normalizeWebAssetsMode(%q) = %q, want %q", tt.input, result, tt.expected)
}
})
}
}

func TestResolvePath(t *testing.T) {
cwd := "/test/cwd"

tests := []struct {
name string
input string
expected string
}{
{"empty", "", ""},
{"whitespace", " ", ""},
{"absolute", "/absolute/path", "/absolute/path"},
{"relative", "relative/path", "/test/cwd/relative/path"},
{"with_whitespace", " relative/path ", "/test/cwd/relative/path"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := resolvePath(tt.input, cwd)
if result != tt.expected {
t.Errorf("resolvePath(%q, %q) = %q, want %q", tt.input, cwd, result, tt.expected)
}
})
}
}

func TestLoadConfig_WebDirResolution(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Test that web-dir gets resolved when web-assets=fs
config, err := LoadConfig([]string{
"-web-assets", "fs",
"-web-dir", "relative/web",
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

expectedWebDir := filepath.Join(tmpDir, "relative/web")
if config.WebDir != expectedWebDir {
t.Errorf("expected WebDir to be resolved to %s, got %s", expectedWebDir, config.WebDir)
}
}

func TestLoadConfig_WebAssets_Embedded(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{"-web-assets", "embedded"})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.WebAssetsMode != "embedded" {
t.Errorf("expected WebAssetsMode embedded, got %s", config.WebAssetsMode)
}
// WebDir can be empty for embedded mode
}

func TestLoadConfig_WebAssets_Off(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{"-web-assets", "off"})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.WebAssetsMode != "off" {
t.Errorf("expected WebAssetsMode off, got %s", config.WebAssetsMode)
}
}

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

The error path at config.go:32 (failed to get cwd) is not covered by tests. While this is difficult to test without mocking (since os.Getwd() rarely fails in practice), consider documenting this known limitation in test comments, or adding a test that uses build tags or other techniques to simulate this failure if comprehensive coverage is a goal. Alternatively, accept this as an acceptable gap for an error that is extremely rare in production.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +567
package main

import (
"os"
"path/filepath"
"strings"
"testing"
"time"
)

func TestLoadConfig_Defaults(t *testing.T) {
// Setup: Create temp dir and cd into it
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

// Verify defaults
if config.Addr != defaultAddr {
t.Errorf("expected default addr %s, got %s", defaultAddr, config.Addr)
}
if config.PollInterval != defaultPollInterval {
t.Errorf("expected default poll interval %v, got %v", defaultPollInterval, config.PollInterval)
}
if config.WebAssetsMode != defaultWebAssetsMode {
t.Errorf("expected default web assets mode %s, got %s", defaultWebAssetsMode, config.WebAssetsMode)
}
if !strings.HasSuffix(config.DBPath, "ratelord.db") {
t.Errorf("expected DBPath to end with ratelord.db, got %s", config.DBPath)
}
if !strings.HasSuffix(config.PolicyPath, "policy.json") {
t.Errorf("expected PolicyPath to end with policy.json, got %s", config.PolicyPath)
}
}

func TestLoadConfig_EnvVars(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Set env vars
os.Setenv("RATELORD_DB_PATH", "/custom/db.db")
os.Setenv("RATELORD_POLICY_PATH", "/custom/policy.json")
os.Setenv("RATELORD_ADDR", "0.0.0.0:9000")
os.Setenv("RATELORD_POLL_INTERVAL", "30s")
os.Setenv("RATELORD_WEB_ASSETS_MODE", "off")
defer func() {
os.Unsetenv("RATELORD_DB_PATH")
os.Unsetenv("RATELORD_POLICY_PATH")
os.Unsetenv("RATELORD_ADDR")
os.Unsetenv("RATELORD_POLL_INTERVAL")
os.Unsetenv("RATELORD_WEB_ASSETS_MODE")
}()

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.DBPath != "/custom/db.db" {
t.Errorf("expected DBPath from env /custom/db.db, got %s", config.DBPath)
}
if config.PolicyPath != "/custom/policy.json" {
t.Errorf("expected PolicyPath from env /custom/policy.json, got %s", config.PolicyPath)
}
if config.Addr != "0.0.0.0:9000" {
t.Errorf("expected Addr from env 0.0.0.0:9000, got %s", config.Addr)
}
if config.PollInterval != 30*time.Second {
t.Errorf("expected PollInterval from env 30s, got %v", config.PollInterval)
}
if config.WebAssetsMode != "off" {
t.Errorf("expected WebAssetsMode from env off, got %s", config.WebAssetsMode)
}
}

func TestLoadConfig_FlagOverridesEnv(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Set env vars
os.Setenv("RATELORD_ADDR", "env-addr:8080")
os.Setenv("RATELORD_POLL_INTERVAL", "15s")
defer func() {
os.Unsetenv("RATELORD_ADDR")
os.Unsetenv("RATELORD_POLL_INTERVAL")
}()

// Flags should override env
config, err := LoadConfig([]string{
"-addr", "flag-addr:9090",
"-poll-interval", "25s",
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.Addr != "flag-addr:9090" {
t.Errorf("expected flag to override env for addr, got %s", config.Addr)
}
if config.PollInterval != 25*time.Second {
t.Errorf("expected flag to override env for poll-interval, got %v", config.PollInterval)
}
}

func TestLoadConfig_InvalidPollInterval_Env(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

os.Setenv("RATELORD_POLL_INTERVAL", "invalid")
defer os.Unsetenv("RATELORD_POLL_INTERVAL")

_, err = LoadConfig([]string{})
if err == nil {
t.Fatal("expected error for invalid RATELORD_POLL_INTERVAL, got nil")
}
if !strings.Contains(err.Error(), "invalid RATELORD_POLL_INTERVAL") {
t.Errorf("expected error message to contain 'invalid RATELORD_POLL_INTERVAL', got: %v", err)
}
}

func TestLoadConfig_InvalidPollInterval_Flag(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-poll-interval", "not-a-duration"})
if err == nil {
t.Fatal("expected error for invalid poll-interval flag, got nil")
}
if !strings.Contains(err.Error(), "invalid poll interval") {
t.Errorf("expected error message to contain 'invalid poll interval', got: %v", err)
}
}

func TestLoadConfig_EmptyAddr(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-addr", ""})
if err == nil {
t.Fatal("expected error for empty addr, got nil")
}
if !strings.Contains(err.Error(), "addr cannot be empty") {
t.Errorf("expected error about empty addr, got: %v", err)
}
}

func TestLoadConfig_EmptyAddr_Whitespace(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-addr", " "})
if err == nil {
t.Fatal("expected error for whitespace-only addr, got nil")
}
if !strings.Contains(err.Error(), "addr cannot be empty") {
t.Errorf("expected error about empty addr, got: %v", err)
}
}

func TestLoadConfig_WebAssets_FS_MissingWebDir(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-web-assets", "fs"})
if err == nil {
t.Fatal("expected error when web-assets=fs but web-dir is missing, got nil")
}
if !strings.Contains(err.Error(), "web-assets=fs requires web-dir") {
t.Errorf("expected error about missing web-dir, got: %v", err)
}
}

func TestLoadConfig_WebAssets_FS_WithWebDir(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

webDir := filepath.Join(tmpDir, "web")
config, err := LoadConfig([]string{"-web-assets", "fs", "-web-dir", webDir})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.WebAssetsMode != "fs" {
t.Errorf("expected WebAssetsMode fs, got %s", config.WebAssetsMode)
}
if config.WebDir != webDir {
t.Errorf("expected WebDir %s, got %s", webDir, config.WebDir)
}
}

func TestLoadConfig_InvalidWebAssetsMode(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

_, err = LoadConfig([]string{"-web-assets", "invalid-mode"})
if err == nil {
t.Fatal("expected error for invalid web-assets mode, got nil")
}
if !strings.Contains(err.Error(), "unsupported web-assets mode") {
t.Errorf("expected error about unsupported web-assets mode, got: %v", err)
}
}

func TestLoadConfig_PathResolution_Relative(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{
"-db", "data/ratelord.db",
"-policy", "config/policy.json",
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

expectedDB := filepath.Join(tmpDir, "data/ratelord.db")
expectedPolicy := filepath.Join(tmpDir, "config/policy.json")

if config.DBPath != expectedDB {
t.Errorf("expected DBPath %s, got %s", expectedDB, config.DBPath)
}
if config.PolicyPath != expectedPolicy {
t.Errorf("expected PolicyPath %s, got %s", expectedPolicy, config.PolicyPath)
}
}

func TestLoadConfig_PathResolution_Absolute(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

absDBPath := "/absolute/path/to/db.db"
absPolicyPath := "/absolute/path/to/policy.json"

config, err := LoadConfig([]string{
"-db", absDBPath,
"-policy", absPolicyPath,
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.DBPath != absDBPath {
t.Errorf("expected DBPath %s, got %s", absDBPath, config.DBPath)
}
if config.PolicyPath != absPolicyPath {
t.Errorf("expected PolicyPath %s, got %s", absPolicyPath, config.PolicyPath)
}
}

func TestLoadConfig_EnvPortOnly(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

os.Setenv("RATELORD_PORT", "3000")
defer os.Unsetenv("RATELORD_PORT")

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

expected := "127.0.0.1:3000"
if config.Addr != expected {
t.Errorf("expected addr %s from RATELORD_PORT, got %s", expected, config.Addr)
}
}

func TestLoadConfig_EnvAddrOverridesPort(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

os.Setenv("RATELORD_ADDR", "0.0.0.0:8080")
os.Setenv("RATELORD_PORT", "3000")
defer func() {
os.Unsetenv("RATELORD_ADDR")
os.Unsetenv("RATELORD_PORT")
}()

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

// RATELORD_ADDR should take precedence over RATELORD_PORT
if config.Addr != "0.0.0.0:8080" {
t.Errorf("expected RATELORD_ADDR to override RATELORD_PORT, got %s", config.Addr)
}
}

func TestLoadConfig_PolicyPathFallback(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Test that RATELORD_CONFIG_PATH is used as fallback for RATELORD_POLICY_PATH
os.Setenv("RATELORD_CONFIG_PATH", "/fallback/config.json")
defer os.Unsetenv("RATELORD_CONFIG_PATH")

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.PolicyPath != "/fallback/config.json" {
t.Errorf("expected PolicyPath from RATELORD_CONFIG_PATH fallback, got %s", config.PolicyPath)
}
}

func TestLoadConfig_PolicyPathPrimaryOverFallback(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Test that RATELORD_POLICY_PATH takes precedence over RATELORD_CONFIG_PATH
os.Setenv("RATELORD_POLICY_PATH", "/primary/policy.json")
os.Setenv("RATELORD_CONFIG_PATH", "/fallback/config.json")
defer func() {
os.Unsetenv("RATELORD_POLICY_PATH")
os.Unsetenv("RATELORD_CONFIG_PATH")
}()

config, err := LoadConfig([]string{})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.PolicyPath != "/primary/policy.json" {
t.Errorf("expected PolicyPath from RATELORD_POLICY_PATH, got %s", config.PolicyPath)
}
}

func TestNormalizeWebAssetsMode(t *testing.T) {
tests := []struct {
input string
expected string
}{
{"", "embedded"},
{"embedded", "embedded"},
{"EMBEDDED", "embedded"},
{" embedded ", "embedded"},
{"fs", "fs"},
{"FS", "fs"},
{"dir", "fs"},
{"directory", "fs"},
{"off", "off"},
{"OFF", "off"},
{"disabled", "off"},
{"none", "off"},
{"unknown", "unknown"},
}

for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
result := normalizeWebAssetsMode(tt.input)
if result != tt.expected {
t.Errorf("normalizeWebAssetsMode(%q) = %q, want %q", tt.input, result, tt.expected)
}
})
}
}

func TestResolvePath(t *testing.T) {
cwd := "/test/cwd"

tests := []struct {
name string
input string
expected string
}{
{"empty", "", ""},
{"whitespace", " ", ""},
{"absolute", "/absolute/path", "/absolute/path"},
{"relative", "relative/path", "/test/cwd/relative/path"},
{"with_whitespace", " relative/path ", "/test/cwd/relative/path"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := resolvePath(tt.input, cwd)
if result != tt.expected {
t.Errorf("resolvePath(%q, %q) = %q, want %q", tt.input, cwd, result, tt.expected)
}
})
}
}

func TestLoadConfig_WebDirResolution(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

// Test that web-dir gets resolved when web-assets=fs
config, err := LoadConfig([]string{
"-web-assets", "fs",
"-web-dir", "relative/web",
})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

expectedWebDir := filepath.Join(tmpDir, "relative/web")
if config.WebDir != expectedWebDir {
t.Errorf("expected WebDir to be resolved to %s, got %s", expectedWebDir, config.WebDir)
}
}

func TestLoadConfig_WebAssets_Embedded(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{"-web-assets", "embedded"})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.WebAssetsMode != "embedded" {
t.Errorf("expected WebAssetsMode embedded, got %s", config.WebAssetsMode)
}
// WebDir can be empty for embedded mode
}

func TestLoadConfig_WebAssets_Off(t *testing.T) {
tmpDir := t.TempDir()
oldWd, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get working directory: %v", err)
}
defer os.Chdir(oldWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("failed to change directory: %v", err)
}

config, err := LoadConfig([]string{"-web-assets", "off"})
if err != nil {
t.Fatalf("LoadConfig failed: %v", err)
}

if config.WebAssetsMode != "off" {
t.Errorf("expected WebAssetsMode off, got %s", config.WebAssetsMode)
}
}

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

The RATELORD_WEB_DIR environment variable (referenced in config.go:50) is not tested. While the -web-dir flag is tested in TestLoadConfig_WebAssets_FS_WithWebDir and TestLoadConfig_WebDirResolution, there's no test verifying that the RATELORD_WEB_DIR environment variable is properly read and used. Consider adding a test like TestLoadConfig_WebDirFromEnv to ensure complete coverage of configuration sources.

Copilot uses AI. Check for mistakes.
Comment on lines +445 to +472
func TestNormalizeWebAssetsMode(t *testing.T) {
tests := []struct {
input string
expected string
}{
{"", "embedded"},
{"embedded", "embedded"},
{"EMBEDDED", "embedded"},
{" embedded ", "embedded"},
{"fs", "fs"},
{"FS", "fs"},
{"dir", "fs"},
{"directory", "fs"},
{"off", "off"},
{"OFF", "off"},
{"disabled", "off"},
{"none", "off"},
{"unknown", "unknown"},
}

for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) {
result := normalizeWebAssetsMode(tt.input)
if result != tt.expected {
t.Errorf("normalizeWebAssetsMode(%q) = %q, want %q", tt.input, result, tt.expected)
}
})
}

Copilot AI Feb 5, 2026

Copy link

Choose a reason for hiding this comment

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

In the table-driven test TestNormalizeWebAssetsMode, using the input value directly as the test name in t.Run(tt.input, ...) can be problematic when the input is an empty string or contains special characters. The empty string test case (line 450) will create a subtest with an empty name. Consider using tt.name field or constructing a more descriptive name. For example, change the test struct to include a name field and use t.Run(tt.name, ...) instead.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants