CASSSIDECAR-308 CDC: Add end-to-end CDC integration tests#317
CASSSIDECAR-308 CDC: Add end-to-end CDC integration tests#317jyothsnakonisa merged 4 commits intoapache:trunkfrom
Conversation
jmckenzie-dev
left a comment
jmckenzie-dev
left a comment
There was a problem hiding this comment.
Handful of nits. Only non-trivial thing is the version comparison being hard-coded to 4.1 for tests as I think we need >= 4.1 on that comparison.
With that change + the analytics version update it's a 👍🏻 from me.
...ork/src/main/java/org/apache/cassandra/sidecar/testing/SharedClusterIntegrationTestBase.java
Outdated
Show resolved
Hide resolved
integration-framework/src/main/java/org/apache/cassandra/sidecar/testing/TestCdcConfig.java
Outdated
Show resolved
Hide resolved
| * Integration test for CDC functionality. | ||
| * Tests that mutations on CDC-enabled tables are captured and published to TestCdcEventConsumer. | ||
| */ | ||
| public class CdcIntegrationTest extends SharedClusterCdcSidecarIntegrationTestBase |
There was a problem hiding this comment.
This looks like solid "happy path does this actually work" testing. Do we have tests that randomly generate data, form a model of what we would expect the kafka messages to look like from that data, then run through the CDC pipeline and then confirm the end state to the modeled data?
If not - be a good follow up JIRA. :)
There was a problem hiding this comment.
Also - when we go that route we'll probably want to refactor out some shared helpers from this test to use in both.
There was a problem hiding this comment.
Sure, we can add randomized integration tests going forward in follow up JIRAs.
...ionTest/org/apache/cassandra/sidecar/testing/SharedClusterCdcSidecarIntegrationTestBase.java
Show resolved
Hide resolved
...ionTest/org/apache/cassandra/sidecar/testing/SharedClusterCdcSidecarIntegrationTestBase.java
Outdated
Show resolved
Hide resolved
| ServiceConfiguration existingConfig = builder.build().serviceConfiguration(); | ||
| ServiceConfiguration cdcServiceConfig = ServiceConfigurationImpl.builder() | ||
| .host(existingConfig.host()) | ||
| .port(9043) // TODO: Make this port dynamically allocated |
There was a problem hiding this comment.
About this... should we go ahead and do this now? Or, if CI is very stable w/the current, have a follow up JIRA for a helper method that'll find an open port and then use it?
There was a problem hiding this comment.
In cassandra-analytics, we are using port number that is passed in SidecarCdcClient.ClientConfig instead of using the port number from CdcDynamicSidecarInstancesProvider and hence test is not able to use correct port number in sidecar client when dynamic ports are being used for sidecar.
This needs a fix in cassandra-analytics, which I made a note of and this fix will go in next release. Until then, it is good to have an integration test(even with fixed port) to validate several cdc related changes that we are making. We can do a followup JIRA for the test to use dynamic port later.
jyothsnakonisa
force-pushed
the
integration-test
branch
from
bb1ce2d to
aa40359
Compare
jyothsnakonisa
force-pushed
the
integration-test
branch
from
94ed92a to
9d47eb9
Compare
3 participants
Adding Integration test for CDC