Check for a label alongside of the server version#25
Conversation
| .as("Docker Client is configured via the Testcontainers desktop app") | ||
| .satisfiesAnyOf( | ||
| dockerString -> assertThat(dockerString).contains("Testcontainers Desktop"), | ||
| dockerString -> assertThat(dockerString).contains("testcontainerscloud") |
There was a problem hiding this comment.
That shouldn't be possible anymore, right?
There was a problem hiding this comment.
Well, I was coding it to support a gradual rollout, since we probably want to update the examples before we ship the new change.
But yes, once rolled out, it should just be assert that the docker cloud label exists.
There was a problem hiding this comment.
satisfiesAnyOf will hit Testcontainers Desktop all the time, as we didn't remove that part from TCD, so it makes no sense even for a gradual rollout.
|
|
||
| boolean isCloudServer = labels != null && Arrays.asList(labels).contains("docker/cloud"); | ||
|
|
||
| if (!isCloudServer) { |
There was a problem hiding this comment.
It's generally a bad practice to have ifs in tests. Perhaps, we can design it as multiple tests and just skip those not matching preconditions?
There was a problem hiding this comment.
It was mostly an intermediate state situation while we are rolling the change out. But yes, I'm open to suggestions.
There was a problem hiding this comment.
Even for the intermediate step, it can be confusing. Also, it shares a bad message about our ways of doing tests this way. I'd vote for having separate tests for TCD connection and another for TCC at the end. But I need to experiment a bit with a zone that has the label first
lanwen
force-pushed
the
DRC-30/check-label
branch
from
1506a83 to
6b90c21
Compare
lanwen
changed the title
lanwen
changed the title
2 participants
A proposal for how to tackle the server version transition via the Java example.
This would need to be repeated for: