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
