-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
PYTHON-4540 Cleaner separation of test lifecycle #2082
Conversation
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.
Incredible work, I'm weeping with joy.
@@ -833,12 +833,9 @@ def create_load_balancer_tasks(): | |||
tags = ["load-balancer", auth, ssl] | |||
bootstrap_vars = dict(TOPOLOGY="sharded_cluster", AUTH=auth, SSL=ssl, LOAD_BALANCER="true") | |||
bootstrap_func = FunctionCall(func="bootstrap mongo-orchestration", vars=bootstrap_vars) | |||
balancer_func = FunctionCall(func="run load-balancer") |
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.
Intentional removal of balancer_func
?
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.
Yes, it was absorbed into setup_test.py
else | ||
_BIN_DIR=$HOME/.local/bin | ||
HERE=$(dirname ${BASH_SOURCE:-$0}) | ||
pushd "$(dirname "$(dirname $HERE)")" > /dev/null |
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.
Missing matching popd
?
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.
Fixed
UV_ARGS.append(f"--extra {extra}") | ||
|
||
for env_var, suite in TEST_SUITE_MAP.items(): | ||
if TEST_SUITES: |
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.
So this loop sets TEST_SUITES
equal to the first encountered suite
?
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.
That or if the environment variable was set
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.
Does it make more sense to have the continue
to be a break
then?
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.
Done
.evergreen/scripts/setup-tests.py
Outdated
|
||
if AUTH != "noauth": | ||
if is_set("TEST_DATA_LAKE"): | ||
DB_USER = "mhuser" |
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.
Should we save these into environment variables in our secrets?
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.
Done
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 scheduled an ADL test for the latest commit
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.
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 assuming the tests pass. Thanks a ton, Steve!
run-tests.sh
into a setup phase and a test run phaseuv
environment and thepytest
invocation.setup-tests.py
.Follow up PRs will move more of the "other" tests such as Auth AWS and Auth OIDC into this centralized pattern of
just setup-test
andjust test-eg
.Scheduled a cross-section of tasks for this build: https://spruce.mongodb.com/version/67abd5b92d7b8f00078f297b/tasks?sorts=STATUS%3AASC%3BBASE_STATUS%3ADESC