yuanbenson commented on code in PR #9295: URL: https://github.com/apache/pinot/pull/9295#discussion_r958994759
########## pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/SegmentUploadIntegrationTest.java: ########## @@ -81,10 +89,15 @@ protected List<String> getBloomFilterColumns() { return null; } + @BeforeMethod + public void setUpTest() Review Comment: Calling `TestUtils.ensureDirectoriesExistAndEmpty(_tempDir, _segmentDir, _tarDir)` in `setUp` is insufficient as there is another test case `testUploadAndQuery` that leaves data in the tmp directories. Using two table names will also lead to more code for now, since it seems like `BaseClusterIntegrationTest` is written in a way that assumes one table/config per subclass, e.g. `getTableName`, `createOfflineTableConfig`. Hence, I prefer to keep the test structure as is with the before and after method annotations, unless this is an anti-pattern. Another way is to create a different test class altogether, but combining the two here to not prevent slow down from too many tests. -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org