Skip to content

CXX-2683 Replace codecov-bash with new Codecov uploader #958

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 10 commits into from
Apr 24, 2023

Conversation

eramongodb
Copy link
Contributor

Description

Resolves CXX-2683 and includes several miscellaneous improvements to the CXX Driver EVG config. Verified by this patch.

The primary motivation for this PR is to reduce output verbosity in CXX Driver EVG patches. Recommend comparing outputs in the linked patch against those in the base commit.

The combination of applied output filters, verbosity reduction, and output-on-failure behavior described below reduces the total lines of output (relative to the base commit) for the following tasks by:

  • 11721 -> 4970 (-57.6%): compile_and_test_with_shared_libs in Ubuntu 18.04 Debug (MongoDB Latest)
  • 7831 -> 4913 (-37.3%): compile_and_test_with_static_libs in Ubuntu 18.04 Release (MongoDB Latest)
  • 7721 -> 1769 (-77.1%): compile_and_test_with_static_libs in Windows (VS 2015) Release (MongoDB 4.2)
  • 7527 -> 1842 (-75.5%): compile_and_test_with_shared_libs in Windows (VS 2017) Debug (MongoDB Latest)
  • 2189 -> 1136 (-48.1%): build_example_with_add_subdirectory in Ubuntu 20.04 Release (MongoDB Latest)

Note: setup failures on Ubuntu 14.04 do not appear to be related to changes in this PR.

codecov-bash

The Codecov Bash Uploader is deprecated and no longer maintained as of February 1, 2022. A new Codecov Uploader is recommended in its place. This PR updates the code coverage EVG function to use the new uploader.

Note, the new uploader requires downloading the appropriate binary from https://uploader.codecov.io/. The available binaries are limited to x86_64 MacOS, Linux, and Windows binaries + AArch64 (Linux?) binaries, but since we are currently only enabling code coverage on ubuntu1804 distros, the script is left minimal to avoid unnecessary complexity (+ I do not expect we will be expanding code coverage to additional distros anytime soon).

A filter (via perl) is applied to the output of the Uploader to avoid the spammy list of "(Found|Processing) " messages generated by the Uploader that often occupied the ending of EVG tasks output, obscuring the failures or errors of actual interest.

Ensure Replica Set Member on Port 27017 is Primary

A long-time cause of flaky test failures in the CXX Driver waterfall has been "not primary" errors due to the test suite assuming the replica set member on port 27017 is primary by directly connecting to it via the default URI (which does not list the other replset members). A routine was added to the start_mongod EVG function that triggers a replset reconfiguration + waits for the member on port 27017 to become primary to address this issue. Because mongo-orchestration does not appear to support directly designating a replset member as primary, mongosh is used instead to send the appropriate commands.

Miscellaneous Output Reduction

Various modifications were applied to reduce verbosity of task output, much of which is not of interest or relevance during regular development workflows:

  • Removed printing contents of the mongoc directory in the compile EVG function.
  • Removed the VERBOSE option from invocations of GNU Make.
  • Added the /verbosity:minimal option to invocations of MSBuild.
  • Silenced standard output when building and installing libmongocrypt and libmongoc.
  • Silenced standard output when registering CA certificates.
  • Only print output of building and running example projects and executables on failure.

Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

The reduced verbosity and fix to "not primary" errors are very much appreciated. LGTM with the re-addition of make run.

@@ -10,13 +10,12 @@ rm -rf build/*
cd build
if [ -z "$MSVC" ]; then
"$CMAKE" -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" -DCMAKE_CXX_STANDARD="${CXX_STANDARD}" ..
make run VERBOSE=1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Replace with make run to run example. Same comment applies to other files under examples/projects.

@@ -191,6 +191,48 @@ functions:
echo "{ \"releases\": { \"default\": \"$MONGODB_BINARIES\" }}" > $MONGO_ORCHESTRATION_HOME/orchestration.config
./.evergreen/run-orchestration.sh

# Ensure server on port 27017 is the primary server.
Copy link
Collaborator

Choose a reason for hiding this comment

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

by directly connecting to it via the default URI (which does not list the other replset members)

Another alternative to consider: the C driver's test_framework_get_uri_str_no_auth constructs a URI with multiple hosts after checking the hello response. Using multiple hosts in the URI may be more robust if tests are expected to step down the primary. AFAICT the C++ driver has no such tests, but there are specification tests that use replSetStepDown.

Though setting the priority still seems like an improvement, and I expect should resolve the "not primary" errors.

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 considered adding the replset hosts to the URIs used by the test framework, but decided against it as many tests use the default-constructed mongocxx::uri object to obtain the URI, and updating the test suite to use a test-framework URI constructor like the C Driver seemed too extensive a change for this PR.

@eramongodb eramongodb merged commit 3b73a2a into mongodb:master Apr 24, 2023
@eramongodb eramongodb deleted the cxx-2683 branch April 24, 2023 20:49
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