Skip to content

Conversation

@rozza
Copy link
Member

@rozza rozza commented May 22, 2025

Added Heartbeat prose test
Migrated DefaultServerMonitorSpecification to JUnit 5.

JAVA-5230

Added Heartbeat prose test Migrated DefaultServerMonitorSpecification to JUnit 5. JAVA-5230
@rozza rozza requested review from a team, Copilot and katcharov and removed request for a team May 22, 2025 09:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the legacy Groovy DefaultServerMonitorSpecification to JUnit 5 tests and refines the heartbeat-started event emission in DefaultServerMonitor.

  • Introduced DefaultServerMonitorTest.java with new JUnit 5 tests covering success, failure, and ordering of heartbeat events before socket connect.
  • Removed the old DefaultServerMonitorSpecification.groovy.
  • Updated DefaultServerMonitor.java to add a VisibleForTesting accessor and de-duplicate serverHeartbeatStarted events using a new flag.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
driver-core/src/test/unit/com/mongodb/internal/connection/DefaultServerMonitorTest.java Added JUnit 5 tests for heartbeat events and ordering before connection open
driver-core/src/test/unit/com/mongodb/internal/connection/DefaultServerMonitorSpecification.groovy Removed legacy Groovy-based spec after migrating tests to JUnit 5
driver-core/src/main/com/mongodb/internal/connection/DefaultServerMonitor.java Added getServerMonitor() for testing, alreadyLoggedHeartBeatStarted flag, and refined heartbeat-started logic
Comments suppressed due to low confidence (2)

driver-core/src/main/com/mongodb/internal/connection/DefaultServerMonitor.java:161

  • [nitpick] Consider renaming 'alreadyLoggedHeartBeatStarted' to 'alreadyLoggedHeartbeatStarted' to follow consistent camelCase for the term 'heartbeat'.
private volatile boolean alreadyLoggedHeartBeatStarted = false; 

driver-core/src/main/com/mongodb/internal/connection/DefaultServerMonitor.java:224

  • Add a unit test that simulates multiple heartbeat cycles to verify the 'alreadyLoggedHeartBeatStarted' flag behavior and ensure only one 'ServerHeartbeatStartedEvent' is emitted per cycle.
boolean shouldStreamResponses = shouldStreamResponses(currentServerDescription); 
@rozza rozza removed the request for review from katcharov May 22, 2025 10:56
@rozza rozza marked this pull request as draft May 22, 2025 10:56
@rozza rozza requested a review from katcharov May 22, 2025 12:23
@rozza rozza marked this pull request as ready for review May 22, 2025 12:23
@rozza rozza requested a review from katcharov May 27, 2025 11:52
Copy link
Collaborator

@katcharov katcharov left a comment

Choose a reason for hiding this comment

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

LGTM!

*/
@Test
@Ignore
@Ignore("JAVA-4484 - events are not guaranteed to be delivered in order")
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@rozza rozza merged commit 25c0262 into mongodb:main May 28, 2025
54 checks passed
@rozza rozza deleted the JAVA-5230 branch May 28, 2025 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants