-
Notifications
You must be signed in to change notification settings - Fork 543
CXX-1201 Support running examples against various topologies #1190
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
Conversation
Resolved merge conflicts and cherry-picked some additional changes:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, with one small recommended change.
examples/CMakeLists.txt
Outdated
if (${EXAMPLE} MATCHES "encryption") | ||
continue() | ||
if (${source} MATCHES "encryption") | ||
set(skip_run_examples ON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the name skip_run_examples
quite confusing. On the first read-through I was convinced that anytime a matching example was encountered that it would skip running all examples. I recognize that this variable is distinct from SKIP_RUN_EXAMPLES
(since CMake is case sensitive when it comes to variable names) and I did eventually figure out that the former is used to set the value of the latter as a property for the individual example executable. However, I recommend renaming this variable to skip_run_this_example
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. There is some double-negation occurring with the current setup relative to the default:
- Is this example added to run-examples?
- Yes (by default), unless:
- No Skip -> Yes (Yes -> No -> Yes) (<--)
- Yes Skip -> No (Yes -> Yes -> No)
- Yes (by default), unless:
Renamed SKIP_RUN_EXAMPLES
-> ADD_TO_RUN_EXAMPLES
and skip_run_examples
-> add_to_run_examples
so there is only one level of negation relative to the default + no singular vs. plural confusion.
- Is this example added to run-examples?
- Yes (by default), unless:
- No Add -> No (Yes -> No -> No)
- Yes Add -> Yes (Yes -> Yes -> Yes)
- Yes (by default), unless:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. That is much more clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with suggested comment.
examples/mongocxx/exception.cpp
Outdated
@@ -31,6 +31,9 @@ using bsoncxx::builder::basic::kvp; | |||
using bsoncxx::builder::basic::make_document; | |||
|
|||
int main() { | |||
// No known way to set MSBuild IgnoreStandardErrorWarningFormat=true via CMake or CLI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// No known way to set MSBuild IgnoreStandardErrorWarningFormat=true via CMake or CLI. | |
// Do not print error messages when run in CI. Avoids MSBuild error message detection causing build failures. | |
// No known way to set MSBuild IgnoreStandardErrorWarningFormat=true via CMake or CLI. |
Summary
Resolves CXX-1201. Verified by this patch.
The changes in this PR are a precursor to the addition of API doc examples in CXX-3082.
Description
The goal of this PR is to prepare for the addition of the
examples/api
subdirectory. This directory will be dedicated to example code that is directly imported into API docs via the \snippet special command (which permits greater control over the imported contents than the blunt\include
command). The existingexamples/bsoncxx
andexamples/mongocxx
executables are preserved for backward compatibility and as a set of standalone example programs not related to API docs.Cleanup
The
CMakeLists.txt
files underexamples
subdirectories were removed in favor offile(GLOB_RECURSE)
, similar to what is already being done for public header files. This will allow for flexible addition, removal, and relocation of example code (although it is expected to mirror the include directory structure). Theadd_subdirectory
andprojects
subdirectories are not actually used by the primary CMake configuration (used via test scripts instead), so their CMake config files have been removed and their contents manually listed in the distribution file list.examples vs. run-examples
The build vs. run targets were redesigned to completely separate the "build the examples" commands from the "run the examples" commands. This reduces redundant build tasks (especially with MSVC) and also permits parallelism during building of executables. However, this requires the
examples
target to be manually built prior to therun-examples
target, otherwise therun-examples
target will fail attempting to execute non-existent executables. EVG scripts were updated accordingly. This change also required the addition of--parallel 1
(overridingCMAKE_BUILD_PARALLEL_LEVEL
) to ensure example executables are executed sequentually rather than in parallel (they are not designed to run in parallel).MONGOCXX_TEST_TOPOLOGY
Several examples executables under
examples/mongocxx
were being skipped entirely (both execution and compilation) due to depending on particular server topologies to run correctly (CXX-1201). Following the changes in this PR, all examples are now built by CI. However, some examples are still not run. Specifically, any executables underexamples/mongocxx
containing"encryption"
in their name are skipped via theSKIP_RUN_EXAMPLES
option in the newadd_examples_executable()
CMake function. Enabling these examples to be run is deferred to a separate PR (e.g. by relying on pre-installed C Driver spawning mongocryptd + signalling with aMONGOCXX_TEST_WITH_LIBMONGOCRYPT
env var?).This PR introduces the
MONGOCXX_TEST_TOPOLOGY
environment variable which serves two purposes: 1) indicate the example executables are being executed within our CI environment (users are not expected to set this) and 2) indicate the server topology currently present in our CI environment (as defined by the preceeding "start_mongod" EVG function per task definition). This variable is set by each call to the"test"
EVG function (which invokestest.sh
).Not all example executables require checking the
MONGOCXX_TEST_TOPOLOGY
variable. e.g. theexamples/mongocxx/connect
executable is defined to to be directly invoked by the "test auth" EVG function (via.evergreen/connect.sh
). To avoid complicating the EVG config, only invocations of the"test"
EVG function definesMONGOCXX_TEST_TOPOLOGY
. We may want to eventually separate running example executables from thetest.sh
script.Updates to Examples and Miscellaneous
The rest of this PR contains updates to example code to permit their execution in CI without causing issues, as well as other miscellaneous improvements. These include:
MONGOCXX_TEST_TOPOLOGY
and its value as needed.replicaSet
URI option to match mongo orchestration configs (replset
->repl0
).--config <build_type>
to specify the build type instead of MSBuild-specific flags.