Hi,
On Thu, Jul 20, 2023 at 10:08 AM Claus Ibsen <claus.ib...@gmail.com> wrote: > Hi > > Sorry at this time I cannot support these kinds of changes. > > We have a standard practice in Camel on how to do unit tests (camel-test) > which we had for 15+ years. > We should dog food our own standard, and it's also a way for end users to > see existing src/test code, how to use a given Camel component, and also > how they can write tests. > I totally understand the need for dog food (which is why the idea is to convert our own tests before). But at the same time, I (respectfully) disagree with the need to do something "just because we did it for 15 years" - to be clear: I am not saying we should crazilly break backwards compatibility, I am just advocating for us to move forward to more modern ways of doing things. One thing that concerns me is that ... our tests are not really in a good shape. They are flaky, some of them were never properly modernized to match JUnit 5, incorrectly written, undocumented, Just as an example: we have been without a stable / green build since January. I am sorry to say this, but this is not an adequate state for our tests to be (this is in part why I have been raising the discussions about compilable commits and ensuring the code is tested/testable before we merge them). > > If I compare the Camel standard today vs test-infra in a PR like this > https://github.com/apache/camel/pull/10739/files > > Then that "new code" is worse and more hard to write, understand and do for > end users. > Granted: there are situations in which the test infra would require more code. But there's a few reasons for that: - As we convert more code, we can identify situations where we duplicate code, and then create the reusable bits for that. - There are some bits already that could have been added to improve the original submission, but keep in mind that it was done by an inexperienced contributor (one that is learning the path of contributing to Camel under my guidance) - Some of that code is there to reduce the migration effort (but it could be made much simpler if we decide not to do that) > > For advanced situations with test-containers / docker / embedded some > database (or other systems) then I agree its good to have some "test-infra" > that can more easily setup this. > Which was my understanding of the need for test-infra in the first place. > > But for plain tests then camel-test should be used. > My view from the test infra is that it provides infrastructure for testing (be it containers, embedded instances or a CamelContext runtime). > > Also we should not change all this code in components "just because" as we > also need to backport fixes to 3.x branches and its always a pain when > there have been too many changes > (yes it's a bit ironic coming from me, who does many changes on the main > branch for the v4 work). > That just risks us not doing backports because it was too painful and time > consuming. > > In terms of Quarkus testing, then mind that camel-core is an Apache project > and not Quarkus. Such things are potentially better done in the > camel-quarkus project itself. > I had made a clarification earlier regarding the Quarkus live testing, but the message did not go through, so let me quote this again: "What I mean is that I hope we could implement a feature that works similarly to Quarkus Continuous Testing, but within Camel (i.e, like when running mvn camel:run or similar stuff)." So, IOW, it's not about supporting that ... but, instead, implementing a similar one within Camel and creating a foundation on which others can leverage these features. That would allow the implementation of even more interesting features (i.e.: Kaoto, Karavan, or Jenkins could bundle plugins that instead of running the tests locally, deploy the tests in a Kubernetes/OpenShift environment and run it there). > > In terms of improving camel-test to for example not need to extend a base > class, then all of that work was started as part of camel-test-spring (such > as: > > https://github.com/apache/camel/blob/main/components/camel-test/camel-test-spring-junit5/src/main/java/org/apache/camel/test/spring/junit5/MockEndpoints.java > ) > > There is an old JIRA to improve and add more features, and today those > annotation based configuration could IMHO be moved out of camel-test-spring > into the general camel-test, to make it more generic. > https://issues.apache.org/jira/browse/CAMEL-6070 > > Over the years some people have stepped up to help with this, but it was > bigger work than they anticipated, so we didn't finish all the work. And > what we have already covers a decent size of use-cases. > So, in theory, what we are trying to achieve with test-infra-core is pretty similar. Just add a couple of annotations and JUnit handles the rest for you (and you can focus on the routes and on the test). But, this does it without requiring Spring (which, IMHO, is another benefit). That said, although this is a topic I am intensely passionate about and I truly believe this could open the room for a LOT more features for Camel, our community and the vendors that build on top of Camel, I am not interested in fractionating/disrupting the community over this. If the consensus is that this is not the way to move forward, then I'll stop this effort, revert the converted code and finish all work on this. Kind regards > > > > On Thu, Jul 20, 2023 at 8:38 AM Otavio Rodolfo Piske <angusyo...@gmail.com > > > wrote: > > > Hi Claus, > > > > test infra solves a few problems that we have in > camel-test-support-junit5: > > - it has a simpler dependency chain (it depends only on core) > > - it allows managing the service ordering explicitly (i.e.: it's possible > > to define in which order services can be initialized) > > - it is much simpler and the JUnit 5 extension model gives us more > > flexibility for additional features in the future (more on that later) > > - it's not tightly coupled to one specific type of context or it's > > lifecycle (i.e.: consider for instance how we have to set > isUseRouteBuilder > > to skip adding route builders on CamelTestSupport based tests) > > > > So, for instance, instead of tightly coupling a test to CamelTestSupport > by > > extending it ... we can just use JUnit's 5 extension model to add a > context > > to the test: > > > > @RegisterExtensionpublic static CamelContextExtension > > camelContextExtension = new DefaultCamelContextExtension(); > > > > > > (+ the required setup of routes and context configuration - which is > > required CamelTestSupport as well). > > > > Ultimately my hope is that we can harden this enough so that we can start > > publicizing this to the end users and envolve it to support other > features > > (like Quarkus Continuous Testing - which, IMHO, would require a lot more > > coordination with the service and context lifecycle than what we can do > > with CamelTestSupport). > > > > What do you think? > > > > Kind regards > > > > > > On Thu, Jul 20, 2023 at 7:24 AM Claus Ibsen <claus.ib...@gmail.com> > wrote: > > > > > Hi > > > > > > I noticed that we are migrating some components to use camel-test-infra > > for > > > unit testing. > > > And I think it makes sense when you have > > > > > > - need to use docker / test-containers > > > - or to setup some internal embedded system that usually would require > a > > > base class with some code to do that. > > > > > > However for basic components that are not in need for that then we > should > > > not use camel-test-infra but just regular camel-test, just as camel end > > > users will do that. > > > > > > For example this PR > > > https://github.com/apache/camel/pull/10739 > > > > > > I fail to see the need for test-infra-core for camel-asn1. This is > just a > > > message transformation (dataformat) library that works purely > in-memory. > > > > > > > > > > > > -- > > > Claus Ibsen > > > ----------------- > > > @davsclaus > > > Camel in Action 2: https://www.manning.com/ibsen2 > > > > > > > > > -- > > Otavio R. Piske > > http://orpiske.net > > > > > -- > Claus Ibsen > ----------------- > @davsclaus > Camel in Action 2: https://www.manning.com/ibsen2 > -- Otavio R. Piske http://orpiske.net
