Skip to content

RUST-395 Log skipped tests #523

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
Feb 4, 2022

Conversation

karmenliang
Copy link
Contributor

RUST-395

This PR adds logging to test cases that are skipped and enables the --nocapture flag to allow the test framework to print output for skipped tests.

@abr-egn
Copy link
Contributor

abr-egn commented Jan 20, 2022

I've updated this to log skips in a way that's not captured by cargo test - @patrickfreed and @isabelatkinson please take a look!

Copy link
Contributor

@patrickfreed patrickfreed left a comment

Choose a reason for hiding this comment

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

functional changes LGTM--great idea with log_uncaptured! Only things missing are a few test names in the log messages. Also looks like this needs some merge conflicts to be fixed up

@@ -431,7 +432,7 @@ async fn cmap_spec_tests() {

let mut options = CLIENT_OPTIONS.clone();
if options.load_balanced.unwrap_or(false) {
println!("skipping due to load balanced topology");
log_uncaptured("skipping due to load balanced topology");
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add which test is being skipped here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.

@@ -643,7 +644,7 @@ async fn direct_connection() {

let test_client = TestClient::new().await;
if !test_client.is_replica_set() {
println!("Skipping due to non-replica set topology");
log_uncaptured("Skipping due to non-replica set topology");
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto add test name here

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.

async fn run_test(uri_env_var: &str, resolver_config: Option<ResolverConfig>) {
if std::env::var_os("MONGO_ATLAS_TESTS").is_none() {
log_uncaptured("skipping test due to undefined environment variable MONGO_ATLAS_TESTS");
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto test name

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.

@@ -31,6 +31,7 @@ async fn run_test<F: Future>(
let client = EventClient::with_additional_options(Some(options), None, None, None).await;

if !client.is_replica_set() {
log_uncaptured("skipping test due to not running on a replica set");
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto name

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.

@@ -173,6 +179,8 @@ async fn run_test(mut test_file: TestFile) {

RUNTIME.delay_for(Duration::from_millis(500)).await;
}
} else {
log_uncaptured("skipping test due to test topology");
Copy link
Contributor

Choose a reason for hiding this comment

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

think this needs test name and maybe more details about what is off about the topology

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.

@abr-egn abr-egn force-pushed the RUST-395/log-skipped-tests branch from 83a337b to 71eeab7 Compare February 4, 2022 19:16
Copy link
Contributor

@patrickfreed patrickfreed left a comment

Choose a reason for hiding this comment

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

LGTM! (Sorry the PR I just merged created another conflict, but besides that this should be good to go)

@abr-egn abr-egn force-pushed the RUST-395/log-skipped-tests branch from 011681e to b54437e Compare February 4, 2022 19:37
@abr-egn abr-egn merged commit 80b34f3 into mongodb:master Feb 4, 2022
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.

4 participants