storage icon indicating copy to clipboard operation
storage copied to clipboard
verified publisher icon gofiber

chore: adopt testcontainers-go for Postgres, MySQL, MSSQL and MongoDB

Open mdelapenya opened this issue 1 year ago • 22 comments

What does this PR do?

  • chore: use testcontainers-go in mongodb store
  • chore: use testcontainers-go in mssql store
  • chore: use testcontainers-go in mysql store
  • chore: use testcontainers-go in postgres store

It uses testcontainers-go in the integration tests for the given stores, refactoring the tests to always consume a new test store, backed by a Docker container.

WHy is it important?

Reproducibility: any developer will be able to run the tests locally exactly equals as in the Github Actions.

Related issues

For reference, please check out previous work for Couchbase and Minio (#1508) and ClickHouse (#1506)

There is more ongoing work, as defined in #1507

Summary by CodeRabbit

  • New Features

    • Updated testing workflows to utilize the latest Docker images for MongoDB, MSSQL, MySQL, PostgreSQL, ClickHouse, Couchbase, and MinIO.
    • Introduced new environment variables for improved configurability of database images during testing.

  • Bug Fixes

    • Enhanced test reliability by ensuring each test case initializes its own database connection, preventing state leakage.

  • Refactor

    • Streamlined test setup processes by replacing static store variables with dynamic initialization functions across all database tests.
    • Added cleanup procedures in test functions to manage resources effectively.

mdelapenya avatar Aug 29 '24 12:08 mdelapenya

[!IMPORTANT]

Review skipped

Review was skipped due to path filters

:no_entry: Files ignored due to path filters (7)
  • .github/workflows/benchmark.yml is excluded by !**/*.yml
  • .github/workflows/test-clickhouse.yml is excluded by !**/*.yml
  • .github/workflows/test-couchbase.yml is excluded by !**/*.yml
  • .github/workflows/test-minio.yml is excluded by !**/*.yml
  • .github/workflows/test-mongodb.yml is excluded by !**/*.yml
  • .github/workflows/test-mysql.yml is excluded by !**/*.yml
  • .github/workflows/test-postgres.yml is excluded by !**/*.yml

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This pull request refactors the test setups across multiple storage backends by replacing static initialization of test stores with dynamic instantiation using Testcontainers. The changes add new functions—primarily newTestStore (and in one case mustStartMySQL)—to create containerized database instances for PostgreSQL, MongoDB, MySQL, and others. Additional modifications include enhanced resource cleanup, wait strategies, updated image versions, and minor adjustments to error handling and assertions in various test files.

Changes

File(s) Change Summary
postgres/..._test.go, mongodb/..._test.go, mysql/..._test.go Introduced new functions (newTestStore and mustStartMySQL) for dynamic test store initialization using Testcontainers; added configuration constants and replaced static test store instances to ensure fresh containerized environments for each test.
clickhouse/..._test.go, couchbase/..._test.go, minio/..._test.go Enhanced test cases with container management improvements: added Testcontainers imports, cleanup calls (e.g., testcontainers.CleanupContainer), deferred closure of test stores, updated wait strategies, and, for ClickHouse, added a dedicated test verifying client closure.
neo4j/..._test.go, s3/init_test.go Modified the TestMain functions to include wait strategies ensuring container readiness—using port checks, log messages, and HTTP health checks—so that the respective services (Neo4j and MinIO) are fully operational before tests proceed.

Sequence Diagram(s)

sequenceDiagram
    participant T as Test Case
    participant N as newTestStore
    participant C as Testcontainers
    participant S as Storage Instance

    T->>N: Call newTestStore(t)
    N->>C: Request container startup
    C->>N: Signal container readiness (ports, health checks)
    N->>T: Return new Storage instance
    T->>S: Execute test operations
    S-->>T: Cleanup on test completion

Possibly related PRs

  • gofiber/storage#1506 – Integrates Testcontainers for dynamic test environment setup in ClickHouse tests.
  • gofiber/storage#1508 – Enhances Testcontainers-based initialization for Couchbase and MinIO, paralleling the modifications in this PR.
  • gofiber/storage#1507 – Proposes adopting Testcontainers for integration testing across various storage layers, including PostgreSQL.

Suggested labels

🧹 Updates

Suggested reviewers

  • sixcolors
  • gaby
  • ReneWerner87
  • efectn

Poem

I’m a rabbit with a hop so keen,
Containers now rule every test scene.
Fresh stores spin up with a flick of code,
Bugs retreat down the winding road.
With cleanup and checks all shining bright,
I nibble on success deep into the night!
🐰✨

[!NOTE]

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Aug 29 '24 12:08 coderabbitai[bot]

For some reason, the memcache service is failing in the benchmark job. Any guess?

mdelapenya avatar Aug 29 '24 14:08 mdelapenya

sorry overlooked, that's really strange, I'll have to take a look at it in the next few days

ReneWerner87 avatar Sep 11 '24 12:09 ReneWerner87

@ReneWerner87 any update on the benchmark thing? 🙏 Once merge, I can send a new PR with more stores

mdelapenya avatar Sep 13 '24 10:09 mdelapenya

unfortunately haven't had time to look yet, will have a look on sunday and fix it then, but feel free to make pull requests for the other stores if you like

in this case we only have to apply the fix to the other PR

ReneWerner87 avatar Sep 13 '24 13:09 ReneWerner87

@mdelapenya now the reason

you removed the other services

image

now the options from postgres for the health check belongs to memcached

image

but there is no postgres started

image

other question do we have to change the golang versions in the test-*.yml's and go.mod files? then we would cause a breaking change as in the previous PR request https://github.com/gofiber/storage/pull/1508#issuecomment-2316937490

ReneWerner87 avatar Sep 15 '24 17:09 ReneWerner87

do we have to change the golang versions in the test-*.yml's and go.mod files? then we would cause a breaking change as in the previous PR request https://github.com/gofiber/storage/pull/1508#issuecomment-2316937490

We changed because otherwise the store project won't compile, failing with:

Error: ../../../../go/pkg/mod/github.com/docker/[email protected]+incompatible/pkg/archive/archive.go:784:24: undefined: strings.CutPrefix
FAIL	github.com/gofiber/storage/mssql/v2 [build failed]

This means that the docker dependency added by testcontainers would need Go 1.20 (please see https://pkg.go.dev/strings#CutPrefix). So, if you agree, I'm keeping the following Go matrix:

                go-version:
                    - 1.20.x
                    - 1.21.x
                    - 1.22.x

And regarding the go.mod files, whatever you prefer (keeping 1.19 or bumping to 1.21) is fine for me.

mdelapenya avatar Sep 16 '24 11:09 mdelapenya

ok didn´t know

then update the go.mod to 1.20 and don´t forget to remove the "options" line in benchmark.yaml

ReneWerner87 avatar Sep 16 '24 12:09 ReneWerner87

@ReneWerner87 I noticed that if we keep Go 1.20 in the go.mod, then the build will complain about mod tidy to be needed. Do you want me to add a step in the pipeline for mod tidy, or simply bump the Go version to 1.21 (as we already did for the clickhouse module).

mdelapenya avatar Sep 16 '24 12:09 mdelapenya

Ok, 1.21 is ok for me

ReneWerner87 avatar Sep 16 '24 14:09 ReneWerner87

Couchbase benchmark is passing for me locally 🤔 Any clue?

mdelapenya avatar Sep 16 '24 19:09 mdelapenya

Will check it tomorrow morning

ReneWerner87 avatar Sep 16 '24 20:09 ReneWerner87

i restarted the benchmarks and now we have a new problem

image

ReneWerner87 avatar Sep 17 '24 06:09 ReneWerner87

taking a look, thanks! 🙏

mdelapenya avatar Sep 17 '24 09:09 mdelapenya

It's weird: they pass locally for me

go test ./... -v -benchmem -run=^$ -bench . | tee -a ../output.txt
goos: darwin
goarch: arm64
pkg: github.com/gofiber/storage/mssql/v2
Benchmark_MSSQL_Set
2024/09/17 15:38:33 github.com/testcontainers/testcontainers-go - Connected to docker: 
  Server Version: 82+testcontainerscloud (via Testcontainers Desktop 1.17.0)
  API Version: 1.46
  Operating System: Ubuntu 22.04.4 LTS
  Total Memory: 15779 MB
  Testcontainers for Go Version: v0.33.0
  Resolved Docker Host: tcp://127.0.0.1:51475
  Resolved Docker Socket Path: /var/run/docker.sock
  Test SessionID: c134e29f4f160c5036506bb926bfd5fef0252e721d3b872da882ac66e699aebd
  Test ProcessID: f85ff9e5-c073-473d-9c5f-10b48aadf30a
2024/09/17 15:38:33 🐳 Creating container for image testcontainers/ryuk:0.8.1
2024/09/17 15:38:33 ✅ Container created: 898ef11dd444
2024/09/17 15:38:33 🐳 Starting container: 898ef11dd444
2024/09/17 15:38:34 ✅ Container started: 898ef11dd444
2024/09/17 15:38:34 ⏳ Waiting for container id 898ef11dd444 image: testcontainers/ryuk:0.8.1. Waiting for: &{Port:8080/tcp timeout:<nil> PollInterval:100ms skipInternalCheck:false}
2024/09/17 15:38:34 🔔 Container is ready: 898ef11dd444
2024/09/17 15:38:34 🐳 Creating container for image mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04
2024/09/17 15:38:34 ✅ Container created: 60c8ace27308
2024/09/17 15:38:34 🐳 Starting container: 60c8ace27308
2024/09/17 15:38:34 ✅ Container started: 60c8ace27308
2024/09/17 15:38:34 ⏳ Waiting for container id 60c8ace27308 image: mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04. Waiting for: &{timeout:<nil> Log:Recovery is complete. IsRegexp:false Occurrence:1 PollInterval:100ms}
2024/09/17 15:38:39 🔔 Container is ready: 60c8ace27308
2024/09/17 15:38:40 🐳 Creating container for image mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04
2024/09/17 15:38:40 ✅ Container created: 16ebc0c630be
2024/09/17 15:38:40 🐳 Starting container: 16ebc0c630be
2024/09/17 15:38:40 ✅ Container started: 16ebc0c630be
2024/09/17 15:38:40 ⏳ Waiting for container id 16ebc0c630be image: mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04. Waiting for: &{timeout:<nil> Log:Recovery is complete. IsRegexp:false Occurrence:1 PollInterval:100ms}
2024/09/17 15:38:45 🔔 Container is ready: 16ebc0c630be
Benchmark_MSSQL_Set-8                          6         191780180 ns/op           99942 B/op        598 allocs/op
Benchmark_MSSQL_Get
2024/09/17 15:38:47 🐳 Creating container for image mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04
2024/09/17 15:38:47 ✅ Container created: dbb74231c3b6
2024/09/17 15:38:47 🐳 Starting container: dbb74231c3b6
2024/09/17 15:38:47 ✅ Container started: dbb74231c3b6
2024/09/17 15:38:47 ⏳ Waiting for container id dbb74231c3b6 image: mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04. Waiting for: &{timeout:<nil> Log:Recovery is complete. IsRegexp:false Occurrence:1 PollInterval:100ms}
2024/09/17 15:38:52 🔔 Container is ready: dbb74231c3b6
2024/09/17 15:38:53 🐳 Creating container for image mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04
2024/09/17 15:38:53 ✅ Container created: 06ded4ac3ae0
2024/09/17 15:38:53 🐳 Starting container: 06ded4ac3ae0
2024/09/17 15:38:54 ✅ Container started: 06ded4ac3ae0
2024/09/17 15:38:54 ⏳ Waiting for container id 06ded4ac3ae0 image: mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04. Waiting for: &{timeout:<nil> Log:Recovery is complete. IsRegexp:false Occurrence:1 PollInterval:100ms}
2024/09/17 15:38:58 🔔 Container is ready: 06ded4ac3ae0
2024/09/17 15:39:00 🐳 Creating container for image mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04
2024/09/17 15:39:00 ✅ Container created: 8e5288e9b2a6
2024/09/17 15:39:00 🐳 Starting container: 8e5288e9b2a6
2024/09/17 15:39:00 ✅ Container started: 8e5288e9b2a6
2024/09/17 15:39:00 ⏳ Waiting for container id 8e5288e9b2a6 image: mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04. Waiting for: &{timeout:<nil> Log:Recovery is complete. IsRegexp:false Occurrence:1 PollInterval:100ms}
2024/09/17 15:39:05 🔔 Container is ready: 8e5288e9b2a6
2024/09/17 15:39:07 🐳 Creating container for image mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04
2024/09/17 15:39:07 ✅ Container created: 63e2053a58eb
2024/09/17 15:39:07 🐳 Starting container: 63e2053a58eb
2024/09/17 15:39:07 ✅ Container started: 63e2053a58eb
2024/09/17 15:39:07 ⏳ Waiting for container id 63e2053a58eb image: mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04. Waiting for: &{timeout:<nil> Log:Recovery is complete. IsRegexp:false Occurrence:1 PollInterval:100ms}
2024/09/17 15:39:12 🔔 Container is ready: 63e2053a58eb
Benchmark_MSSQL_Get-8                          6         191320208 ns/op          109934 B/op        601 allocs/op
Benchmark_MSSQL_SetAndDelete
2024/09/17 15:39:14 🐳 Creating container for image mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04
2024/09/17 15:39:14 ✅ Container created: a17d4139d23b
2024/09/17 15:39:14 🐳 Starting container: a17d4139d23b
2024/09/17 15:39:14 ✅ Container started: a17d4139d23b
2024/09/17 15:39:14 ⏳ Waiting for container id a17d4139d23b image: mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04. Waiting for: &{timeout:<nil> Log:Recovery is complete. IsRegexp:false Occurrence:1 PollInterval:100ms}
2024/09/17 15:39:19 🔔 Container is ready: a17d4139d23b
2024/09/17 15:39:20 🐳 Creating container for image mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04
2024/09/17 15:39:20 ✅ Container created: a33e3ebb2c58
2024/09/17 15:39:20 🐳 Starting container: a33e3ebb2c58
2024/09/17 15:39:21 ✅ Container started: a33e3ebb2c58
2024/09/17 15:39:21 ⏳ Waiting for container id a33e3ebb2c58 image: mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04. Waiting for: &{timeout:<nil> Log:Recovery is complete. IsRegexp:false Occurrence:1 PollInterval:100ms}
2024/09/17 15:39:25 🔔 Container is ready: a33e3ebb2c58
2024/09/17 15:39:27 🐳 Creating container for image mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04
2024/09/17 15:39:27 ✅ Container created: d53d076a7b6c
2024/09/17 15:39:27 🐳 Starting container: d53d076a7b6c
2024/09/17 15:39:28 ✅ Container started: d53d076a7b6c
2024/09/17 15:39:28 ⏳ Waiting for container id d53d076a7b6c image: mcr.microsoft.com/mssql/server:2022-RTM-GDR1-ubuntu-20.04. Waiting for: &{timeout:<nil> Log:Recovery is complete. IsRegexp:false Occurrence:1 PollInterval:100ms}
2024/09/17 15:39:32 🔔 Container is ready: d53d076a7b6c
Benchmark_MSSQL_SetAndDelete-8                 3         412130153 ns/op          195160 B/op       1167 allocs/op
PASS
ok      github.com/gofiber/storage/mssql/v2     61.584s

Is there any way to relaunch the benchmark job, and try to identify where the flakiness is?

mdelapenya avatar Sep 17 '24 13:09 mdelapenya

I already restarted it 2 time

Pls test the workflow with act https://github.com/nektos/act

ReneWerner87 avatar Sep 17 '24 17:09 ReneWerner87

I restarted the benchmark action again with debug info https://github.com/gofiber/storage/actions/runs/10886797247?pr=1515

ReneWerner87 avatar Sep 17 '24 17:09 ReneWerner87

@ReneWerner87 I noticed a regression in the recent update (Azure DevOps agent / GitHub Actions runner): https://github.com/actions/runner-images/issues/10649

I think it could be related 🤔

mdelapenya avatar Sep 19 '24 12:09 mdelapenya

@ReneWerner87 I resolved the MSSQL errors caused by the recent GH workers upgrade, and I'm currently debugging the couchbase failures (https://github.com/gofiber/storage/actions/runs/10950265038/job/30405176292?pr=1515) where it only fails for Go 1.20 🤷 Do you think it could be caused by the number of running containers so the worker could eventually be non-responsive? I'd say that the same happens for the benchmarks. Will update this thread after my research.

Sorry for the inconvenience 🙏

mdelapenya avatar Sep 20 '24 09:09 mdelapenya

Do you think it could be caused by the number of running containers

could be possible

thanks for the effort so far

ReneWerner87 avatar Sep 20 '24 11:09 ReneWerner87

@mdelapenya Did you add "wait_for" to each service? A lot of these services take a bit to start, so it could be that.

gaby avatar Sep 20 '24 12:09 gaby

@mdelapenya Did you add "wait_for" to each service? A lot of these services take a bit to start, so it could be that.

Indeed, we always recommend adding wait strategies for container, so our modules usually come with a wait strategy.

For couchbase we have this: https://github.com/testcontainers/testcontainers-go/blob/main/modules/couchbase/couchbase.go#L189

mdelapenya avatar Sep 20 '24 14:09 mdelapenya

@mdelapenya in mssql I saw that you changed the version to 1.22 in the go.mod, but the test workflow still tests down to 1.20 that means you would have to use this lower version in the go.mod, because it is the minimum

ReneWerner87 avatar Nov 28 '24 12:11 ReneWerner87

same for couchbase

ReneWerner87 avatar Nov 28 '24 12:11 ReneWerner87

@ReneWerner87 thanks for going back to this. I was focused on other topics and left this behind. I can jump on this now.

Before I add more (probably wrong) code, what is the Go version policy for the modules? We collected the values in https://github.com/gofiber/storage/pull/1508#issuecomment-2304549792. This will be important because in older Go versions, we'll find some issues with missing types required by packages using a more modern version.

Could you help me out defining the right version? 🙏

mdelapenya avatar Nov 28 '24 14:11 mdelapenya

https://github.com/gofiber/storage/pull/1508#issuecomment-2316937490

We should leave the versions as they are, otherwise we would have to increase the major version of the respective package and update other things if possible, so that this breaking change was worth it

ReneWerner87 avatar Nov 28 '24 14:11 ReneWerner87

We should leave the versions as they are, otherwise we would have to increase the major version of the respective package and update other things if possible, so that this breaking change was worth it

Ok, makes sense. There would be modules we won't be able to adopt the integration tests as in others. Will take a look, and would close this PR in case it's not possible to do it. Thanks for your work here!

mdelapenya avatar Nov 28 '24 14:11 mdelapenya

It would be possible if we upgrade the major version and no longer support the old golang version But maybe we should only do that when we make the change for version 3

ReneWerner87 avatar Nov 28 '24 14:11 ReneWerner87

Makes sense. I can send this work again once we are in v3. We can close this and create an issue for that.

mdelapenya avatar Nov 28 '24 15:11 mdelapenya

@ReneWerner87 I've added 78da7fc in order to align the modules modified by this PR (couchbase, mongo, mssql, mysql and postgres) in regards of the Go version matrix: all using 1.21 and above, as in the existing minio module.

I have one question regarding the clickhouse module, which runs the tests for 1.21 and 1.22: I guess we still want to do it in 1.23 too, right? I can add it in this PR.

Last thing: the benchmark is failing, and I think it's because the default timeout per test (60s) it's not enough to pull the MSSQL Docker image (2.19GB). Would it be OK to add the -timeout 120s flag to the benchmarks?

mdelapenya avatar Dec 11 '24 13:12 mdelapenya