fix(config): de-flake TestAddFlags by stubbing time source

[CaelRowley]

Overview

Fixes a flaky TestAddFlags caused by non-deterministic defaults.
DA.Namespace used randString seeded with time.Now().Unix(), and DefaultConfig() was called twice in the test (once in AddFlags, once for the expected value). If the timestamp second changed between calls, the seeds diverged and the assertion failed.

This PR extracts the time source into a package-level nowUnix variable in defaults.go, allowing the test to fix the seed and make both calls deterministic.

Reproduced locally via go test ./... -count=1:

  --- FAIL: TestAddFlags (0.00s)                                                                         
      config_test.go:502:
          Error: Not equal:                                                                              
              expected: "5ofJh48irg"
              actual  : "Duu3osBRoN"                                                                     
          Test:    TestAddFlags                                                                          
          Messages: Flag evnode.da.namespace should have default value 5ofJh48irg
  FAIL    github.com/evstack/ev-node/pkg/config   3.396s   

Summary by CodeRabbit

Release Notes

• Tests
  • Enhanced test infrastructure to ensure deterministic test execution by implementing time control mechanisms during testing, enabling consistent and reliable test results.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR introduces a package-level nowUnix function variable to abstract the time source, enabling tests to stub the current Unix timestamp. The randString function is updated to use this stub instead of directly calling time.Now().Unix(), allowing deterministic seed generation during testing.

Changes

Time Abstraction for Deterministic Testing

Layer / File(s)	Summary
Data Shape / Abstraction pkg/config/defaults.go	Introduces package-level nowUnix variable that returns the current Unix timestamp, replacing direct time.Now().Unix() calls.
Core Logic Update pkg/config/defaults.go	randString function is refactored to seed its RNG using nowUnix() instead of time.Now().Unix().
Test Stubbing pkg/config/config_test.go	TestAddFlags saves the original nowUnix, overrides it to return a fixed timestamp (2_000_000_000) for deterministic DA.Namespace generation, and restores it via cleanup.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 Time stands still when tests must pass,
A frozen moment, steady as glass.
Two thousand million marks our line,
Where seeds and randomness align.
Determinism hops through the day!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: fixing a flaky test by stubbing the time source.
Description check	✅ Passed	The description provides a comprehensive overview with context, the problem, the solution, and reproduction steps; it fully satisfies the template requirements.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

pkg/config/defaults.go (1)

136-137: 💤 Low value

Package-level mutable nowUnix is unsafe if tests ever run in parallel.

Mutating a bare var from a test goroutine while another goroutine reads it (e.g., via a concurrent DefaultConfig() call) is a data race that go test -race will catch. Currently no tests in this package call t.Parallel(), so there is no active race, but the risk is latent.

A minimal guard would use sync/atomic with a stored int64, or accept the constraint by adding a comment that parallel tests must not stub nowUnix. As an alternative, passing the seed as a parameter to randString (and accepting it from DefaultConfig) avoids the global entirely.

🛡️ Option A – atomic swap (low-footprint)

-// nowUnix returns the current Unix timestamp; package-level so tests can stub it.
-var nowUnix = func() int64 { return time.Now().Unix() }
+import "sync/atomic"
+import "unsafe"
+
+// nowUnixFn is the actual function pointer; swapped atomically in tests.
+var nowUnixPtr atomic.Pointer[func() int64]
+
+func init() {
+    f := func() int64 { return time.Now().Unix() }
+    nowUnixPtr.Store(&f)
+}
+
+func nowUnix() int64 { return (*nowUnixPtr.Load())() }

In tests:

-origNowUnix := nowUnix
-nowUnix = func() int64 { return 2_000_000_000 }
-t.Cleanup(func() { nowUnix = origNowUnix })
+f := func() int64 { return 2_000_000_000 }
+nowUnixPtr.Store(&f)
+t.Cleanup(func() {
+    orig := func() int64 { return time.Now().Unix() }
+    nowUnixPtr.Store(&orig)
+})

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/config/defaults.go` around lines 136 - 137, The package-level mutable var
nowUnix must be replaced with an atomic-backed value to avoid races: introduce a
package int64 nowUnixVal and replace the var nowUnix func with a getter
NowUnix() that returns atomic.LoadInt64(&nowUnixVal), add a test helper
SetNowUnixForTest(v int64) that does atomic.StoreInt64(&nowUnixVal), and update
all callers (e.g., DefaultConfig and randString) to call NowUnix() instead of
invoking the former nowUnix variable; this preserves test stubbing without data
races.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/config/defaults.go`:
- Around line 136-137: The package-level mutable var nowUnix must be replaced
with an atomic-backed value to avoid races: introduce a package int64 nowUnixVal
and replace the var nowUnix func with a getter NowUnix() that returns
atomic.LoadInt64(&nowUnixVal), add a test helper SetNowUnixForTest(v int64) that
does atomic.StoreInt64(&nowUnixVal), and update all callers (e.g., DefaultConfig
and randString) to call NowUnix() instead of invoking the former nowUnix
variable; this preserves test stubbing without data races.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d64c11e7-8bfc-4fc1-b2f6-1932d49513c1

📥 Commits

Reviewing files that changed from the base of the PR and between 264314f and be48b91.

📒 Files selected for processing (2)

• pkg/config/config_test.go
• pkg/config/defaults.go