fix(EventHubs): Ignore runtime property to support reuse#1644
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds a Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Testcontainers.EventHubs/EventHubsConfiguration.cs`:
- Around line 64-65: Add a regression test in the EventHubs test suite that
constructs two EventHubsConfiguration instances which differ only in their
AzuriteContainer runtime instance (leave ServiceConfiguration identical) and
asserts their reuse signature/hash (the same mechanism used elsewhere to
determine container reuse) is equal, then mutate ServiceConfiguration on one
instance and assert the reuse signature/hash changes; reference the
EventHubsConfiguration type and its AzuriteContainer and ServiceConfiguration
properties in the test to ensure AzuriteContainer is ignored in the signature
while ServiceConfiguration affects it.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Testcontainers.EventHubs/EventHubsConfiguration.cssrc/Testcontainers.EventHubs/Usings.cs
| [JsonIgnore] | ||
| public AzuriteContainer AzuriteContainer { get; } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | head -20Repository: testcontainers/testcontainers-dotnet
Length of output: 613
🏁 Script executed:
fd -t f "EventHubsConfiguration" --max-results 5Repository: testcontainers/testcontainers-dotnet
Length of output: 134
🏁 Script executed:
fd -t d "test|Test" | head -10Repository: testcontainers/testcontainers-dotnet
Length of output: 440
🏁 Script executed:
cat -n "src/Testcontainers.EventHubs/EventHubsConfiguration.cs" | head -80Repository: testcontainers/testcontainers-dotnet
Length of output: 3634
🏁 Script executed:
fd -t d "EventHubs" | grep -i testRepository: testcontainers/testcontainers-dotnet
Length of output: 147
🏁 Script executed:
rg -l "EventHubsConfiguration" --type=cs | grep -i testRepository: testcontainers/testcontainers-dotnet
Length of output: 300
🏁 Script executed:
find tests/Testcontainers.EventHubs.Tests -type f -name "*.cs" | head -20Repository: testcontainers/testcontainers-dotnet
Length of output: 257
🏁 Script executed:
rg "reuse|signature|hash|WithReuse|ResourceHash" tests/Testcontainers.EventHubs.Tests --type=cs -lRepository: testcontainers/testcontainers-dotnet
Length of output: 62
🏁 Script executed:
rg "JsonIgnore|json|JSON" tests/Testcontainers.EventHubs.Tests --type=cs -lRepository: testcontainers/testcontainers-dotnet
Length of output: 62
🏁 Script executed:
rg -l "reuse|signature|hash|WithReuse|ResourceHash" src/Testcontainers.EventHubs --type=csRepository: testcontainers/testcontainers-dotnet
Length of output: 62
🏁 Script executed:
cat tests/Testcontainers.EventHubs.Tests/EventHubsContainerTest.csRepository: testcontainers/testcontainers-dotnet
Length of output: 4556
🏁 Script executed:
rg "reuse|signature|hash|WithReuse|ResourceHash" src --type=cs | head -30Repository: testcontainers/testcontainers-dotnet
Length of output: 4560
🏁 Script executed:
rg "JsonIgnore" src --type=cs | head -20Repository: testcontainers/testcontainers-dotnet
Length of output: 1935
🏁 Script executed:
rg -l "reuse|hash.*signature|ResourceHash" tests --type=cs | head -10Repository: testcontainers/testcontainers-dotnet
Length of output: 388
🏁 Script executed:
rg "WithReuse|ResourceReuse" tests --type=cs -A 5 | head -40Repository: testcontainers/testcontainers-dotnet
Length of output: 3910
🏁 Script executed:
find src -name "*ResourceConfiguration*" -o -name "*ReuseHash*" | xargs catRepository: testcontainers/testcontainers-dotnet
Length of output: 5873
🏁 Script executed:
cat tests/Testcontainers.Platform.Linux.Tests/ReusableResourceTest.cs | head -100Repository: testcontainers/testcontainers-dotnet
Length of output: 4148
🏁 Script executed:
rg "EventHubsConfiguration|GetReuseHash" tests --type=cs -B 2 -A 5Repository: testcontainers/testcontainers-dotnet
Length of output: 4252
🏁 Script executed:
find tests -name "*Hash*" -o -name "*Signature*" | head -10Repository: testcontainers/testcontainers-dotnet
Length of output: 62
Add a regression test to lock this hashing behavior for EventHubs configurations.
This fix is correct—excluding AzuriteContainer from the reuse hash via [JsonIgnore] aligns with the framework pattern and prevents runtime container state from affecting the signature. However, EventHubs currently lacks specific test coverage verifying that the reuse signature remains stable when only Azurite runtime state changes, while still changing when actual EventHubs config inputs (ServiceConfiguration) change. Add a test case in the EventHubs test suite to assert this behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Testcontainers.EventHubs/EventHubsConfiguration.cs` around lines 64 - 65,
Add a regression test in the EventHubs test suite that constructs two
EventHubsConfiguration instances which differ only in their AzuriteContainer
runtime instance (leave ServiceConfiguration identical) and asserts their reuse
signature/hash (the same mechanism used elsewhere to determine container reuse)
is equal, then mutate ServiceConfiguration on one instance and assert the reuse
signature/hash changes; reference the EventHubsConfiguration type and its
AzuriteContainer and ServiceConfiguration properties in the test to ensure
AzuriteContainer is ignored in the signature while ServiceConfiguration affects
it.
What does this PR do?
Makes the configuration hash of EventHubsContainer stable by removing the AzeriteContainer from the hashed configuration.
Why is it important?
The AzeriteContainer contains runtime information about the database container such as the start time which changes everytime, resulting in an always changing configuration hash, which breaks the Resource Reuse feature for EventHubs containers.
Related issues
How to test this PR
Since this concerns cross-execution tests, an automated test would become quite convoluted. The easiest way to test this is to temporarily modify the configurations on EventHubsCustomAzuriteConfiguration and DatabaseFixture to both use .WithReuse(true) and running the test twice. The first execution should create the containers and the second should reuse them.
Follow-ups
Issue #1642 can be closed after this and #1643 are merged
Summary by CodeRabbit