[GitHub] [flink] XComp commented on a diff in pull request #22893: [FLINK-31814][runtime] Enables Precondition that checks that the issuedLeaderSessionID is set on revoke processing

["XComp (via GitHub)" <gi...@apache.org>]

XComp commented on code in PR #22893:
URL: https://github.com/apache/flink/pull/22893#discussion_r1253347147


##########
flink-runtime/src/test/java/org/apache/flink/runtime/leaderelection/DefaultLeaderElectionServiceTest.java:
##########
@@ -247,38 +247,33 @@ void testDelayedGrantCallAfterContenderBeingDeregisteredAgain() throws Exception
         };
     }
 
-    /**
-     * Test to cover the issue described in FLINK-31814. This test could be removed after
-     * FLINK-31814 is resolved.
-     */
     @Test
-    void testOnRevokeCallWhileClosingService() throws Exception {
-        final TestingLeaderElectionDriver.TestingLeaderElectionDriverFactory driverFactory =
-                new TestingLeaderElectionDriver.TestingLeaderElectionDriverFactory(
-                        LeaderElectionEventHandler::onRevokeLeadership);
-
-        try (final DefaultLeaderElectionService testInstance =
-                new DefaultLeaderElectionService(
-                        driverFactory, fatalErrorHandlerExtension.getTestingFatalErrorHandler())) {
-            testInstance.startLeaderElectionBackend();
-
-            final TestingLeaderElectionDriver driver = driverFactory.getCurrentLeaderDriver();
-            assertThat(driver).isNotNull();
-
-            driver.isLeader();
-
-            final LeaderElection leaderElection =
-                    testInstance.createLeaderElection(createRandomContenderID());
-            final TestingContender contender =
-                    new TestingContender("unused-address", leaderElection);
-            contender.startLeaderElection();
+    void testDelayedRevokeCallAfterContenderBeingDeregisteredAgain() throws Exception {

Review Comment:
   fair enough - I added proper asserts that show that the revocation is only called once. The exception would have happened in the final trigger call because the `issuedLeaderSessionID` would have been null'd by the close call already. I didn't add this as a comment because it doesn't add much value to the test and was more of an issue due to the previously immature state of the code.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org