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

Reply via email to