testcontainers / testcontainers-node

Testcontainers is a NodeJS library that supports tests, providing lightweight, throwaway instances of common databases, Selenium web browsers, or anything else that can run in a Docker container.
https://testcontainers.com
MIT License
1.92k stars 191 forks source link

Containers should be using protected variables, not private. #829

Open ben-hn opened 2 months ago

ben-hn commented 2 months ago

Expected Behaviour I should be able to extend the PostgreSqlContainer without issues as follows

 public override async start() {
    return new StartedPostgreSqlContainer(
      await super.start(),
      this.database,
      this.username,
      this.password,
    );
  }

Actual Behaviour I'm required to add

private override database = 'test';
private override username = 'test';
private override password = 'test';

Testcontainer Logs Not relevant

Steps to Reproduce

import postgresql from '@testcontainers/postgresql';

export class PostgreSqlContainer extends postgresql.PostgreSqlContainer {
  // These are the same as upstream, however they use 'private' instead of 'protected'
  private override database = 'test';
  private override username = 'test';
  private override password = 'test';

  public override async start() {
    return new StartedPostgreSqlContainer(
      await super.start(),
      this.database,
      this.username,
      this.password,
    );
  }
}

/*
 * This is here to wrap the provided postgresql container so we can use 'using'
 */
export class StartedPostgreSqlContainer
  extends postgresql.StartedPostgreSqlContainer
  implements Disposable
{
  async [Symbol.dispose](): Promise<void> {
    await this.stop();
  }
}

Environment Information

cristianrgreco commented 2 months ago

What do you need that using the public withUsername/password doesn't provide?

ben-hn commented 2 months ago

I'm wanting to use the using command brought in with Typescript 5.2, I thought my only options were to extend the existing class, wait for you to add

    [Symbol.dispose]: async () => {
      await (await container).stop();
    },

to StartedTestContainer (which would need a typescript bump), or now I realise I can just use a wrapper with

export const DisposableContainer = async <
  T extends Promise<StartedTestContainer>,
>(
  container: T,
) => {
  return Object.assign(await container, {
    [Symbol.dispose]: async () => {
      await (await container).stop();
    },
  });
};

Which might be helpful for others.