Skip to content

PB: database_test.go doesn't isolate DB_* env vars, so tests can fail depending on process environment #524

Description

@mushrafmim

Describe the Bug

internal/pb/v1/database_test.go:

  • TestNewDatabaseConfig (lines ~11-23) asserts default values ("localhost", "5432", etc.) without clearing any DB_* env vars first, so it fails if the test process already has one set.
  • TestNewDatabaseConfig_WithEnvVars (lines ~26-49) sets DB_* vars with manual os.Setenv and manually os.Unsetenvs them in t.Cleanup, rather than restoring whatever value (if any) existed beforehand.

Surfaced by CodeRabbit reviewing #517 — this code predates that PR, it just moved location (was portal-backend/v1/database_test.go).

Expected Behavior

Use t.Setenv for every DB_* variable in both tests — set explicit empty values in TestNewDatabaseConfig before asserting defaults, and use t.Setenv (instead of manual os.Setenv/os.Unsetenv) in TestNewDatabaseConfig_WithEnvVars, since t.Setenv automatically restores the prior value after the test and fails the test if run with t.Parallel() misuse.

Version

Surfaced on PR #517 (refactor/portal-backend-restructure).

Additional Context

Flagged 🟡 Minor (Functional Correctness) by CodeRabbit.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions