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

feat: Add KAFKA mocking #13

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

SebastienDegodez
Copy link
Contributor

@SebastienDegodez SebastienDegodez commented Dec 5, 2024

Pull Request

Proposed Changes

The goal of this enhancement is to add KAFKA support to the testcontainers module.

For the moment, AsynchronousAPI mocking are not supported. This feature would require an additionnal container microcks-async-minion.

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • run dotnet test and ensure you have test coverage for the lines you are introducing
  • run dotnet husky run and fix any issues that you have introduced

Reviewer

  • Label as either feature, fix, documentation, enhancement, maintenance or breaking

@SebastienDegodez
Copy link
Contributor Author

In JAVA, @lbroudoux introduce a notion MicrocksContainersEnsemble.
An ensemble allows you to configure and controle the multiple containers with one line.
It's a good choice to increase a developper experience.

But this approach use testcontainer lifecycle Startable, i don't see the similar usage in .NET.

@lbroudoux
Copy link
Member

In JAVA, @lbroudoux introduce a notion MicrocksContainersEnsemble. An ensemble allows you to configure and controle the multiple containers with one line. It's a good choice to increase a developper experience.

But this approach use testcontainer lifecycle Startable, i don't see the similar usage in .NET.

Yes, this approach was quite convenient in Java (and necessary to integrate with frameworks like Spring that can manage the testcontainers lifecycle). However it is typically not available in other Testcontainers bindings like Node or Go.

I think it's nice to keep this notion of MicrocksContainersEnsemble being different from a single MicrocksContainer and allowing reuse of this one + wrapping the other needed container. Also, I think it's important to stick with the idioms of each language/platform and thus the MicrocksContainersEnsemble would have its own ContainerBuilder while implementing DockerContainer even if - behind the scenes - it manages multiple containers. What do you think?

@SebastienDegodez
Copy link
Contributor Author

SebastienDegodez commented Dec 12, 2024

Issue KafkaContainer (.NET) testcontainers/testcontainers-dotnet#1314

@SebastienDegodez
Copy link
Contributor Author

Contribution to add network on Kafka: testcontainers/testcontainers-dotnet#1316

@lbroudoux
Copy link
Member

Contribution to add network on Kafka: testcontainers/testcontainers-dotnet#1316

Hey @SebastienDegodez! I had the same issue with Kafka module on testcontainers Go unfortunately...

@SebastienDegodez
Copy link
Contributor Author

Contribution to add network on Kafka: testcontainers/testcontainers-dotnet#1316

Hey @SebastienDegodez! I had the same issue with Kafka module on testcontainers Go unfortunately...

Hey, I have develop the feature in the train this morning. For the moment, i replace the kafka_listener, protocole and advertised (in startupCallback).

SebastienDegodez and others added 6 commits December 19, 2024 01:17
Signed-off-by: SebastienDegodez <[email protected]>
Signed-off-by: SebastienDegodez <[email protected]>
Bumps [FluentAssertions](https://github.com/fluentassertions/fluentassertions) from 6.12.0 to 7.0.0.
- [Release notes](https://github.com/fluentassertions/fluentassertions/releases)
- [Changelog](https://github.com/fluentassertions/fluentassertions/blob/develop/AcceptApiChanges.ps1)
- [Commits](fluentassertions/fluentassertions@6.12.0...7.0.0)

---
updated-dependencies:
- dependency-name: FluentAssertions
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [xunit](https://github.com/xunit/xunit) from 2.9.0 to 2.9.2.
- [Commits](xunit/xunit@v2-2.9.0...v2-2.9.2)

---
updated-dependencies:
- dependency-name: xunit
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [RestAssured.Net](https://github.com/basdijkstra/rest-assured-net) from 4.4.0 to 4.5.1.
- [Changelog](https://github.com/basdijkstra/rest-assured-net/blob/main/CHANGELOG.md)
- [Commits](basdijkstra/rest-assured-net@rest-assured-net-4.4.0...rest-assured-net-4.5.1)

---
updated-dependencies:
- dependency-name: RestAssured.Net
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
@SebastienDegodez
Copy link
Contributor Author

Hello @lbroudoux , "sonar.token is invalid"

I don't understand. We have lost a secret ?

@lbroudoux
Copy link
Member

lbroudoux commented Dec 19, 2024

Hello @lbroudoux , "sonar.token is invalid"

I don't understand. We have lost a secret ?

I've checked and SONAR_TOKEN is still declared in configuration.

It is used here: https://github.com/microcks/microcks-testcontainers-dotnet/blob/main/.github/workflows/steps.dotnet-build-test.yml#L67

I think the issue you get is because the build is happening on your own branch (your fork) and not on the Microcks one. In that case, the secret we defined is not available. Maybe we should add an additional condition to prevent trying to launch the Sonar analysis? Will check this.

@SebastienDegodez
Copy link
Contributor Author

Hello @lbroudoux , "sonar.token is invalid"

I don't understand. We have lost a secret ?

I've checked and SONAR_TOKEN is still declared in configuration.

It is used here: https://github.com/microcks/microcks-testcontainers-dotnet/blob/main/.github/workflows/steps.dotnet-build-test.yml#L67

I think the issue you get is because the build is happening on your own branch (your fork) and not on the Microcks one. In that case, the secret we defined is not available. Maybe we should add an additional condition to prevent trying to launch the Sonar analysis? Will check this.

I can have a condition but to follow Sonar for all pr, it's not practical. For the future, do you think it is possible to add me the right to create and push only the branch in feature/* without risk for other repo or the main branch (or another strategy) ?

@SebastienDegodez SebastienDegodez force-pushed the main branch 4 times, most recently from f714277 to 4ceb187 Compare December 21, 2024 01:34
@SebastienDegodez SebastienDegodez marked this pull request as draft December 21, 2024 01:36
@SebastienDegodez SebastienDegodez force-pushed the main branch 3 times, most recently from 87049af to 88c1bba Compare December 21, 2024 11:42
@SebastienDegodez SebastienDegodez force-pushed the main branch 11 times, most recently from 85affe5 to bc81b95 Compare December 22, 2024 00:27
Skip SonarCloud Scan if the pull request is from a forked repository

Signed-off-by: SebastienDegodez <[email protected]>
@SebastienDegodez SebastienDegodez marked this pull request as ready for review December 22, 2024 00:47
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.

2 participants