feat(etcd): use testcontainers-go for etcd tests

[mdelapenya]

What does this PR do?

It uses Testcontainers Go's etcd module for testing the etcd Storage module.

It uses the latest release of the official etcd Docker image (there is no latest tag in the repo), which is replacing the bitnami/etcd:latest that was used before these changes.

The test store that is created spins up an etcd cluster of two nodes.

Why is it important?

Enable local testing of the module, also making the CI reproducible.

Summary by CodeRabbit

• Refactor
  • Improved test isolation and reliability by updating tests to use dynamically created, temporary etcd clusters for each run.
  • Replaced shared global test resources with a helper function for creating fresh storage instances in tests.
• Tests
  • Updated all tests and benchmarks to use the new test setup, ensuring each test runs against its own isolated environment.

[coderabbitai[bot]]

[!NOTE]

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Replaces the global TestMain/global testStore with a per-test helper newTestStore(t testing.TB) that starts a one-node etcd container via testcontainers-go; tests create isolated Storage instances and defer Close. Benchmarks reuse a singleton store via sync.Once and perform per-benchmark key cleanup; container cleanup is skipped for benchmarks.

Changes

Cohort / File(s)	Summary
Test helper & tests etcd/etcd_test.go	Added newTestStore(t testing.TB) *Storage which starts a 1-node etcd via testcontainers-go (image from env or default). Removed TestMain/global testStore; refactored tests to call newTestStore(t) and defer Close(). Added image constants, env handling, context usage, error handling and new imports.
Benchmark sharing & cleanup etcd/etcd_test.go	Introduced getBenchmarkStore() singleton using sync.Once to share an etcd container for benchmarks; benchmarks use per-benchmark key namespaces and cleanup keys after runs instead of tearing down the container.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant T as Test (t)
  participant H as newTestStore
  participant C as etcd Container
  participant S as Storage

  rect rgb(230,245,255)
  Note over T,H: Per-test setup (new behavior)
  T->>H: call newTestStore(t)
  H->>C: start container (TEST_ETCD_IMAGE or default)
  C-->>H: return endpoint
  H->>S: init Storage with endpoint
  H-->>T: return *Storage
  T->>S: run test operations
  T->>S: defer Close()
  S->>C: stop & remove (unless benchmark)
  end

  rect rgb(245,250,230)
  Note over T,H: Benchmark path (shared)
  T->>H: call getBenchmarkStore()
  H->>H: sync.Once ensures single start
  H->>C: start container (once)
  H-->>T: return shared *Storage
  T->>S: run benchmark operations (per-key namespace)
  T->>S: cleanup keys only
  end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Check container start/stop timeouts and error handling in newTestStore.
• Verify sync.Once correctness for benchmark singleton and that benchmarks clean up keys reliably.
• Confirm imports and build pass in CI.

Possibly related PRs

• gofiber/storage#1639 — Same pattern: adds newTestStore replacing a global testStore with testcontainers-go per-test setup.
• gofiber/storage#1724 — Similar refactor removing TestMain/global store in favor of per-test containerized instances.
• gofiber/storage#1508 — Refactors tests to use testcontainers-go newTestStore helper across storage tests.

Suggested reviewers

• sixcolors
• efectn
• gaby
• ReneWerner87

Poem

🐇 In a burrow of code I quietly dart,

Spawning containers, each test gets its part.
Benchmarks share, keys hop away,
No globals linger to spoil the play,
A rabbit nods — tidy tests, warm heart.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: replacing the previous etcd test setup with testcontainers-go library for testing.

✨ Finishing touches

• [ ] 📝 Generate docstrings

🧪 Generate unit tests (beta)

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0bb070d45d4935508b7cc81135ef6f5674a03269 and 7d8ee80cbe42ac396d7003654e857df55f718cb6.

⛔ Files ignored due to path filters (2)

• .github/workflows/benchmark.yml is excluded by !**/*.yml
• .github/workflows/test-etcd.yml is excluded by !**/*.yml

📒 Files selected for processing (1)

• etcd/etcd_test.go (10 hunks)

🧰 Additional context used

🧠 Learnings (17)

📓 Common learnings

Learnt from: mdelapenya
Repo: gofiber/storage PR: 1665
File: cassandra/cassandra_test.go:35-38
Timestamp: 2025-04-20T23:52:03.362Z
Learning: In testcontainers-go, calling `testcontainers.CleanupContainer(t, c)` before checking the error from container creation is safe due to the implementation details of the library. The CleanupContainer function handles nil or partially initialized containers gracefully.

Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.

Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests in this project use testify require.

📚 Learning: 2024-10-12T10:01:44.206Z

Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/session/middleware_test.go:190-191
Timestamp: 2024-10-12T10:01:44.206Z
Learning: When testing session `IdleTimeout` expiration, it's acceptable to use `time.Sleep` to simulate the passage of time in tests.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2024-11-29T12:37:27.581Z

Learnt from: efectn
Repo: gofiber/fiber PR: 3162
File: app_test.go:893-895
Timestamp: 2024-11-29T12:37:27.581Z
Learning: In the `Test_App_ShutdownWithContext` function in `app_test.go`, the `clientDone` channel is used to synchronize the client's request completion before proceeding, eliminating the need for additional `time.Sleep` calls.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2025-04-20T23:52:03.362Z

Learnt from: mdelapenya
Repo: gofiber/storage PR: 1665
File: cassandra/cassandra_test.go:35-38
Timestamp: 2025-04-20T23:52:03.362Z
Learning: In testcontainers-go, calling `testcontainers.CleanupContainer(t, c)` before checking the error from container creation is safe due to the implementation details of the library. The CleanupContainer function handles nil or partially initialized containers gracefully.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2025-02-12T11:24:31.153Z

Learnt from: ReneWerner87
Repo: gofiber/storage PR: 0
File: :0-0
Timestamp: 2025-02-12T11:24:31.153Z
Learning: The storage package in gofiber/storage repository can be used independently of the Fiber web framework.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2025-01-30T12:29:41.774Z

Learnt from: the-real-i9
Repo: gofiber/storage PR: 1562
File: neo4j/neo4j.go:50-52
Timestamp: 2025-01-30T12:29:41.774Z
Learning: Storage drivers in the gofiber/storage repository intentionally panic on connection errors as a design choice to fail fast when storage is unavailable.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2025-11-24T15:57:17.557Z

Learnt from: mdelapenya
Repo: gofiber/storage PR: 2261
File: mysql/mysql.go:131-136
Timestamp: 2025-11-24T15:57:17.557Z
Learning: In the gofiber/storage repository, storage backends currently have inconsistent expiry handling patterns. Four backends (cassandra, mysql, surrealdb, nats) use eager deletion during Get operations that delete expired keys immediately, while most other backends (~25+) use lazy deletion that simply returns nil for expired keys. Among eager deleters, cassandra and mysql propagate deletion errors (fail-fast), while surrealdb and nats ignore deletion errors (fire-and-forget). There is an ongoing effort to standardize this behavior across all storage backends.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2025-05-01T11:46:33.057Z

Learnt from: gaby
Repo: gofiber/storage PR: 1638
File: surrealdb/surrealdb.go:34-43
Timestamp: 2025-05-01T11:46:33.057Z
Learning: In the gofiber/storage package, storage drivers should panic when encountering database connection failures during initialization rather than returning errors. This is an intentional design pattern to ensure applications fail fast and explicitly when critical database dependencies are unavailable.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2024-12-01T10:28:36.011Z

Learnt from: ReneWerner87
Repo: gofiber/fiber PR: 0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Feature request #3224 has been created to add support for square bracket notation and comma-separated values in multipart form data in Fiber, while maintaining binary data transfer capabilities. This would bring parity with the existing form-urlencoded functionality.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2024-10-02T23:03:31.727Z

Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-02T23:03:31.727Z
Learning: Unit tests in this project use testify require.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2024-10-08T19:06:06.583Z

