Skip to content
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

DRIVERS-2951 Test serverMonitoringMode=poll waits after a successful heartbeat #1626

Merged
merged 3 commits into from
Aug 5, 2024

Conversation

ShaneHarvey
Copy link
Member

@ShaneHarvey ShaneHarvey commented Aug 2, 2024

Fixes DRIVERS-2951.

Please complete the following before merging:

  • Make sure there are generated JSON files from the YAML test files.
  • Test changes in at least one language driver.
  • Test these changes against all server versions and topologies (including standalone, replica set, sharded
    clusters, and serverless).

Question, should we still update the prose test here added in DRIVERS-1251 to run on both streaming and polling? Currently it implicitly tests polling on <4.4 servers.

@ShaneHarvey ShaneHarvey requested a review from a team as a code owner August 2, 2024 18:17
@ShaneHarvey ShaneHarvey requested review from W-A-James and prestonvasquez and removed request for a team August 2, 2024 18:17
@ShaneHarvey
Copy link
Member Author

@prestonvasquez could you review this to ensure it covers the Go driver bug? When I introduce the bug in Python the test fails like this:

TestUnifiedServerMonitoringMode.test_poll_waits_after_successful_heartbeat
...
test/unified_format.py:1635: in _testOperation_assertEventCount
    self.assertEqual(self._event_count(client, event), count, f"expected {count} not {event!r}")
E   AssertionError: 5169 != 1 : expected 1 not {'serverHeartbeatStartedEvent': {}}

@ShaneHarvey ShaneHarvey changed the title DRIVERS-2951 Test that serverMonitoringMode=poll waits after a successful heartbeat DRIVERS-2951 Test serverMonitoringMode=poll waits after a successful heartbeat Aug 2, 2024
@prestonvasquez
Copy link
Contributor

@prestonvasquez could you review this to ensure it covers the Go driver bug? When I introduce the bug in Python the test fails like this:

TestUnifiedServerMonitoringMode.test_poll_waits_after_successful_heartbeat
...
test/unified_format.py:1635: in _testOperation_assertEventCount
    self.assertEqual(self._event_count(client, event), count, f"expected {count} not {event!r}")
E   AssertionError: 5169 != 1 : expected 1 not {'serverHeartbeatStartedEvent': {}}

@ShaneHarvey Go has not implemented DRIVERS-2366 and so we don't have wait or assertEventCount. Though tests described in this PR do appear to work for POC mongodb/mongo-go-driver#1731 and fail when ran against the bug:

=== RUN   TestUnifiedSpec/server-discovery-and-monitoring/unified/serverMonitoringMode.json/poll_waits_after_successful_heartbeat
    unified_spec_runner.go:169: error running operation "assertEventCount" at index 3: execution failed: failed to assert count. want: 1, got: 8642
--- FAIL: TestUnifiedSpec (0.60s)
    --- FAIL: TestUnifiedSpec/server-discovery-and-monitoring/unified (0.60s)
        --- FAIL: TestUnifiedSpec/server-discovery-and-monitoring/unified/serverMonitoringMode.json (0.60s)
            --- FAIL: TestUnifiedSpec/server-discovery-and-monitoring/unified/serverMonitoringMode.json/poll_waits_after_successful_heartbeat (0.60s)
FAIL
exit status 1
FAIL    go.mongodb.org/mongo-driver/mongo/integration/unified   1.856s

Copy link
Contributor

@W-A-James W-A-James left a comment

Choose a reason for hiding this comment

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

Seems like the file runs this on standalones, replicasets and sharded clusters. It seems that we'd either want to write separate tests for replsets and sharded clusters or we want to restrict this to run against standalones.

@ShaneHarvey
Copy link
Member Author

We already have this at the top of the file:

# These tests cannot run on replica sets because the order of the expected
# SDAM events are non-deterministic when monitoring multiple servers.
# They also cannot run on Serverless or load balanced clusters where SDAM is disabled.
runOnRequirements:
  - topologies: [single, sharded, sharded-replicaset]
    serverless: forbid

This test works on both standalone and sharded clusters.

@ShaneHarvey ShaneHarvey merged commit 121be10 into mongodb:master Aug 5, 2024
4 checks passed
@ShaneHarvey ShaneHarvey deleted the DRIVERS-2951 branch August 6, 2024 20:36
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.

3 participants