Codegass opened a new issue, #12436: URL: https://github.com/apache/druid/issues/12436
### Description & Motivation Hi, I would like to propose an idea of using Assume feature offered in JUnit 4&5 for some of the test cases in Druid. The Assume function is to replace Assertion in precondition checking. As [mentioned in the document](https://junit.org/junit4/javadoc/4.12/org/junit/Assume.html): > Assume functions is a set of methods useful for stating assumptions about the conditions in which a test is meaningful. A failed assumption does not mean the code is broken, but that the test provides no useful information. I notice that the test case [testPollOnDemand](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java#L187) can benefit from the Assume feature. Right now this case is asserting the precondition(`dataSourcesSnapshot`) before the actual testing target(`sqlSegmentsMetadataManager.forceOrWaitOngoingDatabasePoll()`) is executed. So test `testPollOnDemand` may fail due to 2 reasons: * The functions related to the testing precondition have bug (here is the `sqlSegmentsMetadataManager.getDataSourcesSnapshot()` `sqlSegmentsMetadataManager.useLatestSnapshotIfWithinDelay()`). * The functions related to Poll on Demand have bugs.(What we actually want to check with this test) Here I suggest replacing the `assertNull` function with the `assumeThat` function, so when the precondition fails, JUnit can skip it instead of report a failure. This can help us to focus on failure that is raise by the target testing function, and filter the precondition failure. #### Proposed Changes Before ```java DataSourcesSnapshot dataSourcesSnapshot = sqlSegmentsMetadataManager.getDataSourcesSnapshot(); Assert.assertNull(dataSourcesSnapshot); // This should return false and not wait/poll anything as we did not schedule periodic poll Assert.assertFalse(sqlSegmentsMetadataManager.useLatestSnapshotIfWithinDelay()); Assert.assertNull(dataSourcesSnapshot); ``` After ```java DataSourcesSnapshot dataSourcesSnapshot = sqlSegmentsMetadataManager.getDataSourcesSnapshot(); Assume.assumeThat(dataSourcesSnapshot, is(null)); // This should return false and not wait/poll anything as we did not schedule periodic poll Assume.assumeFalse(sqlSegmentsMetadataManager.useLatestSnapshotIfWithinDelay()); Assume.assumeThat(dataSourcesSnapshot, is(null)); ``` Please let me know if you think this proposal makes sense, I would be more than happy to try the refactoring. (If not, comments are appreciated.) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