Learnt from: luk3skyw4lker
Repo: gofiber/storage PR: 1342
File: clickhouse/clickhouse_test.go:138-160
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `Test_Reset` function in `clickhouse/clickhouse_test.go` already includes a verification step to ensure the storage is empty after a reset operation by checking that a previously set key returns an empty byte slice.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2024-12-13T08:14:22.851Z

Learnt from: efectn
Repo: gofiber/fiber PR: 3162
File: hooks_test.go:228-228
Timestamp: 2024-12-13T08:14:22.851Z
Learning: In Go test files, prefer using the `require` methods from the `testify` package for assertions instead of manual comparisons and calls to `t.Fatal` or `t.Fatalf`.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2024-10-08T19:06:06.583Z

Learnt from: sixcolors
Repo: gofiber/fiber PR: 2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The project uses the testify/assert package for assertions in unit tests.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2024-07-26T21:00:12.902Z

Learnt from: luk3skyw4lker
Repo: gofiber/storage PR: 1342
File: clickhouse/clickhouse.go:57-58
Timestamp: 2024-07-26T21:00:12.902Z
Learning: The pattern of returning `nil` for empty keys or values is consistent across different storage drivers in the gofiber/storage repository, including Redis, Rueidis, and MongoDB.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2024-07-01T15:47:48.701Z

Learnt from: luk3skyw4lker
Repo: gofiber/storage PR: 1342
File: clickhouse/clickhouse.go:114-115
Timestamp: 2024-07-01T15:47:48.701Z
Learning: The pattern of returning `nil` for empty keys or values is consistent across different storage drivers in the gofiber/storage repository, including Redis, Rueidis, MongoDB, and Clickhouse.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2024-09-26T03:08:50.917Z

Learnt from: sigmundxia
Repo: gofiber/fiber PR: 3151
File: middleware/adaptor/adaptor_test.go:226-230
Timestamp: 2024-09-26T03:08:50.917Z
Learning: In Fiber, the `c.Set` method does not return an error.

Applied to files:

• etcd/etcd_test.go

📚 Learning: 2024-09-25T16:17:00.969Z

Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/session/config.go:16-26
Timestamp: 2024-09-25T16:17:00.969Z
Learning: In the session middleware `Config` struct, `Store` is backed by `fiber.Storage`; they are different entities serving distinct purposes in session management.

Applied to files:

• etcd/etcd_test.go

🔇 Additional comments (2)

etcd/etcd_test.go (2)

59-213: Per-test store initialization with newTestStore looks solid

The refactor to create a fresh *Storage via newTestStore(t) in each test, with defer testStore.Close(), gives good isolation and keeps the test code consistent and readable. The context-cancellation tests and TTL test still exercise the intended behaviors correctly against the containerized etcd backend.

---

217-295: Benchmark harness reuse and per-benchmark cleanup are well-structured

The singleton benchmarkStore with sync.Once plus per-benchmark keys and explicit key deletion after each benchmark nicely balance reuse (for stable timings) with state cleanup. The comment block warning about non–thread-safety and reuse is clear. No functional issues from the changes here.

---

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.}

[mdelapenya]

I see the benchmarks do fail with a massive degradation 🤔 Do you think it's because of the etcd cluster? I verified the containers are started before the the benchmarks code, so they should not interfere, right?

[gaby]

@mdelapenya @ReneWerner87 The bitnami image was faster for some reason, and it also supports semver.

https://hub.docker.com/r/bitnami/etcd

[ReneWerner87]

Ok then let's take this

[mdelapenya]

I'll take a look, as I'm not sure if the testcontainers module supports the image. I'm on PTO until Monday, will check it then 🙏

[gaby]

@mdelapenya My guess is that a single etcd is more performant than a cluster in this case.

The cluster is adding network latency, replication, etc

[mdelapenya]

