Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use testcontainers for multi-broker integration tests and benchmarks #939

Merged

Conversation

seglo
Copy link
Contributor

@seglo seglo commented Oct 17, 2019

Purpose

The sbt-docker-compose plugin is abandoned and difficult to manage. The only reason it was used was to support multi-broker Kafka clusters for integration testing. Now that we can setup equivalent clusters using testcontainers there's no need to include this plugin.

References

Changes

  • Integration Tests: Set minimum in sync replicas for topics to 2 so produced messages get retried when a broker is unexpectedly shutdown
  • Integration Tests: Use takeWhile to signal consumer stream shutdowns.
  • Uses testcontainers-java to spin up Kafka clusters. This required changes to KafkaContainer. I created an upstream PR to add this support to testcontainer-java, but I've copied the implementations into our internal testkit DSL in the meantime.
  • Add TestcontainersKafkaPerClassLike to set testcontainer lifetime to test class
  • Remove sbt-docker-compose and sbt-build-info SBT plugins
  • Testcontainers settings KafkaTestkitTestcontainersSettings

TODO

  • Enable logs from running containers?
  • Compare benchmark test results before and after this change (after merge) No strange difference visible.

@ennru ennru added this to the 2.0.0 milestone Oct 18, 2019
@ennru ennru mentioned this pull request Oct 18, 2019
@seglo seglo force-pushed the seglo/integration-test-hardening-testcontainers branch from f635b02 to bf4a4fd Compare October 18, 2019 19:53
@seglo seglo force-pushed the seglo/integration-test-hardening-testcontainers branch 4 times, most recently from 0e3294f to a83bd42 Compare October 18, 2019 21:26
@seglo seglo force-pushed the seglo/integration-test-hardening-testcontainers branch 7 times, most recently from a9a0348 to 2a42967 Compare October 19, 2019 14:28
@seglo seglo force-pushed the seglo/integration-test-hardening-testcontainers branch from 2a42967 to dd8908a Compare October 19, 2019 14:28
@seglo seglo force-pushed the seglo/integration-test-hardening-testcontainers branch 2 times, most recently from 046191b to 5548ab1 Compare October 20, 2019 01:12
@seglo seglo force-pushed the seglo/integration-test-hardening-testcontainers branch from 5548ab1 to 06f4c24 Compare October 20, 2019 01:39
@seglo seglo changed the title [WIP] Use testcontainers for multi-broker integration tests and benchmarks Use testcontainers for multi-broker integration tests and benchmarks Oct 21, 2019
@seglo seglo requested a review from ennru October 21, 2019 19:49
Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

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

This looks really powerful and much more structured than the former solution with lots of moving parts.

With this competent Testcontainers support, we should create a separate documentation page for it with a proper separation of Scala and Java.

An overview table of the possible testing variants seems necessary now.

build.sbt Outdated Show resolved Hide resolved
docs/src/main/paradox/testing.md Outdated Show resolved Hide resolved
docs/src/main/paradox/testing.md Outdated Show resolved Hide resolved
seglo and others added 4 commits October 22, 2019 08:41
Co-Authored-By: Enno <458526+ennru@users.noreply.github.com>
…ainersSettings.scala

Co-Authored-By: Enno <458526+ennru@users.noreply.github.com>
@seglo seglo requested a review from ennru October 22, 2019 19:58
@seglo seglo force-pushed the seglo/integration-test-hardening-testcontainers branch from 6de8f4b to 06be1b3 Compare October 23, 2019 01:28
Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

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

Great work.
LGTM.
We can follow up with a PR to use Testcontainers for more of our tests so that they run on Scala 2.13 and move some long-running tests to the IntegrationTest scope.

build.sbt Outdated Show resolved Hide resolved
core/src/main/scala/akka/kafka/scaladsl/Committer.scala Outdated Show resolved Hide resolved
core/src/main/scala/akka/kafka/javadsl/Committer.scala Outdated Show resolved Hide resolved
docs/src/main/paradox/testing.md Outdated Show resolved Hide resolved
@seglo seglo force-pushed the seglo/integration-test-hardening-testcontainers branch from f4db0e8 to a2be53e Compare October 23, 2019 14:34
@seglo seglo merged commit 0dd89fc into akka:master Oct 23, 2019
@seglo seglo deleted the seglo/integration-test-hardening-testcontainers branch October 23, 2019 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants