Combine DB container tests into single module, and improve error handling/display#243
Merged
Conversation
184b690 to
1ce1d9f
Compare
bsideup
reviewed
Nov 18, 2016
|
|
||
| } | ||
|
|
||
| if (!isRunning()) { |
| public void stopAndRemoveContainer(String containerId) { | ||
| stopContainer(containerId, registeredContainers.get(containerId)); | ||
|
|
||
| registeredContainers.remove(containerId); |
Member
There was a problem hiding this comment.
maybe we should do it in finally {} block?
Member
Author
There was a problem hiding this comment.
Actually even though it's a little brittle I'd prefer to keep it that way. If stopContainer fails for some reason, my feeling is that it's slightly better to potentially try again by leaving the containerId in the registered set.
|
|
||
| List<Container> allContainers = dockerClient.listContainersCmd().withShowAll(true).exec(); | ||
|
|
||
| if (allContainers.stream().noneMatch(it -> it.getId().equals(containerId))) { |
Member
There was a problem hiding this comment.
just FYI:
allContainers.stream().map(Container::getId).noneMatch(containerId::equals)
| } | ||
| } | ||
|
|
||
| try { |
Member
There was a problem hiding this comment.
we don't use containerInfo anywhere
4164e57 to
381d9c7
Compare
Closed
…r refactoring of common elements. Adopt MySQL and MariaDB default configuration with lighter resource requirements
…o enable them to be volume mounted into containers.
…path resources that have been extracted from JAR files.
* ensure that liveness `exec` calls don't fire when a container is not running * check that containers exist before trying to clean up in ResourceReaper * filter out common ANSI control code in stderr output * amend frame consumer result callback to suppress errors
…when DB containers are slow to start
a29d49e to
408e983
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR was created on top of #242, and should be merged after that.This change includes what was previously covered by #242: Refactoring of all DB container tests into a single separate module. The idea is that this will make it easier to reduce duplication between tests in the future. Along the way, some reliability issues with DB containers have been worked on:
This also goes through a few places where unnecessary stack traces or errors were indicated during tests. It emerged that some behaviour (container reaper and exec-based liveness checks) were running against stopped or deleted containers, causing loud error logging from docker-java.