We have just merged a PR fixing 1-node clusters in etcd: https://github.com/testcontainers/testcontainers-go/pull/3149

I think we can create a tc-go release soon to address that. I'm putting this PR as draft until then

[ReneWerner87]

Currently blocked from the PR and release of the testcontainer or ?

[ReneWerner87]

@mdelapenya can we continue with this?

[mdelapenya]

I need to release tc-go with the fix for etcd 🙏 I'll go back to this once released, which will happen asap

Thanks for the ping @ReneWerner87 🙇

[mdelapenya]

Testcontainers Go v0.38.0 was released yesterday, I'm working on this branch with single cluster support

[mdelapenya]

I'm not sure why there is so much overhead when running the benchmarks... Any idea? I'm pretty sure the numbers are higher for starting a container programatically than compare simply calling docker run in the same shell. But we did not see that in other modules, did't we?

[mdelapenya]

I've run the benchmarks in the main branch, running docker run to start the etcd image:

➜ etcd (main) ✔ docker run -d --name Etcd-server \
            --publish 2379:2379 \
            --publish 2380:2380 \
            --env ALLOW_NONE_AUTHENTICATION=yes \
            --env ETCD_ADVERTISE_CLIENT_URLS=http://etcd-server:2379 \
            bitnami/etcd:latest
Unable to find image 'bitnami/etcd:latest' locally
latest: Pulling from bitnami/etcd
d4f99d431d87: Pull complete 
Digest: sha256:532525457ea850b494cd605c4d04d7f082f3588081a71ce69ccea093b3df30ee
Status: Downloaded newer image for bitnami/etcd:latest
f72b9e5cf7914ff12902cfb1c33aecddd574ccbe07acf9d83a76cdc1acbe1c90
➜ etcd (main) ✔ set -o pipefail;go test ./... -timeout 900s -benchmem -run=^$ -bench . | tee -a output.txt
go: downloading golang.org/x/text v0.23.0
goos: darwin
goarch: arm64
pkg: github.com/gofiber/storage/etcd/v2
cpu: Apple M1 Pro
Benchmark_Etcd_Set-8            	      18	  64125340 ns/op	   14326 B/op	     253 allocs/op
Benchmark_Etcd_Get-8            	      37	  37204883 ns/op	    7776 B/op	     133 allocs/op
Benchmark_Etcd_SetAndDelete-8   	       9	 114569931 ns/op	   21902 B/op	     379 allocs/op
PASS
ok  	github.com/gofiber/storage/etcd/v2	5.309s

And next, did the same for this branch, but multiple times:

➜ etcd (etcd-tc-go) ✗ set -o pipefail;go test ./... -timeout 900s -benchmem -run=^$ -bench . | tee -a output.txt
goos: darwin
goarch: arm64
pkg: github.com/gofiber/storage/etcd/v2
cpu: Apple M1 Pro
Benchmark_Etcd_Set-8            	       4	 303310073 ns/op	   36172 B/op	     344 allocs/op
Benchmark_Etcd_Get-8            	      38	  31038329 ns/op	    7851 B/op	     135 allocs/op
Benchmark_Etcd_SetAndDelete-8   	       9	 199249176 ns/op	   31407 B/op	     418 allocs/op
PASS
ok  	github.com/gofiber/storage/etcd/v2	44.388s

➜ etcd (etcd-tc-go) ✗ set -o pipefail;go test ./... -timeout 900s -benchmem -run=^$ -bench . | tee -a output.txt
goos: darwin
goarch: arm64
pkg: github.com/gofiber/storage/etcd/v2
cpu: Apple M1 Pro
Benchmark_Etcd_Set-8            	       3	 428007931 ns/op	   71349 B/op	     472 allocs/op
Benchmark_Etcd_Get-8            	      37	  31356338 ns/op	    7855 B/op	     135 allocs/op
Benchmark_Etcd_SetAndDelete-8   	       1	1068802084 ns/op	  122144 B/op	     905 allocs/op
PASS
ok  	github.com/gofiber/storage/etcd/v2	21.665s

