Skip to content

Conversation

@arthurschreiber
Copy link
Contributor

@arthurschreiber arthurschreiber commented Oct 21, 2024

Description

Unit tests in gh-ost are pretty basic right now. They often only verify simple behaviour like query generation, but fall short of testing more complex behaviours like actually running the changelog applier, because those require an actual MySQL instance to be running, and then might also depend on the exact settings of that instance, which we might not even have control over.

By using testcontainers-go, we can spawn MySQL containers using exactly the settings we want to test.

This adds a few simple test cases to the Applier tests that show how this could work.

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

@arthurschreiber arthurschreiber force-pushed the arthur/use-testcontainers branch from 07749f7 to db9c69a Compare October 21, 2024 13:49
@arthurschreiber arthurschreiber merged commit 8fabcc6 into arthur/use-stretchr-testify Oct 21, 2024
arthurschreiber added a commit that referenced this pull request Oct 21, 2024
Use testcontainers to spawn MySQL server container in unit tests. #1457

func (suite *ApplierTestSuite) SetupSuite() {
ctx := context.Background()
req := testcontainers.ContainerRequest{
Copy link

@mdelapenya mdelapenya Oct 30, 2024

Choose a reason for hiding this comment

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

Hello from Testcontainers Go community! 👋 Thanks for using us

@arthurschreiber I'm here just to suggest using the dedicated MySQL module, if possible: https://golang.testcontainers.org/modules/mysql/

And please let us know if you need anything from our side 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

👋 thanks for the suggestion! We will look into using this.

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.

4 participants