Documentation for StartupCheckStrategy includes non-working code snippet

[nick-jones]

Expected Behaviour https://github.com/testcontainers/testcontainers-node/blob/main/docs/features/wait-strategies.md#other-startup-strategies shows a snippet of code which demonstrates how custom wait strategies can be implemented.

This code is not valid after c97e2942f047814a5d80835402cf2fcf3d944913 introduced a constructor to StartupCheckStrategy, with the expectation that ContainerRuntimeClient is injected.

I would think something like this would perhaps be more suitable?

const client = await getContainerRuntimeClient();
const container = await new GenericContainer("alpine")
  .withWaitStrategy(new ReadyAfterDelayWaitStrategy(client))
  .start();

Actual Behaviour If I follow the documentation as suggested, I'll get an error, since the client is not injected:

TypeError: Cannot read properties of undefined (reading 'container')
    at common_1.IntervalRetry.retryUntil.message (.../node_modules/testcontainers/build/wait-strategies/startup-check-strategy.js:13:134)
    at IntervalRetry.retryUntil (.../node_modules/testcontainers/build/common/retry.js:26:28)
    at ReadyAfterDelayWaitStrategy.waitUntilReady (.../node_modules/testcontainers/build/wait-strategies/startup-check-strategy.js:13:70)
    at waitForContainer (.../node_modules/testcontainers/build/wait-strategies/wait-for-container.js:8:28)
    at GenericContainer.startContainer (.../node_modules/testcontainers/build/generic-container/generic-container.js:140:57)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async .../test.js:15:21

Testcontainer Logs N/A

Steps to Reproduce N/A

Environment Information N/A

[cristianrgreco]

Thanks for pointing this out. IMO it makes sense to default the constructor property as it's always gonna be getContainerRuntimeClient() , then we don't have to update the docs. Would you be willing to raise this PR?

[nick-jones]

I've noted that the constructor argument was removed via https://github.com/testcontainers/testcontainers-node/commit/0bc8c42897e5a1b1c2a5f346ca16b74aa4809231#diff-0dc3fa260946ebb11dcb8e3028cd00770e58df3e2aaa84b7014f4f7ff8307ca7 - so I would say the documentation is (perhaps incidentally) correct again.