Adding a number of missing features for container management, generated by Claude Code AI

[haster]

Hi there,

we're looking into updating test setup by either upgrading to the latest arquillian-cube (since we use arquillian-cube now), or migrating to testcontainers. We're making a comparison between various scenario's, and one of those scenario's involves using this extension to use testcontainers from our arquillian based tests.

We have quite an extensive test suite and Claude (which we've been using to analyze the various codebases and scenario's) has detected some missing functionality that we would need in order to use testcontainers the way we are now using cube.

We think our testsuite is a useful reallife, extensive test suite and that the realworld issues we run into are broadly applicable/can be used to make arquillian-extension-testcontainers more useful.

This is Claude's summary:

What the extension has today

• @Testcontainer field injection (type-based lookup only)
• @TestcontainersRequired Docker availability check
• TestcontainerRegistry (ClassScoped, accessible to other extensions via @Inject)
• Container lifecycle: start in AfterEnrichment, stop in AfterClass
• LoggingConsumer helper

What it needs (all generic, useful to any Arquillian user) <--- this is roughly what we would be adding

Feature	Why any user needs this	Cube equivalent
Named container lookup	Multiple containers, lookup by role	CubeRegistry.getCube("name")
Container lifecycle events	React to container start/stop (copy files, inject IPs, collect logs)	BeforeCreate, AfterStart, BeforeDestroy
Shared network management	Inter-container communication	NetworkRegistry + CUBE YAML networks
@ArquillianResource integration	Standard Arquillian injection pattern	CubeRegistryProvider
arquillian.xml configuration	Global settings (timeouts, network mode)	Cube's <extension qualifier="docker">

Our questions:
a) Would you be opposed to a largely AI-generated PR adding functionality? We always keep a human in control, and would make sure to have the code be reviewed by a human before opening a PR, but most of the code would probably be AI-generated.
b) Do you agree (in general) with the above summary and features to add or would you leave any out?

To be clear, we are as of now still in the planning/gather information stage, though we tend to move quickly once we make a decision.
We have not yet made a decision so no guarantees as to whether we would implement this or not.

[rhusar]

Hey @haster thanks for a nice write-up. No objection to AI generated PRs but yes, they still need to make sense.

Here is a summary of my discussion with AI:

Named container lookup — This is the most useful addition. Right now if you have two GenericContainer fields you can't distinguish them. Adding a name attribute to @TestContainer and a name-based lookup in TestcontainerRegistry would be straightforward and valuable. Worth doing.

@ArquillianResource integration — Reasonable. A TestcontainerRegistryResourceProvider would let other extensions inject the registry the standard Arquillian way. Small and clean.

Container lifecycle events — This is where I'd push back. Arquillian already has its own event model, and the current AfterEnrichment/AfterClass hooks work well. Adding custom events like BeforeContainerStart/AfterContainerStop adds complexity and coupling. Users who need to react to container start can just use @TestContainer(false) and manage it manually,
or use Testcontainers' own withStartupCheckStrategy/container callbacks. I'd leave this out unless there's a concrete use case that can't be solved otherwise.

Shared network management — Useful but needs careful design. Testcontainers already has Network.newNetwork() — the question is whether the extension should manage network lifecycle or just let users create networks and wire them up. I'd lean toward a lightweight approach: maybe a @TestcontainerNetwork annotation on Network fields with automatic lifecycle,
rather than a full NetworkRegistry.

arquillian.xml configuration — Least compelling. Testcontainers already has its own configuration (.testcontainers.properties, environment variables). Duplicating that into arquillian.xml creates two sources of truth. I'd skip this unless there's a specific setting that only makes sense in the Arquillian context.

A single PR touching all five features would be hard to review. I'd suggest one PR per feature, starting with named lookup + @ArquillianResource since those are the smallest and most clearly useful.

@jamezp @jasondlee @fabiobrz for feedback

[jasondlee]

@rhusar This all sounds good to me. I think the changes suggested in your convo with AI (what a world! :) are good.

Regarding arquillian.xml, I agree with the caution about one source of truth, but requiring that a file in the user/account home directory moves what could be critical test configuration into a file outside of source control, which makes reproducibility a bit trickier, so I think we'll want to consider that one more carefully. It might be that the need for that is small enough that a README and a little extra leg work when setting up new environments is acceptable. I do lean that way, but we'll want to be careful with that.

[voidborne-d]

Drive-by observation from having reviewed a lot of AI-generated PRs recently — @rhusar's "they still need to make sense" is the right bar, but it's hard to hold by feel on a 5-feature PR. A structured pre-review pass on @haster's side before opening could front-load the stuff reviewers would otherwise catch. Four dimensions that consistently surface issues on AI-generated contributions:

1. Scope bloat. Already flagged — 5 features in one PR. @rhusar's "one PR per feature, starting with named lookup + @ArquillianResource" is the single biggest lever. The AI wants to be helpful, so it bundles adjacent features; splitting is the human's job.

2. API-surface justification. Every new public type needs an existence rationale that doesn't reduce to "pattern-matched from Cube." BeforeContainerStart / AfterContainerStop got pushed back on for exactly this reason — the AI proposed a parallel abstraction without verifying that Arquillian's existing event model + Testcontainers' own callbacks couldn't cover the use case. Useful prompt for the author: "for each new public type, name a concrete caller in our own suite that cannot be served by the current API."

3. "Cube equivalent" → "file:line in our suite." The summary table has a helpful Cube equivalent column; adding a second column pointing at the actual Cube usage in @haster's test suite for each row would cleanly separate real requirements from features the AI extrapolated off the Cube docs. Anything without a real-world row gets cut.

4. Config duplication. @jasondlee's concern about arquillian.xml vs .testcontainers.properties is a canonical AI mistake: the model sees both conventions in its training data and generates both. Pick one layer, document the trade-off explicitly — don't let a second source of truth land "because the AI wrote it."

Tests for this kind of extension should skew hard toward failure modes — named container doesn't exist, two fields declare the same name, network fails to create, @Testcontainer(false) + manual start race — not happy-path tests that duplicate the implementation. AI-generated test suites tend to invert this.

(I maintain sober-coding which codifies ~27 of these dimensions as CI checks for this exact situation, if a checklist helps. But the four-point rubric above is more portable than any tool.)

[jasondlee]

I'm inclined to ask for one of two things:

1. This issue is closed, and new issues are opened for each feature requested. It's OK if A blocks B, but each issue should ask for exactly one thing.
2. This request remains open as a parent/tracker, and number 1 is done as well. :)

A multifaceted issue will require a multifaceted PR, and those are hard to review. Insofar as a feature/task is decomposable, I think it should be done here until each request is at a reasonable level of granularity.

@rhusar @jamezp ?

[rhusar]

@jasondlee I think that was assumed that would be the case, this issue would kick off the discussion and then followup issues and PRs would be opened. Otherwise it's not reviewable.

[haster]

Hi all, thanks for your replies and helpful suggestions. I'll try and answer them all.

First, this is very much a real-world scenario. We're not trying to rebuild Cube as Testcontainers, we're migrating our extensive test suite to Testcontainers, and we've identified some missing features that would prevent us from doing so (Claude suggested that it would be easier to build our own Arquillian extension to manage Testcontainers, to which we told it to instead come up with a plan to extend arquillian-extension-testcontainers with the missing features 😁 .)

I think this might be on me, I could have been clearer on this. I think @voidborne-d 's suggestion to add a column pointing back to specific usages in our codebase is a good suggestion to make that explicit, but every row in the table should have at least one such reference.

I also see your suggestions about not combining everything in one PR (funnily enough the AI feedback @rhusar posted suggested "one PR per feature", starting with a PR for the two smallest features 😂 ). I've started work on this last friday, just before your reactions started coming in, and as it stands now the idea is to create 3 PRs.

Reading between the lines, I think you prefer one PR per issue, ie for me to first create an issue per eventual PR, referring back to this issue.

I'll look into the Arquillian lifecycle events, and compare config in arquillian.xml vs testcontainers properties. De define cube containers in arquillian.xml right now, so migrating that to a similar-looking testcontainers configuration seems logical, but the 'one source of truth' is a real concern, as is the 'testsetup in source control' one.

I'll respond in this ticket for overall design things, like adding that column to the comparison table, and then go on and create some more specific smaller issues as anchor for the various PRs.

[rhusar]

@haster I went ahead and implemented 1. – #123 – so now 2 containers of the same type can be named and can coexist. Please have a look if that's what you had in mind.

[haster]

@rhusar At first glance it looks very similar to what I (well, Claude) have created for that issue. I think we have a slightly more extensive test. I'm about to go home now but I'll see about sending you the link tomorrow. I think there's at least one genuine question for maintainers in there.

[rhusar]

@haster Sure, keep it coming. Also, you can submit the extensive tests too (in any way you prefer).

[haster]

After processing your feedback and some parallel internal tracts we've come to the following updated proposal feature table, incorporating @voidborne-d 's suggestion for more specific references to our code.
("KeyHub" below refers to our product, properly called "Topicus KeyHub".)

