Implement dependsOn for cross-container dependencies

[bsideup]

No description provided.

[aguibert]

@bsideup instead of depending on containers with the actual Starable instances, could we perhaps introduce some sort of naming mechanism for containers, and then we could simply do dependsOn(String)? This would be nice because we wouldn't have to swap from field-based declarations to a static init block.

An example of what I had in mind:

public class MyTest {

  @Container
  public static GenericContainer a = new GenericContainer(...).withID("a");

  @Container
  public static GenericContainer b = new GenericContainer(...).dependsOn("a");

}

WDYT?

[bsideup]

@aguibert I have a really bad feeling about it.
References give strong guarantees on what user refers, while strings can be anything, they are global (imagine somebody defining some generic name like "kafka" in a big code base?) and in general are less safe.

This would be nice because we wouldn't have to swap from field-based declarations to a static init block.

dependsOn does not rely on the static blocks or anything and works with fields/rules too:

public class MyTest {

  @Container
  public static GenericContainer a = new GenericContainer(...);

  @Container
  public static GenericContainer b = new GenericContainer(...).dependsOn(a);

}

[aguibert]

ah, that code example you posted makes sense. I forgot that was an option because I'm used to not being able to reference other container fields like this:

public class MyTest {

  @Container
  public static GenericContainer a = new GenericContainer(...);

  @Container
  public static GenericContainer b = new GenericContainer(...)
                  .withEnv("A_PORT", a.getFirstMappedPort()); // blows up because we call `getFirstMappedPort()` too early -- before `a` is started and has its port assigned.

}

[rnorth]

Looks good! I have some minor comments but nothing big; feel free to merge when you're satisfied.

[rnorth]

I'm sure there's a good reason, but why do we not propagate a stop() call through the dependencies?

[bsideup]

because there is no guarantee that the dependencies will be a part of the same "group".

We may introduce deepStop in future

[btiernay]

I think a deepStop would be useful for when you know these are the semantics you want. I end up having to manually create one in several projects. Thoughts on adding an API?

Btw, are both stop and start required to be idempotent? I think the current implementation of deepStart depends on this.

[rnorth]

Just eyeballing the code, it looks like we're safe from circular dependencies, and also 'diamond' dependencies should be OK too.

Do you think it might be good to have tests for these scenarios in case a regression is introduced later?

[bsideup]

Circular dependencies won't work (we have "hard depends on")

I will add a test for the diamond case 👍

[rnorth]

Looks great!