[junit-jupiter] Add parallel attribute to @Testcontainers
#6658
Conversation
|
Hi, this is my first contribution to the project. If you can review and leave some comments I would be happy to address them. @eddumelendez @kiview Thanks |
There was a problem hiding this comment.
Thanks for your contribution, @samed-bicer ! Left some comments.
...es/junit-jupiter/src/main/java/org/testcontainers/junit/jupiter/TestcontainersExtension.java
Outdated
Show resolved
Hide resolved
...es/junit-jupiter/src/main/java/org/testcontainers/junit/jupiter/TestcontainersExtension.java
Outdated
Show resolved
Hide resolved
samed-bicer
force-pushed
the
junit-parallel-container-startup
branch
from
9a5f69e
to
6b875c2
Compare
...es/junit-jupiter/src/main/java/org/testcontainers/junit/jupiter/TestcontainersExtension.java
Outdated
Show resolved
Hide resolved
...es/junit-jupiter/src/main/java/org/testcontainers/junit/jupiter/TestcontainersExtension.java
Outdated
Show resolved
Hide resolved
Thanks for the review comments, applied them! Should I also update the documentation? |
...les/junit-jupiter/src/test/java/org/testcontainers/junit/jupiter/ParallelExecutionTests.java
Outdated
Show resolved
Hide resolved
...les/junit-jupiter/src/test/java/org/testcontainers/junit/jupiter/ParallelExecutionTests.java
Outdated
Show resolved
Hide resolved
...les/junit-jupiter/src/test/java/org/testcontainers/junit/jupiter/ParallelExecutionTests.java
Outdated
Show resolved
Hide resolved
|
Thanks! I think we should add docs at the end of https://www.testcontainers.org/quickstart/junit_5_quickstart/ Once this PR is merged, then we can add the docs in a separate PR |
eddumelendez
changed the title
@Testcontainers
| if (isParallelExecutionEnabled(context)) { | ||
| Startables.deepStart(restartContainers.stream().map(adapter -> adapter.container)).join(); | ||
| } else { | ||
| restartContainers.forEach(adapter -> store.getOrComputeIfAbsent(adapter.getKey(), k -> adapter.start())); | ||
| } |
There was a problem hiding this comment.
One more thing, this can be replaced by calling startSharedContainers(), of course, the name should change. Also, in order to prevent to get this evaluated every single time even when there is no containers, a check can be added. WDYT?
There was a problem hiding this comment.
I agree; there is no need for duplication, will rename the method as startContainers()
For the check you mentioned is it OK to put a guard clause that checks the size of the given container list? something like
if (restartContainers.isEmpty()) { return; }
There was a problem hiding this comment.
renamed the function and added the guard clause
|
Thanks for your contribution, @samed-bicer ! This is now merged in |
#5037 add an option to Testcontainers annotation for JUnit to start containers in parallel