➜ etcd (etcd-tc-go) ✗ set -o pipefail;go test ./... -timeout 900s -benchmem -run=^$ -bench . | tee -a output.txt
goos: darwin
goarch: arm64
pkg: github.com/gofiber/storage/etcd/v2
cpu: Apple M1 Pro
Benchmark_Etcd_Set-8            	       3	 383360347 ns/op	   43090 B/op	     372 allocs/op
Benchmark_Etcd_Get-8            	      37	  31712941 ns/op	    7858 B/op	     136 allocs/op
Benchmark_Etcd_SetAndDelete-8   	       3	 461405903 ns/op	   78384 B/op	     598 allocs/op
PASS
ok  	github.com/gofiber/storage/etcd/v2	24.615s

I decided to go with a singleton container for benchmarks only. Added comments in the test code for that. PLMK if that's enough as a warning for other contributors when working on benchmarks of the etcd module. 🙏

Results now are:

➜ etcd (etcd-tc-go) ✗ set -o pipefail;go test ./... -timeout 900s -benchmem -run=^$ -bench . | tee -a output.txt
goos: darwin
goarch: arm64
pkg: github.com/gofiber/storage/etcd/v2
cpu: Apple M1 Pro
Benchmark_Etcd_Set-8            	      19	  62740546 ns/op	   14341 B/op	     253 allocs/op
Benchmark_Etcd_Get-8            	      36	  31087274 ns/op	    7768 B/op	     133 allocs/op
Benchmark_Etcd_SetAndDelete-8   	      12	  95558979 ns/op	   21774 B/op	     378 allocs/op
PASS
ok  	github.com/gofiber/storage/etcd/v2	7.199s

[mdelapenya]

They fail on GH:

Benchmark suite	Current: f58844bd3366b81a01b538a91716e331a5bf1635	Previous: aecb06b2659bf2ea06fde8b5bb90ee3b34763c15	Ratio
Benchmark_Etcd_Set	1004915387 ns/op 426976 B/op 2599 allocs/op	1071419 ns/op 14262 B/op 253 allocs/op	937.93
Benchmark_Etcd_Set - ns/op	1004915387 ns/op	1071419 ns/op	937.93
Benchmark_Etcd_Set - B/op	426976 B/op	142[62](https://github.com/gofiber/storage/actions/runs/16344750103/job/46175742910#step:12:63) B/op	29.94
Benchmark_Etcd_Set - allocs/op	2599 allocs/op	253 allocs/op	10.27

I wonder why that huge difference 🤔

Could it be possible that the stored benchmarks are not accurate? The above table is obtained from the GH action for comparing prev Vs current. I'm building the same table but with my local experiment with the main branch (previous comment's values)

Benchmark suite	Current: b602df572336dabe7118a53f0e6e1c87f249bdbc	Previous: f58844bd3366b81a01b538a91716e331a5bf1635	Ratio
Benchmark_Etcd_Set	64125340 ns/op 14326 B/op 253 allocs/op	62740546 ns/op 14341 B/op 253 allocs/op	1.02
Benchmark_Etcd_Set - ns/op	64125340 ns/op	62740546 ns/op	1.02
Benchmark_Etcd_Set - B/op	14326 B/op	14341 B/op	0.99
Benchmark_Etcd_Set - allocs/op	253 allocs/op	253 allocs/op	1

Is it possible to get the baseline benchmarks and repeat them again to verify their accuracy?

[mdelapenya]

BTW I think there is something wrong with the storage benchmarks: https://gofiber.github.io/storage/benchmarks/ Last udpate is from Aug '24. Weird 🤔

[mdelapenya]

I've just rebased this PR, resolving conflicts. I think it will simplify the story with the Bitnami images being removed from Hub.

[mdelapenya]

Test are passing, I'm marking as ready to review. Just waiting for the benchmarks to be finished. There is some research we did before that needs to be addressed.