I agree, it's not optimal. In Pulsar, most tests that we call "unit tests" are actually integration tests. Since there a lot of such tests which rely on clean state (state cleared after every method), switching to some other solution for these tests could potentially increase test times. That's the tradeoff. It would be possible to extend the main test framework class org.apache.pulsar.broker.testcontext.PulsarTestContext to easily configure LocalBookkeeperEnsemble when that would be needed. The benefit of moving towards PulsarTestContext as the way to wire up things is that PulsarTestContext could also be used in Junit 5 when we start migrating away from TestNG. Perhaps we could enable Junit 5 for new tests and start moving to this direction? In BookKeeper, Junit 5 is enabled and some tests have been migrated from Junit 4 to 5 in that project. BK doesn't use TestNG.
Currently what we call "integration tests" in Pulsar CI are using a minimal docker image built with tests/docker-images/java-test-image/Dockerfile and "system tests" are using the pulsar-all image extended with some testing files, built with tests/docker-images/latest-version-image/Dockerfile. The test execution is handled with TestNG and TestContainers using the testing framework that is currently in tests/integration/src/test/java and that's what is being extracted to the pulsar-inttest-lib module. -Lari On 2024/11/01 02:05:48 Yunze Xu wrote: > This is a good start to. I have another suggestion. Currently we > relied heavily on mock ZK and BK services. However, if we're not going > to mock some behaviors of ZK or BK, it might not be good. I have > encountered the following error logs in the downstream application: > > ``` > > 12:28:36.903 > [ZKMetadataStore-216-1:org.apache.pulsar.broker.PulsarService@1167] > INFO org.apache.pulsar.broker.PulsarService - This broker was elected > leader > ... (400+ lines) > 12:28:44.630 > [ZKMetadataStore-12-1:org.apache.pulsar.broker.PulsarService@1167] > INFO org.apache.pulsar.broker.PulsarService - This broker was elected > leader > ``` > > However, eventually I found it's an issue with mock ZK or BK. After > changing the BK to `LocalBookkeeperEnsemble`, the errors above > disappeared. We should really avoid mocking external services as much > as possible. > > Thanks, > Yunze > > On Fri, Nov 1, 2024 at 9:55 AM Jie crossover <crossover...@gmail.com> wrote: > > > > LGTM, the current test code is relatively confusing, and this proposal can > > improve this situation. > > -- > > Best Regards！ > > crossoverJie > > > > > > Heesung Sohn <hees...@apache.org> 于2024年11月1日周五 06:16写道： > > > > > Hi all, > > > > > > I think we can refactor/split the `tests` module(the integration test > > > module) to reuse the integration test code better. > > > > > > Currently, there are good test classes to reuse(such as topic message > > > test cases, container setup, and other test util funcs), but this > > > module is built under "test" scope only, making it hard to reuse in > > > other projects. > > > > > > I propose the following module structure. > > > > > > `tests` : contains actual integ test runners built under test > > > scope(e.g. test-ng test classes, depending on `pulsar-inttest-lib, > > > `pulsar-inttest-client` and other modules) > > > > > > `pulsar-inttest-lib` : contains common test libraries(test-utils, test > > > containers, and other sharable integ test setups) > > > > > > `pulsar-inttest-client` : contains common integ-test client cases (it > > > should contain only client side test logic, depending on PulsarClient > > > and PulsarAdmin). These test cases can be used in `tests`. > > > > > > I am looking forward to your feedback. > > > > > > Thanks, > > > Heesung > > > >
