> 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.
I realized that Pulsar tests are based on TestNG, which is not ideal to share with other projects running on Junit-5. If Junit-5 is indeed a better choice and our long-term direction, then I guess we can start using Junit-5 for new tests and for common tests in pulsar-inttest-client as well. Thanks, Heesung On Tue, Nov 5, 2024 at 2:35 PM Heesung Sohn <hees...@apache.org> wrote: > > Hi all, > > I raised a PIP PR here. > > https://github.com/apache/pulsar/pull/23565 > > Thanks, > Heesung > > On Sat, Nov 2, 2024 at 6:31 PM Heesung Sohn <hees...@apache.org> wrote: > > > > Hi, > > > > Sure. Let me open a PIP for Pulsar Test Improvement. > > > > I think I can summarize this discussion and document it as a PIP for > > future reference and as the agreed direction. > > > > Thanks, > > Heesung > > > > On Sat, Nov 2, 2024 at 2:13 AM Lari Hotari <lhot...@apache.org> wrote: > > > > > > On Sat, 2 Nov 2024 at 02:32, Heesung Sohn <hees...@apache.org> wrote: > > > > > > > > Hi, > > > > > > > > I agree with your points that there are tradeoffs between in-memory vs > > > > container test clusters, > > > > and that we should add more "unit" tests to cover behavior and avoid > > > > deep state checks. > > > > We should smartly move/add integ tests in in-memory or container test > > > > clusters, without increasing the test execution time. > > > > > > > > > > Documenting the target state in a PIP would be useful. One of the > > > roles of PIPs is for documenting designs. However, doing this doesn't > > > have to happen before starting the work to extract the integration > > > test libraries. It will be helpful for designing future improvements > > > and making conscious decisions about the direction. > > > > > > > Also, I think there are some rooms to optimize our integ tests. > > > > For example, many of the integ tests can run in parallel, under the > > > > same cluster. > > > > > > Yes, there's room for improvement. For pure integration tests, the > > > test execution time hasn't been a huge issue for Pulsar CI for some > > > time. There's a lot of resources available in Apache's GitHub account > > > and there is the possibility to split test jobs even further to run in > > > parallel in independent test jobs. > > > I'm not sure if parallel execution in a single test job would be > > > extremely helpful due to the limited resources of each GitHub Actions > > > runner VM. The resources have increased. The GitHub Actions runner VM > > > specs are at > > > https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories > > > . > > > The runner VM has 4 CPUs, 16 GB RAM, 14 GB SSD . It used to be 2 CPUs > > > and 7GB RAM before. The resources have doubled since then. We could > > > run more tests in parallel. However I don't think that pointing tests > > > to a single cluster would be a useful solution due to test state > > > management. Tests become brittle if there isn't sufficient isolation > > > of state between test runs. The results are very nondeterministic. > > > Sharing environments is a very different direction than the move > > > towards deterministic simulation where all parameters are controlled > > > in each test run. In deterministic testing, state management is > > > completely controlled and managed to always produce the same results > > > with the same input. > > > > > > > We probably need to add test cases more often instead of creating new > > > > test classes. > > > > Also, we should reuse PulsarClient and PulsarAdmin as much as > > > > possible, as initiating them in each test can add several seconds > > > > more. > > > > Some of the test classes can be merged, and the clusters can be shared > > > > among them too. > > > > > > I don't agree directly on this point. Technical details shouldn't > > > impact test design. If there's a need to cache some resources across > > > multiple tests, that could be done in other ways. For example, > > > Spring's testing framework has the "DirtiesContext" annotation to > > > control Spring's application context caching. > > > (https://docs.spring.io/spring-framework/reference/testing/annotations/integration-spring/annotation-dirtiescontext.html) > > > . > > > For Pulsar, our unit tests are integration tests in many cases. The > > > improvements should go in PulsarTestContext and possibly building a > > > caching solution for Junit 5 so that a context could be shared when > > > the state of the in-memory Pulsar broker isn't relevant for a > > > particular test and state isolation can be handled in other ways such > > > as unique topic names for each test case as we already do. > > > > > > For the actual integration tests, we should decouple the Pulsar > > > cluster initialization from the test classes. This way we won't have > > > to start merging test classes and other tricks because of technical > > > reasons. I think that a PIP to document the design would be useful so > > > that we can find the proper direction which will be feasible also in > > > the future. > > > > > > > > > > > I consider my proposal received positive feedback in this thread, so I > > > > will move on and raise a PR for this change. > > > > > > Yes, extracting the integration test libraries is something that will > > > be useful. I'd also recommend taking it to the direction where test > > > resource initialization is decoupled from the base test classes. > > > That's something that has been done with the PulsarTestContext > > > solution for the "unit tests" (which are integration tests too). This > > > could happen iteratively so that the first step is to simply decouple > > > the test framework and the test classes in tests/integration. > > > > > > -Lari
