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.9k stars 188 forks source link

Possible documentation refinement for the MongoDBContainer module #697

Open csprocket777 opened 10 months ago

csprocket777 commented 10 months ago

Expected Behaviour I was following the instructions on the MongoDB Module page and trying to implement in an ES6 environment that outputs CommonJS. The examples don't include the require()/import statement which would have helped a ton.

I also expected that once getting the container running and returning the value from container.getConnectionString() that it would simply connect using that string. I found that I had to add ?directConnection=true to the connection string for it to actually connect.

Also, the checkMongo function seems to be from the test suite instead of from the library itself.

Proposed changes:

  1. Make the generic similar to the below:

    // ES6
    import { MongoDBContainer, StartedMongoDBContainer }  from '@testcontainers/mongodb';
    // --or--
    // CommonJS
    const { MongoDBContainer, StartedMongoDBContainer } = require('@testcontainers/mongodb');
    
    const mongodbContainer:StartedMongoDBContainer = await new MongoDBContainer().start();
    
    // ...your code
    
    await mongodbContainer.stop();
  2. It would be super helpful to have some API docs for the modules. I ended up using container.getConnectionString() to get the connection string back to my testing suite.
  3. Include a note about the usage of ?directConnection=true in conjunction with the connection string returned. I ended up concatenating it to the end elsewhere: mongodb://localhost:[PORT]/test-db?directConnection=true

Environment Information

robvanderleek commented 8 months ago

I bumped into the same issues.

FWIW, I've created a minimal example repository that shows the issue when not using directConnection: true: https://github.com/robvanderleek/testcontainers-mongodb-issue

Environment Information

cristianrgreco commented 8 months ago

I'm no Mongo expert.. should directConnection=true be included by default in the connection string?

robvanderleek commented 8 months ago

Also not an expert, but I tested connecting to a natively running MongoDB (using Homebrew) and a containerized MongoDB (launched with docker run -p 27018:27017 mongo:6.0.1) and they both work without directConnection=true.

cristianrgreco commented 8 months ago

I've noticed the Java implementation of Testcontainers does not expose this directConnection option either. IMO it's a decision for whoever is setting up Mongo/the library they use to connect to decide whether or not to use this option.

talgendler commented 8 months ago

@cristianrgreco Hi, by default the code creates a Mongodb Replica Set

class MongoDBContainer extends testcontainers_1.GenericContainer {
    constructor(image = "mongo:4.0.1") {
        super(image);
        this.withExposedPorts(MONGODB_PORT)
            .withCommand(["--replSet", "rs0"])
            .withWaitStrategy(testcontainers_1.Wait.forLogMessage(/.*waiting for connections.*/i))
            .withStartupTimeout(120000);
    }
....

Hence you need to tell the driver to use a direct connection. If you tested this container with mongosh from a command line you can see that it adds this option by default.

$ mongosh localhost:55076/somedb

Current Mongosh Log ID: 65d36046a01ccf3f8e133100
Connecting to:      mongodb://localhost:55076/somedb?directConnection=true&serverSelectionTimeoutMS=2000&appName=mongosh+2.1.4
kelchm commented 7 months ago

This tripped me up as well, as I wasn't expecting Testcontainers to create a replica set by default.

I agree that at minimum the docs need to be updated to make this clear and ideally the implementation would support both use cases.

cristianrgreco commented 7 months ago

Thanks all for the feedback. If the docs made it clear that by default the container starts a replica set, and update the connection URI to work appropriately, would you still feel the need to support both use cases? Any PR to address either of these would be very welcome.