Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Couchbase module #646

Merged
merged 9 commits into from
Jan 16, 2024
Merged

Conversation

halilkaankarakoc
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented Aug 24, 2023

Deploy Preview for testcontainers-node ready!

Name Link
🔨 Latest commit 06bd698
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-node/deploys/65a6e733e347260008d4a686
😎 Deploy Preview https://deploy-preview-646--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.


constructor(startedTestContainer: StartedTestContainer, username: string, password: string) {
super(startedTestContainer);
this.mappedKvPort = startedTestContainer.getMappedPort(PORTS.KV_PORT);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I was writing the redis module (#642), I also oriented on this style from mysql. But when I start testing it by restaring the container, it will fail, because the container gots new ports after restart. So I moved it to the getter function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you :)

}

getConnectionString() {
return `couchbase://${this.host}:${this.mappedKvPort}`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use getMappedPort in the getter, to get actual port, also when container was restarted.

 return `couchbase://${this.host}:${startedTestContainer.getMappedPort(PORTS.KV_PORT)}`;

@halilkaankarakoc
Copy link
Contributor Author

Hi @cristianrgreco could you please review?

@welsonrcosta
Copy link

You might want to remove the packages/modules/couchbase/.DS_Store

@cristianrgreco cristianrgreco added enhancement New feature or request minor Backward compatible functionality labels Oct 26, 2023
@halilkaankarakoc
Copy link
Contributor Author

@cristianrgreco sorry for the forced-push. Could you please review again? 🙏 I removed .DS_Store and made couchbase as dev dependency as you did.

@cristianrgreco
Copy link
Collaborator

cristianrgreco commented Dec 4, 2023

Thanks @halilkaankarakoc. Looks like some tests are failing:

FAIL packages/modules/couchbase/src/couchbase-container.test.ts (83.22 s)
  CouchbaseContainer
    Enterprise Image
      ✕ should connect and query using enterprise image (31795 ms)
      ✕ should flush bucket if flushEnabled and check any document exists (6723 ms)
    Community Image
      ✕ should connect and query using community image (20788 ms)
      ✕ should flush bucket if flushEnabled and check any document exists (6498 ms)
      ✓ should throw error if analytics service enabled with community version (6501 ms)
      ✓ should throw error if eventing service enabled with community version (6754 ms)

  ● CouchbaseContainer › Enterprise Image › should connect and query using enterprise image

    TypeError: Cannot read properties of undefined (reading 'ipAddress')

      215 |     log.debug(`Renaming Couchbase Node from localhost to ${container.getHost()}`);
      216 |
    > 217 |     const internalIpAddress = inspectResult.networkSettings.bridge.ipAddress;
          |                                                                    ^
      218 |     const body = new URLSearchParams();
      219 |     body.append("hostname", internalIpAddress);
      220 |

      at CouchbaseContainer.renameNode (src/couchbase-container.ts:217:68)
      at CouchbaseContainer.containerStarted (src/couchbase-container.ts:537:16)
      at async CouchbaseContainer.startContainer (../../testcontainers/src/generic-container/generic-container.ts:213:7)
      at async CouchbaseContainer.start (src/couchbase-container.ts:569:34)
      at async Object.<anonymous> (src/couchbase-container.test.ts:55:30)

  ● CouchbaseContainer › Enterprise Image › should flush bucket if flushEnabled and check any document exists

    TypeError: Cannot read properties of undefined (reading 'ipAddress')

      215 |     log.debug(`Renaming Couchbase Node from localhost to ${container.getHost()}`);
      216 |
    > 217 |     const internalIpAddress = inspectResult.networkSettings.bridge.ipAddress;
          |                                                                    ^
      218 |     const body = new URLSearchParams();
      219 |     body.append("hostname", internalIpAddress);
      220 |

      at CouchbaseContainer.renameNode (src/couchbase-container.ts:217:68)
      at CouchbaseContainer.containerStarted (src/couchbase-container.ts:537:16)
      at async CouchbaseContainer.startContainer (../../testcontainers/src/generic-container/generic-container.ts:213:7)
      at async CouchbaseContainer.start (src/couchbase-container.ts:569:34)
      at async Object.<anonymous> (src/couchbase-container.test.ts:73:30)

  ● CouchbaseContainer › Community Image › should connect and query using community image

    TypeError: Cannot read properties of undefined (reading 'ipAddress')

      215 |     log.debug(`Renaming Couchbase Node from localhost to ${container.getHost()}`);
      216 |
    > 217 |     const internalIpAddress = inspectResult.networkSettings.bridge.ipAddress;
          |                                                                    ^
      218 |     const body = new URLSearchParams();
      219 |     body.append("hostname", internalIpAddress);
      220 |

      at CouchbaseContainer.renameNode (src/couchbase-container.ts:217:68)
      at CouchbaseContainer.containerStarted (src/couchbase-container.ts:537:16)
      at async CouchbaseContainer.startContainer (../../testcontainers/src/generic-container/generic-container.ts:213:7)
      at async CouchbaseContainer.start (src/couchbase-container.ts:569:34)
      at async Object.<anonymous> (src/couchbase-container.test.ts:110:30)

  ● CouchbaseContainer › Community Image › should flush bucket if flushEnabled and check any document exists

    TypeError: Cannot read properties of undefined (reading 'ipAddress')

      215 |     log.debug(`Renaming Couchbase Node from localhost to ${container.getHost()}`);
      216 |
    > 217 |     const internalIpAddress = inspectResult.networkSettings.bridge.ipAddress;
          |                                                                    ^
      218 |     const body = new URLSearchParams();
      219 |     body.append("hostname", internalIpAddress);
      220 |

      at CouchbaseContainer.renameNode (src/couchbase-container.ts:217:68)
      at CouchbaseContainer.containerStarted (src/couchbase-container.ts:537:16)
      at async CouchbaseContainer.startContainer (../../testcontainers/src/generic-container/generic-container.ts:213:7)
      at async CouchbaseContainer.start (src/couchbase-container.ts:569:34)
      at async Object.<anonymous> (src/couchbase-container.test.ts:126:30)

You cannot assume that the default network will be called bridge, for podman the default network is called podman. This logic is implemented here

const networkName = strategyResult.uri.includes("podman.sock") ? "podman" : "bridge";
But this is not exposed anywhere.

It may be enough to simply do startedContainer.getNetworkNames()[0]. If not you or I can make a commit in this PR to expose it in the container-runtime info. WDYT?

@cristianrgreco cristianrgreco changed the title Add couchbase module Add Couchbase module Dec 9, 2023
@cristianrgreco cristianrgreco merged commit 52ab0fc into testcontainers:main Jan 16, 2024
100 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request minor Backward compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants