Skip to content

Conversation

brainded
Copy link
Contributor

What does this PR do?

Adds Typesense testcontainers implementation.

Why is it important?

No Typesense Module currently available for Dotnet.

Related issues

No existing issues found related to Typesense.

How to test this PR

tests/Testcontainers.Typesense.Tests contains two tests:

  • Call Healthy endpoint to verify container up and running
  • Call Collection endpoint which will receive an empty json array

Copy link

netlify bot commented May 11, 2025

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit ab3d0d5
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-dotnet/deploys/683b5770f478b20008b730c1
😎 Deploy Preview https://deploy-preview-1446--testcontainers-dotnet.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 project configuration.

@brainded brainded changed the title Add Typesense Module and Tests feat: Add Typesense Module and Tests May 11, 2025
@brainded
Copy link
Contributor Author

Not sure why Pulsar failed, I don't believe I impacted that?

@HofmeisterAn
Copy link
Collaborator

Not sure why Pulsar failed, I don't believe I impacted that?

No, the PR isn't related. It's Pulsar... It seems like the consumer sometimes isn't in a working state. I'll try to review it in the next days.

Copy link
Contributor

@TheConstructor TheConstructor left a comment

Choose a reason for hiding this comment

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

I just had a very brief look at your code and added two questions

@HofmeisterAn HofmeisterAn added enhancement New feature or request module An official Testcontainers module labels May 21, 2025
Copy link
Collaborator

@HofmeisterAn HofmeisterAn left a comment

Choose a reason for hiding this comment

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

I've updated the PR to follow the repository standards.

We only create container builder APIs for settings that are absolutely necessary to start the container. If a setting can be configured with methods like WithCommand(string), we avoid adding separate APIs to keep it simple.

I removed the --cors flag. When it's always set, developers can't disable it later.

The container configuration type only includes properties needed to modify the setup in Build() or needed in the container type.

LMKWYT. Happy to merge the PR if you're okay with the changes.

@brainded
Copy link
Contributor Author

@HofmeisterAn Yeah, thank you for doing that. Been quite busy lately and just getting back to it now. The changes look good to me. Thanks again, I really appreciate it!

@HofmeisterAn HofmeisterAn changed the title feat: Add Typesense Module and Tests feat: Add Typesense module Jun 1, 2025
Copy link
Collaborator

@HofmeisterAn HofmeisterAn left a comment

Choose a reason for hiding this comment

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

@HofmeisterAn Yeah, thank you for doing that. Been quite busy lately and just getting back to it now. The changes look good to me. Thanks again, I really appreciate it!

Oh, no worries. Thanks for the contribution!

@HofmeisterAn HofmeisterAn merged commit 6306178 into testcontainers:develop Jun 1, 2025
71 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 module An official Testcontainers module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants