Skip to content

CXX-2700 Add search index management e2e testing against Atlas #1002

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

Merged
merged 13 commits into from
Aug 3, 2023

Conversation

joshbsiegel
Copy link
Contributor

@joshbsiegel joshbsiegel commented Jul 31, 2023

Summary

This PR will add 5 prose tests that will test search index management against a live Atlas cluster, modify the existing Evergreen config to automatically run these tests in the CI pipeline, and make small changes to the existing spec tests to sync with the drivers ticket.

Background

This ticket (mainly the changes made in the README) gives the vast majority of context needed to understand what prose tests should be added, why they should be added, and how to run them in Evergreen.

How to Test Locally

Being that running the prose tests could take around 20 minutes in total, there was not a lot of emphasis placed on making them easy to run locally. However, if you do wish to run them locally, here are the steps that you'll have to take:

  1. Spawn an Atlas cluster. The easiest way to do this is by running the setup-atlas-cluster.sh script in drivers-evergreen-tools/.evergreen/atlas. You'll need to export some of the environment variables mentioned in the comments of the script, they can be found on the Evergreen variables page. Spawning the cluster can take 7-10 minutes.
  2. Export the MONGODB_URI environment variable as the cluster URI (printed to atlas-expansion.yml by the script)
  3. Compile and run the tests. The test name is atlas search indexes prose tests. This will take 5-10 minutes.
  4. Teardown the cluster using teardown-atlas-cluster.sh. You'll need some environment variables for this one too, it's all mentioned in the comments of the script.

Evergreen

We chose to use a task group to run the search index prose tests. What's unique about task groups is that they have a setup_group and a teardown_group, which are commands that will run before the tasks attached to the group are run (setup) and after they are run (teardown). Both the setup commands and the teardown commands will run regardless of the outcome of the actual tests. This is important in this use case because we need to ensure that the Atlas cluster gets torn down even if the tests don't pass. Failure to teardown the Atlas cluster is inefficient and will cause problems the next time that we try to spawn an Atlas cluster.

What's New

  • Minor changes to the spec tests
  • Added another overload to create_one that only takes in a definition (see comment for explanation)
  • Added 5 prose tests in test/search_index_view.cpp
  • Updated Evergreen config to automatically spawn a cluster, run the tests, and teardown the cluster

@joshbsiegel joshbsiegel marked this pull request as draft July 31, 2023 14:47
Comment on lines 89 to 109
///
/// This is a convenience method for creating a single search index.
///
/// @param definition
/// The document describing the search index to be created.
///
/// @return The name of the created search index.
///
std::string create_one(bsoncxx::document::view_or_value definition);

///
/// This is a convenience method for creating a single search index.
///
/// @param definition
/// The document describing the search index to be created.
///
/// @return The name of the created search index.
///
std::string create_one(const client_session& session,
bsoncxx::document::view_or_value definition);

Copy link
Contributor Author

@joshbsiegel joshbsiegel Jul 31, 2023

Choose a reason for hiding this comment

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

These two overloads were added due a change made in DRIVERS-2630 that allows for create_one to be called while only passing through a definition, no name. I kept the other overloads of create_one because the drivers ticket specified that a name should still be able to be passed through, but in the options of create_one. Because CXX allows function overloads, we don't have a need to have an "options" parameter.

@@ -65,23 +74,24 @@ std::string search_index_view::create_one(const client_session& session,
return _get_impl().create_one(&session, model);
}

std::vector<bsoncxx::string::view_or_value> search_index_view::create_many(
std::vector<std::string> search_index_view::create_many(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran into some issues with reading the values of the std::vector<bsoncxx::string::view_or_value> returned from this function due to ownership semantics. After talking to @eramongodb, we came to the conclusion that it would be easier and more clear to just make this a std::vector<std::string>. This minor change seen across this PR.

@joshbsiegel joshbsiegel requested a review from kevinAlbs August 2, 2023 16:08
@joshbsiegel joshbsiegel marked this pull request as ready for review August 2, 2023 16:08
@joshbsiegel joshbsiegel requested a review from kevinAlbs August 3, 2023 17:52
@kevinAlbs kevinAlbs removed their request for review August 3, 2023 19:48
@joshbsiegel joshbsiegel requested a review from kevinAlbs August 3, 2023 21:08
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