Additional tests for named container lookup#1
Additional tests for named container lookup#1rhusar merged 2 commits intorhusar:named-container-lookupfrom
Conversation
Adds two test additions on top of the named-lookup implementation: - NamedContainerTest: a third field reusing an existing name, asserting that two fields with the same @TestContainer(name) resolve to the same instance. - TestcontainerRegistryTest: plain-JUnit unit tests exercising the registry's lookup semantics directly — same-name same-instance via lookupOrCreate, lookup of an unknown name, and lookup of the empty string against a registry holding only named containers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- TestcontainerRegistry.lookup(String): flip the equals direction so callers passing null get a null result instead of an NPE on a populated registry. The compared `containerDesc.name` is sourced from Testcontainer.name(), which by annotation rules cannot be null. - TestcontainerRegistry.lookup(String, Class<T>): typed overload that casts the looked-up container to the requested type, throwing IllegalArgumentException if the registered container is not assignable to the requested type. Returns null if no container with the given name is registered. Tests cover the null-name path on a populated registry, the typed-lookup happy path including a missing-name null return, and the type-mismatch throw path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@haster I am slightly confused by addition of org.arquillian.testcontainers.TestcontainerRegistry#lookup(java.lang.String, java.lang.Class) which is never used from prod code (only from test code)? The 'convenience API for downstream consumers' - i don't think the org.arquillian.testcontainers.TestcontainerRegistry is obtainable by users. Rest look good. |
|
One of our other changes would make the Registry (or the View, or both, if your PR arquillian#127 drops) injectable via @ArquillianResource It's a convenience based on our current code but not necessary. (I wonder if the extension could use it for injection 🤔 but I can't check that right now). |
rhusar
left a comment
rhusar
left a comment
There was a problem hiding this comment.
LGTM – we will resolve the situation with the new lookup method in the subsequent PR.
|
Thanks @haster |
2 participants
What was left of the code I had locally after taking into account what you had already done on this branch. Do with it what you will, I offer them as suggestions so as not to let the code get lost. They are not necessary for our case.
Two commits:
The 2nd commit is a slight change in your production code. Feel free to pick only the first commit if you so wish.