amrishlal opened a new pull request #6305: URL: https://github.com/apache/incubator-pinot/pull/6305
**Description** This PR addresses issue #5688. To improve run time efficiency, all test cases under pinot-controller were added to a TestNG suite (see `testng.xml`and `ControllerTestSetup.java`). Before any of the test cases are run, TestNG invokes the `@BeforeSuite` method in `ControllerTestSetup` where the shared state (Zookeeper, Pinot Controller, Broker and Server instances) is initialized. This shared state is used by all the test classes in the suite. Each test case class, that uses shared state, has a `@BeforeClass` method where `ControllerTestUtils.validate()` function is called to ensure that shared state is clean. If the `validate()` function does not assert, all the `@Test` functions in the class are run. In the end, `@AfterClass` function calls `ControllerTestUtils.cleanup()` to clean up shared state and avoid conflict with subsequent test classes. Test classes may also need to do custom cleanup depending upon the state changes they make. Once all the test classes are done executing, TestNG invokes the `@AfterSuite` method in ControllerTestSetup to de-initialize the shared state to prevent conflicts with subsequent test suites. **Performance** Before this change, pinot-controller test cases took ~5 minutes to run. After this PR, pinot-controller test cases take ~1.5 minutes to run: ``` ORIGINAL: Tests run: 207, Failures: 0, Errors: 0, Skipped: 0[INFO] [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 04:54 min [INFO] Finished at: 2020-12-01T10:46:58-08:00 [INFO] ------------------------------------------------------------------------ ``` ``` MODIFIED: Tests run: 200, Failures: 0, Errors: 0, Skipped: 0 [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 01:31 min [INFO] Finished at: 2020-12-01T12:20:42-08:00 [INFO] ------------------------------------------------------------------------ ``` **Remaining Issues** As part of this PR, 7 test cases had to be disabled either because they did not fit into the shared state model or because the implementation appeared too complex to do a clean migration to shared state model. I have marked these test cases with `TODO` comment. There is also additional cleanup that can be done within individual test cases (for example some of the test cases may not need to create tenants any more since the shared state has default tenants setup). For now, I have deferred such test specific cleanup for subsequent cleanup efforts to keep this PR focused towards the overall goal of moving the test cases to the shared state model. ---------------------------------------------------------------- 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. 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
