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

Add integration tests for Consumer, Producer and Streams #67

Merged
merged 2 commits into from
Dec 6, 2024

Conversation

mimaison
Copy link
Contributor

@mimaison mimaison commented Nov 22, 2024

This is adding integration tests for running the metrics reporter in consumers, producer and streams applications.

@mimaison mimaison force-pushed the integration-tests branch 2 times, most recently from e7be3d7 to 5aa8ac9 Compare November 29, 2024 15:14
@mimaison mimaison changed the title Add integration tests for Admin, Consumer, Producer and Streams Add integration tests for Consumer, Producer and Streams Nov 29, 2024
@mimaison mimaison added this to the 0.2.0 milestone Nov 29, 2024
@mimaison mimaison marked this pull request as ready for review November 29, 2024 19:58
@see-quick see-quick requested review from see-quick, im-konge and a team November 30, 2024 09:10
Copy link
Member

@see-quick see-quick left a comment

Choose a reason for hiding this comment

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

Looks good just two minor nits to consider.

public class TestUtils {

private static final String VERSION = "1.0.0-SNAPSHOT";
private static final String CLIENTS_IMAGE = "quay.io/strimzi-test-clients/test-clients:latest-kafka-3.9.0";
Copy link
Member

Choose a reason for hiding this comment

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

Maybe extract the Kafka version here to separate variable?

public void setUp() throws Exception {
broker = new StrimziKafkaContainer()
.withKraft()
.withNetworkAliases("kafka");
Copy link
Member

Choose a reason for hiding this comment

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

Extract this network kafka alias into a TestUtils so it can be reused across all integration tests.

Signed-off-by: Mickael Maison <[email protected]>
@Frawless
Copy link
Member

Frawless commented Dec 6, 2024

@mimaison I don't see the tests running in azure. Is that intended from some reason?

@Frawless
Copy link
Member

Frawless commented Dec 6, 2024

I can see the new tests (io.strimzi.kafka.metrics.integration) running in both pipelines:

* Java 11: https://dev.azure.com/cncf/strimzi/_build/results?buildId=182087&view=logs&jobId=2de397de-21fc-5e8b-587d-4865bafb3e06&j=441a6687-78c7-548e-9324-bade41cb68f6&t=cce61017-8585-548c-ef2e-c6a3bc420f66

* Java 17: https://dev.azure.com/cncf/strimzi/_build/results?buildId=182087&view=logs&jobId=2de397de-21fc-5e8b-587d-4865bafb3e06&j=2de397de-21fc-5e8b-587d-4865bafb3e06&t=d9c95e40-2ff1-577f-4ff1-10fe2890ceac

Am I missing something?

ah sorry, I forgot that ctrl + f doesn't work within azp log. 🤦

@mimaison mimaison merged commit 8263db2 into strimzi:main Dec 6, 2024
7 checks passed
@mimaison mimaison deleted the integration-tests branch December 6, 2024 12:28
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.

3 participants