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 Test containers for test cases #14

Merged
merged 8 commits into from
Sep 24, 2024
Merged

Add Test containers for test cases #14

merged 8 commits into from
Sep 24, 2024

Conversation

rtclauss
Copy link
Contributor

Note that some tests may fail. If you want to skip the tests add -DskipTests=true as part of your build.

Also, I've added a startup bean in 7b54dcf which creates the necessary indexes in CouchDB to improve performance when there is a lot of content in the CouchDB.

Copy link
Member

@jwalcorn jwalcorn left a comment

Choose a reason for hiding this comment

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

In general, I like it - thanks for adding these tests, and automatically making that index. I do see some random line of output in the readme where it shows how to invoke stuff (line 66), that should be cleaned up. Also not sure why the mention of the option to run CouchDB in a VM in the cloud got removed from the readme; if I use the Azure portal to request a CouchDB deployment, it is indeed a VM in the cloud. Lastly, unless I'm misunderstanding stuff, I don't see it actually checking if CouchDB is available in the CouchDBReadinessTest (it seems to be the same as the NoCouchDBReadinessTest)

@rtclauss
Copy link
Contributor Author

I added additional clarification to the README.md.

The difference between CouchDBReadinessTest and NoCouchDBReadinessTest is that the first test scenario starts up CouchDB testcontainer, then tests the health check endpoint to verify that the health check has passed while the second scenario does not start CouchDB and verifies that the health check fails.

I've updated the Javadocs in the most recent commit.

Copy link
Member

@jwalcorn jwalcorn left a comment

Choose a reason for hiding this comment

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

Looks good now. Thanks!

@rtclauss rtclauss merged commit 1268072 into master Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants