Expose OracleContainer image name property as public constant

[vpavic]

This makes it easier and safer for user code to refer to image name property. For example:

@BeforeAll
static void setUpClass() {
    Assumptions.assumeTrue(
        TestcontainersConfiguration.getInstance().getProperties().containsKey(OracleContainer.IMAGE_NAME_PROPERTY));
}

[rnorth]

Thanks @vpavic - I think this is pretty sensible. I just wonder if there's a more general solution that we could look at.

#1345 will remove the image pull from the container constructor. This should make it safe to instantiate an OracleContainer even if the image is not available.

We would then have freedom to add some methods to the container class itself - for example, isImageAvailable(). Maybe with an optional boolean argument to prevent pulling of the image, if that's desired.

You could then make your assumption be: assumeTrue(myOracleContainer.isImageAvailable()), reducing the amount of 'knowledge' that's needed to find out whether the image is available.

WDYT?

[kiview]

@rnorth Maybe I am missing something, but I thought this is not the point of @vpavic.

I thought about a use case like:

if (!TestcontainersConfiguration.getInstance().getProperties()
  .containsKey(OracleContainer.IMAGE_NAME_PROPERTY)) {

  TestcontainersConfiguration.getInstance()
    .updateGlobalConfig(OracleContainer.IMAGE_NAME_PROPERTY, "myTestingImage");
}

Not sure though? 😅

[vpavic]

Thanks for the feedback @rnorth.

Yes, @kiview is right - my idea was simply to support a safer way of verifying presence of oracle.container.image Testcontainers property.

The background is that in Spring Session we have an Oracle integration test that is conditional on presence of this property. This is needed due to lack of public Docker images for Oracle database so the test only runs when explicitly enabled.

[bsideup]

@kiview the code you linked should never be used, actually :D
updateGlobalConfig is semi-internal API, but we had to call it from another package. Setting environment properties imperatively is not a good idea, since it will be shared between projects

[bsideup]

I agree with @rnorth that it would be better to not rely on internal details of the container (property) but on the behaviour, with maybe a helper method.

For now, one can attempt to instantiate a container and catch the exception

[vpavic]

@bsideup Note that instantiating container is exactly what I prevent in this case, hence the use of this check in @BeforeAll.

[vpavic]

Any further feedback on this? To clarify a bit more, as can be seen from the example in the opening comment, I'm preventing the entire test class from being set up, and would like to do it in a less fragile way, referencing the constant (or possibly something else, but it has to be static).

So any helper method would need to be static to be an option for this use case.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this.

[stale]

This issue has been automatically closed due to inactivity. We apologise if this is still an active problem for you, and would ask you to re-open the issue if this is the case.

[vpavic]

Could this be reopened please?