Skip to content

Conversation

@botflux
Copy link
Contributor

@botflux botflux commented Jan 27, 2025

Hey, here is a PR to implement the request of #850.
I've implemented the following behavior:

  • .withAutoRemove(false) disables container's auto removal
  • .stop({ remove: true }) overrides the .withAutoRemove(false), so the container is removed
  • By default, containers are automatically removed

I've also added sections about the new behaviors close to existing sections about .stop({ remove: false }).

On the other hand, I've noted that docker prefixes container name with a slash, meaning that the test I wrote would fail:

  it("should stop but not remove the container", async () => {
    const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14")
      .withName(`container-${new RandomUuid().nextUuid()}`)
      .withAutoRemove(false)
      .start();

    await container.stop();

    expect(await getRunningContainerNames()).not.toContain(container.getName()); // never fails because container is prefixed
    expect(await getStoppedContainerNames()).toContain(container.getName()); // fails because `container.getName()`' result is prefixed with '/'.
  });

Here is the output of jest:

Summary of all failing tests
 FAIL  packages/testcontainers/src/generic-container/generic-container.test.ts (90.581 s)
  ● GenericContainer › should stop but not remove the container

    expect(received).toContain(expected) // indexOf

    Expected value: "/container-4e957eea11e6"
    Received array: ["container-4e957eea11e6", "testcontainers-22930a5025fc-service_2-1", "testcontainers-686a91976fd7-service_1-1", "testcontainers-686a91976fd7-service_2-1", "container-797b0e580f81", "testcontainers-a1999481affa-service_2-1", "testcontainers-39d1bd573914-service_1-1", "testcontainers-39d1bd573914-service_2-1", "goofy_aryabhata", "silly_hofstadter", …]

      530 |
      531 |     expect(await getRunningContainerNames()).not.toContain(container.getName());
    > 532 |     expect(await getStoppedContainerNames()).toContain(container.getName());
          |                                              ^
      533 |   });
      534 |
      535 |   it("should stop and override .withAutoRemove", async () => {

      at Object.<anonymous> (src/generic-container/generic-container.test.ts:532:46)


Test Suites: 1 failed, 44 passed, 45 total
Tests:       1 failed, 3 skipped, 323 passed, 327 total
Snapshots:   0 total
Time:        114.873 s
Ran all test suites.

To fix the test I've removed the leading '/'

    const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14")
      .withName(`container-${new RandomUuid().nextUuid()}`)
      .withAutoRemove(false)
      .start();

    await container.stop();

    expect(await getRunningContainerNames()).not.toContain(container.getName().replace("/", ""));
    expect(await getStoppedContainerNames()).toContain(container.getName().replace("/", "")); // passes

This does feel more like a workaround rather than a proper solution, and I'm open to explanations/better solutions.

EDIT: The history contains previous PR commits, so I reopened a new PR with a clean history. Sorry about the notifications.

@netlify
Copy link

netlify bot commented Jan 27, 2025

Deploy Preview for testcontainers-node ready!

Name Link
🔨 Latest commit 16f2ec7
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-node/deploys/679752b8923ee00008be79cd
😎 Deploy Preview https://deploy-preview-904--testcontainers-node.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@botflux botflux closed this Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant