Avoid duplicate /dev/shm bind when reusing BrowserWebDriverContainer

[kdelay]

What

Reusing a BrowserWebDriverContainer (stop, then start again) fails on Linux with:

com.github.dockerjava.api.exception.BadRequestException: Status 400: {"message":"Duplicate mount point: /dev/shm"}

Why

configure() runs on every start(). On non-Windows hosts it appended a /dev/shm bind to the container binds unconditionally. When the same container instance is started more than once (reuse), the bind was added again, producing a duplicate mount point that Docker rejects at create time.

Change

Only add the /dev/shm bind when one is not already present. Applied to both the org.testcontainers.selenium.BrowserWebDriverContainer and its deprecated org.testcontainers.containers.BrowserWebDriverContainer counterpart, which shared the same logic.

Verification

Added BrowserWebDriverContainerReuseTest which calls configure() twice (as a reused container would) and asserts a single /dev/shm bind. It fails on the current code (2 binds) and passes with this change. No Docker daemon is required for the test, as it only exercises the container configuration.

./gradlew :testcontainers-selenium:spotlessCheck :testcontainers-selenium:compileJava :testcontainers-selenium:test --tests "org.testcontainers.selenium.BrowserWebDriverContainerReuseTest" is green.

Fixes #11941

[duoduobingbing]

@kdelay Doesn't that test need a safeguard so that it does not run on Windows. E.g. JUnit's @DisabledOnOs(OS.WINDOWS) at the test?

[kdelay]

Good catch, thanks. Done in f66bd9e - added @DisabledOnOs(OS.WINDOWS) to the test, since the Windows branch of configure() does not add the /dev/shm bind and the assertion would otherwise fail there.

[renechoi]

The fix looks right to me on the mechanism: binds is the only accumulating structure configure() touches (exposedPorts is a LinkedHashSet and the env vars are a Map, so those are already idempotent across repeated configure() calls), and keying the guard on the container-side volume path is the correct axis, since that is exactly what Docker rejects as a duplicate mount point.

One coverage gap, though. The regression test only covers org.testcontainers.selenium.BrowserWebDriverContainer, but the change also patches the deprecated org.testcontainers.containers.BrowserWebDriverContainer — which is the class the issue report actually points at (BrowserWebDriverContainer.java#L224). That second hunk is currently unguarded: if I revert only the deprecated class back to a plain else and leave everything else on this branch untouched, BrowserWebDriverContainerReuseTest still passes. The only other test that touches that class, SeleniumStartTest, starts each container once, so it does not exercise reuse either.

Mirroring the test into org.testcontainers.containers closes it — that test source package already exists in the module:

// modules/selenium/src/test/java/org/testcontainers/containers/BrowserWebDriverContainerReuseTest.java
package org.testcontainers.containers;

import com.github.dockerjava.api.model.Bind;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.OS;
import org.testcontainers.utility.DockerImageName;

import static org.assertj.core.api.Assertions.assertThat;

class BrowserWebDriverContainerReuseTest {

    private static final DockerImageName CHROME_IMAGE = DockerImageName.parse("selenium/standalone-chrome:4.13.0");

    @Test
    @DisabledOnOs(OS.WINDOWS)
    void configureDoesNotAddDuplicateShmBindOnReuse() {
        BrowserWebDriverContainer<?> container = new BrowserWebDriverContainer<>(CHROME_IMAGE);

        // configure() runs on every start(), so a reused container that is started
        // more than once must not accumulate duplicate /dev/shm binds (see #11941).
        container.configure();
        container.configure();

        long shmBinds = container
            .getBinds()
            .stream()
            .map(Bind::getVolume)
            .filter(volume -> "/dev/shm".equals(volume.getPath()))
            .count();

        assertThat(shmBinds).isEqualTo(1);
    }
}

I ran this locally against f66bd9e with the JDK 17 toolchain. With the deprecated hunk reverted it fails expected: 1L but was: 2L while the existing selenium test stays green; with the fix in place both pass, and :testcontainers-selenium:spotlessCheck is clean.

Minor nit, take it or leave it: the image tag here is selenium/standalone-chrome:4.10.0, while the other selenium tests use 4.13.0. Nothing is pulled in this test so it makes no functional difference, purely consistency.

Do the maintainers want the deprecated class covered as well, or is leaving it test-free deliberate given it is on the way out?