Skip to content

Fix RUBY-2287 Max staleness spec tests do not correctly assert all requirements - #1962

Merged
p-mongo merged 33 commits into
mongodb:masterfrom
neilshweky:RUBY-2287
Jun 25, 2020
Merged

Fix RUBY-2287 Max staleness spec tests do not correctly assert all requirements#1962
p-mongo merged 33 commits into
mongodb:masterfrom
neilshweky:RUBY-2287

Conversation

@neilshweky

Copy link
Copy Markdown
Contributor

All I did was add a check that suitable servers was empty if available servers was empty. I checked that if there was an available Primary or Secondary server and the suitable servers was empty that it would fail.

@neilshweky
neilshweky requested review from egiurleo and p-mongo June 17, 2020 18:13
Comment thread spec/spec_tests/max_staleness_spec.rb Outdated
expect do
server_selector.select_server(cluster)
end.to raise_exception(Mongo::Error::NoServerAvailable)
expect(spec.suitable_servers).to eq([])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

spec is actually an object representing the yaml file that the test is based on, so I think you'll want to use this as the expected value in your tests.

Take a look at line 128 for an example of something similar -- in_latency_window is derived from spec.in_latency_window. We take the results of server selection and make sure that they match the value specified by the yml file.

@p-mongo

p-mongo commented Jun 22, 2020

Copy link
Copy Markdown
Contributor

I ended up moving a lot of code around to make the tests test the driver instead of testing themselves.

Behavior changes:

  • I believe the driver did not validate deployment's staleness support in some cases. Now the validation happens in the same place for all topology types.
  • select in server selector classes was renamed to select_in_replica_set which makes more sense to me based on what it does. It so happens that select method is still there which is the Kernel.select (the file descriptor/socket selection one). This annoyingly causes code that uses the old method to hang when select is now invoked. I believe this change is in private API although the methods weren't marked as such (they are now).
  • Selectable module is now gone, its functionality was moved to base server selector. This shouldn't affect the public API.

Test changes:

  • Max staleness spec tests were redone to no longer hack their way into greenness. Some tweaks to driver code were needed to provide a list of suitable servers to the tests.
  • Spec test runner reorganized to be more linear when defining various cases, to guard against case lossage.
  • Suitable servers assertions demanded by the spec are now implemented.
  • I've done some spot checks and spec tests now fail when yaml files are edited to be bogus.
  • Server selector and max staleness use the same exact runner instead of having close-but-not-quite identical copy-pastes of it.
  • Spec test definition moved to the runner since it's now shared between two tests.

@p-mongo p-mongo changed the title RUBY-2287 expect suitable servers to be empty if none available Fix RUBY-2287 Max staleness spec tests do not correctly assert all requirements Jun 23, 2020
@p-mongo

p-mongo commented Jun 23, 2020

Copy link
Copy Markdown
Contributor

@p-mongo
p-mongo requested a review from egiurleo June 24, 2020 05:14
@p-mongo
p-mongo merged commit 08e5107 into mongodb:master Jun 25, 2020
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