Short summary: we're moving ahead on named containers/lookup and @ArquillianResource integration, and arquillian.xml changes only for configuration not present in testcontainers.properties. We're deferring the shared network setup slightly until we have a better handle on what exactly we need (might just be a matter of days). We're dropping separate lifecycle events for now and focusing on seeing what we can do with the already-existing events.

Feature	Proposal (effort)	Internal use case (file:line)	Maintainer reply	Updated decision
C1 named lookup	String name() attribute on @Testcontainer; lookup(String name) on TestcontainerRegistry.	AbstractKeyHubSeleniumTest.groovy:392,409 (cubeRegistry.getCube("keyhub-test-app*", DockerCubeInNetwork.class), cubeRegistry.getCube("keyhub-terraform-tester*", DockerCubeInNetwork.class)). arquillian.xml declares 2 Selenium containers (chrome + firefox) sharing the selenium/standalone-* image family — name disambiguation isn't optional.	Approved — most useful addition.	Ship in PR-1 (implemented).
C4 @ArquillianResource	TestcontainerRegistryResourceProvider implements ResourceProvider; standard @ArquillianResource TestcontainerRegistry injection.	AbstractKeyHubSeleniumTest.groovy:83-84 (@ArquillianResource CubeRegistry cubeRegistry;), AbstractBaseKeyHubSeleniumTest.groovy:70-71 (@ArquillianResource CubeRegistry cubes;). 19 test files in keyhub inject CubeRegistry via @ArquillianResource today; the same shape needs to work post-migration.	Approved — small and clean.	Combined with C1 in PR-1 (implemented).
C2 lifecycle events	ContainerStartedEvent + ContainerStoppingEvent fired in TestContainersObserver during start/stop; documented precedence model.	WildFlySetup.java:196,218 (@Observes BeforeCreate for Selenium and WildFly cert/config prep), :273 (@Observes AfterStart for loadAutoExplain), :290 (@Observes BeforeStart for yourkit-agent copy), :306,343 (@Observes BeforeDestroy for coverage and log capture — possibly obsolete after parallel changes; verify at Phase 2c start), :479 (@Observes AfterCreate for cert installation). Real use cases exist; they don't prove that event objects are the right abstraction.	Push back — use @Testcontainer(false) + manual orchestration, or testcontainers' own callbacks.	Drop from upstream request.
C3 shared network	Class-scoped shared Network; boolean network() on @Testcontainer; injectable Network for inter-container communication.	Docker.java:80,110 (@Observes(precedence=10000) CubeDockerConfiguration → assignFreeNetwork() allocates a random /28 subnet to avoid Docker pool exhaustion under 'Docker-in-Docker' + parallel testrunners + cross-class container reuse). DockerCubeInNetwork.java (entire 147-loc class) is the network-aware container wrapper exposing getAccessibleIp/Port.	Useful but careful design needed; lean toward declarative @TestcontainerNetwork over a full NetworkRegistry/SPI.	Defer; keyhub-side initially.
C5 arquillian.xml config	TestcontainersConfiguration reading <extension qualifier="testcontainers">; global settings (default timeouts, network mode, etc.).	arquillian.xml:18-20 (<extension qualifier="cube"><property name="connectionMode">STARTANDSTOP</property></extension>) shows that keyhub already concentrates Arquillian-extension knobs in arquillian.xml. The single property dockerRequiredExceptionClass doesn't have a .testcontainers.properties equivalent — testcontainers itself has no concept of "abort the test class with this Throwable type when Docker is unavailable"; that's a property of the Arquillian extension's @TestcontainersRequired mechanism. The chosen layer is arquillian.xml; trade-off explicit in the PR description.	Skeptical — risk of duplicating testcontainers' own properties.	Standalone PR-2 with focused framing.

[voidborne-d]

Thanks for the ping @haster. The per-feature table with file:line references is exactly the shape that makes reviewer pushback concrete — C1+C4 land cleanly because the cubeRegistry.getCube("keyhub-test-app*", ...) call sites in AbstractKeyHubSeleniumTest.groovy:392,409 are unambiguous and map 1:1 to the proposed API; the C2 walk-back is well-reasoned (the BeforeCreate/AfterStart/BeforeDestroy cluster in WildFlySetup.java does prove use cases exist but doesn't prove events are the right abstraction, and @Testcontainer(false) + manual orchestration is a lower-coupling alternative). Splitting C5 (dockerRequiredExceptionClass + arquillian.xml) into a standalone PR-2 is the right call — the duplication-vs-testcontainers-properties question is its own design conversation and shouldn't gate the PR-1 merge. No further input from me on scope; happy to look at PR-1 once it's up if you'd like a fresh pair of eyes.