Skip to content

Update InitialDnsSeedlistDiscoveryTest #1009

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 1 commit into from
Oct 5, 2022
Merged

Conversation

jyemin
Copy link
Collaborator

@jyemin jyemin commented Oct 4, 2022

JAVA-4532

This is harder to read than I thought it would be, probably because I tried to do a few things at the same time:

  1. Refactor the test class so that it's a single test instead of multiple tests. This matches the spec test description more closely
  2. Pull out a couple of methods that do the bulk of the assertions
  3. Add parsed_options assertions
  4. Support ping field in tests

If it's too hard to read I can split it up. Let me know.

@jyemin jyemin self-assigned this Oct 4, 2022
@jyemin jyemin requested a review from stIncMale October 4, 2022 23:06
Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

Why do we have two effectively separate implementations of the test runner (assertErrorCondition, assertNonErrorCondition)? Intuitively, I'd expect that there is a single implementation that ignores some failures of some operations when isError, and fails otherwise.

@jyemin
Copy link
Collaborator Author

jyemin commented Oct 5, 2022

It just works out that way from following the logic of the test runner specification, which says:

If error is specified and true, drivers MUST verify that initializing the MongoClient throws an error. If error is not specified or is false, both initializing the MongoClient and running a ping operation must succeed without throwing any errors.

None of the assertions required for the non-error case are applicable for the error case, and vice versa.

@jyemin jyemin requested a review from stIncMale October 5, 2022 18:47
@jyemin jyemin merged commit dc0d5a1 into mongodb:master Oct 5, 2022
@jyemin jyemin deleted the j4532 branch October 5, 2022 23:22
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