CamelTestSupport style of testing #3511

[JiriOndrusek]

fixes #3511

This is a draft with a simple POC to start a discussion about camel way of testing in camel-quarkus.

I tried to add a functionality of CamelTestSupport into the camel-quarkus.
I created our own annotation CamelQuarkusTest, which should be helpful for creating some Processors to enhance test functionality, but in this POC it is unused.

I copied some tests from the camel core tests (from this module/package repo), which should cover bigger part of the required functionality.

As you can see in the draft, each test has to be annotated with @CamelQuarkusTest and should extend CamelQuarkusTestSupport.

There are a few limitations:

• Quarkus runs all test within the smallest number of restarting quarkus engine (which means that quarkius starts before all tests and finishes after all the tests), therefore Camel context is created and started at the beginning of the lifecycle. Therefore tests has to be aware, that the context could be modified by the other tests (and reset mock endpoints for example or take care of it globally). To avoid this behavior, the simplest (but not performance wise) solution is to add annotation @TestProfile, which instructs Quarkus to restart the engine before running the test.
• The fact that the camel context is already started, limits several usecases, which requires context restart (like debugger).
• JUnit5 is not aware of quarkus's classloader and context, therefore all code (like junit5's callback after/before *) has to be rewritten into the Quarkus's callbacks (see for example BeforeEachCallback. added by this draft)
• Properties override does not work, but it has to be investigated further whether this functionality should work in camel-quarkus (Test UseOverridePropertiesWithPropertiesComponentTest)
• I enhanced a Injectionpointsprocessor to detect duplications of the synthetic beans, produced by tests. This code is just a "basic" POC to show theway of proper solution. (We can not register synthetic beans for each tests, becauseall of them are part of the context and there can be no duplicated beans. It is not possible to produce different beans with the same name)

Some little changes has to be done also on camel side (to allow overriding of some test methods). For the POC, following changes are necessaery: apache/camel@camel-3.17.0...JiriOndrusek:camel-quarkus-test-support

WDYT? @ppalaga @jamesnetherton @zhfeng @aldettinger

I thinng that this "feature" is quite complicated and should be discussed. In this POC I tried to show one possible way of resolving some problems. From my POV similar approach should work and should be helpful for users. In case we can add the full stop and start capabilty of the camel context, this would help this case a lot. Even without it, users should be able to use a big part of the functionality of camel test support. In case that tests are written safely, there might not be necessary to use @TestProfile and therefore avoid the performance issues.

[essobedo]

Just to let you know that in case of camel-test-main, we propose an implementation that is purely annotation based to avoid extending any classes like in SpringBoot more details in https://issues.apache.org/jira/browse/CAMEL-17690 / apache/camel#7041

[JiriOndrusek]

@essobedo Thanks for showing me the latest approach used in camel. My POC aims to help users migrating the old tests extending CamelTestSupport. I'll look into the feature you shared to me tomorrow to see wether it covers also this feature.

[essobedo]

@essobedo My POC aims to help users migrating the old tests extending CamelTestSupport.

@JiriOndrusek I see it makes sense indeed. It was just in case it could be helpful to know sorry for the noice 😄

[zhfeng]

The fact that the camel context is already started, limits several usecases, which requires context restart (like debugger).

I think we have an option to disable the autoStart?

camel-quarkus/extensions-core/core/runtime/src/main/java/org/apache/camel/quarkus/core/CamelConfig.java

Lines 75 to 82 in f8604b5

	@ConfigGroup
	public static class BootstrapConfig {
	/**
	* When set to true, the {@link CamelRuntime} will be started automatically.
	*/
	@ConfigItem(defaultValue = "true")
	public boolean enabled;
	}

[JiriOndrusek]

@zhfeng Thanks for pointing to the config! I'll try to integrate it into the PoC

[zhfeng]

@JiriOndrusek But I'm not very sure that the option is effect since we have no any test for bootstrap options. :(

[jamesnetherton]

I think we should create a new top level module for this stuff. I see integration-tests-support as an internal thing to support our project and not something that we put into the hands of our users.

So maybe we do like Quarkus does and have a test-framework module where we can build up a set of testing libraries. So initially something like test-framework/junit5. Which would give us an artifact named similar to the plain Camel equivalent org.apache.camel.quarkus:camel-quarkus-junit5.

Agree with @essobedo that annotation driven tests would be good to have eventually. The more consistency we have with the other Camel runtimes, the better it is for our users. But lets take things one step at a time....

[JiriOndrusek]

@jamesnetherton creating a test-framework/junit5 sounds nice. I' refactor the draft to do it (I used integration-tests-support/test-support-jvm in the PoC so far).
Just an idea,because we aim the functionality for the JVM only, would it make sense to add -jvm to the module?
(I'm not sure in what time-frame we can make it work in native)

Using annotation driven tests seems to be nice. I still think that this mid-step (supporting old testing approaches for migration of tests) makes sense.

[jamesnetherton]

would it make sense to add -jvm

I'd prefer to not add suffixes to the artifact ids and cover native not being supported with documentation and / or code that can detect that scenario.

[JiriOndrusek]

@jamesnetherton I reworked the draft and put the sources into test-framework/junit5.

It's possible to not use @TestProfile. I managed to limit the number of @TestProfile annotations to 2 in the selected tests subset

• The main problem is caused by not stopping/starting the camel context after each test. I'll try to dig into the conf. option mentioned by @zhfeng.
  (for example following interceptor InterceptSendToMockEndpointStrategy is not unregistered after the use, thus affecting following tests.)

I'm not sure how find all features which may be affected but not-restarting came context. Therefore introducing this functionality for the test would be 100% covering solution (in comparison to clear the context in quarkus test callbacks)

[JiriOndrusek]

Change in Camel main is prepared as apache/camel#7813

[JiriOndrusek]

@jamesnetherton The selection of tests, which are currently in the draft, works -> all of them are successful.
There are several "problems to be solved":

• I have to change detection of synthetic bean removal, because it doesn't consider parents - it should be easy
• There is a test asserting number of calls of some callbacks (they are different in 'perClass' vs 'perMethod' mode) - refactor is not finished
• I have to configure logging to mimic the behavior of camel
• Probably more small issues to be covered

I'm leveraging the fact, that I can "suspend" and "resume" camel context. If context is suspended, camel skips several configurations (like start of the routes) because camel "thinks" that context is not started. I can then resume context and make it work even when context was in reality not stopped.

I know that this is still a PoC, but it seems promising. I personally think that i need to run more tests using CamelQuarkusTestSupport to be sure it works and covers all we need.

I plan to create copies of "an interesting tests" from camel (usually core ones), which could "break" this feature - to see how stable it is.
Do you have any better idea how to cover all we need?

[ppalaga]

Is there already a piece of user guide that would sketch how to use this? I have not seen it in this PR, but maybe I have just overseen it because there are many files.

[jamesnetherton]

I plan to create copies of "an interesting tests" from camel (usually core ones), which could "break" this feature - to see how stable it is.

That seems like a decent approach to me. I'm guessing you've already done this, but there are a bunch of test framework tests in Camel that we could probably take inspiration from in these modules:

https://github.com/apache/camel/tree/main/components/camel-test

BTW if you want the CI build to stay stable, I'd avoid camel-main as the target branch since it's volatile and often broken. If you only need a small change made in Camel 3.18, then it might be easier to temporarily copy that code here until the release is available.

[JiriOndrusek]

s there already a piece of user guide that would sketch how to use this? I have not seen it in this PR, but maybe I have just overseen it because there are many files.

Missing doc is another issue here, I forgot to write it into my comment. Thanks for noticing.

I'll write the documentation.

[JiriOndrusek]

I know that this PR brings a lot of changes, but from my PoV it is ready to merge.
I'd like to ask for some (at least partial) approvals here.
(I'm mentioning everyone who commented in this PR)
@jamesnetherton @aldettinger @zbendhiba @ppalaga @zhfeng

[ppalaga]

Some suggestions inline

[JiriOndrusek]

@ppalaga Thanks for the above comments. I'll incorporate it into the PR.

[JiriOndrusek]

Majority of the issues is fixed, I'll close appropriate conversations tomorrow morning.

[JiriOndrusek]

I closed appropriate conversations and kept 3 opened - with my questions.
@ppalaga There is one comment, which I cannot resolve (there are co options in UI to make a comment or to resolve). It is fixed.

Add some checks asserting that CamelTestSupport.context == CamelQuarkusTestSupport.context. E.g. in
override of context() method
override of bindToRegistry()
override of postProcessTest()

[JiriOndrusek]

@ppalaga There are 2 tests, which asserts whether methods like afterAll was called (and the number of their executions) . In camel component code the test starts runnable and logs an error if there is a problem after a while (see test
Unfortunately there is no way how to fail a test with such failure.

I kept the same code in our tests

• and I created second test, which is alphabetically after the first one (CallbacksPerTestFalse01Test and CallbacksPerTestFalse02Test). The second test verifies the tmp files (files in /target/) and fails in case that there is a problem. For that purpose I configured maven-surefire-plugin to use alphabetical order of test classes. I know that this is not standard approach, so I'm pointing at it, whether it is acceptable.

[JiriOndrusek]

I found a small difference between CaqmelTestSupport and CamelQuarkusTestSupport.
CameTestSupport offers a method beforeAll -> which execution depends on @TestInstance(TestInstance.Lifecycle.PER_CLASS) vs @TestInstance(TestInstance.Lifecycle.PER_METHOD)
I debugged the tests and discovered, that in the test class with 3 test methods

• with Lifecycle.PER_CLASS is executed 1 time.
• with Lifecycle.PER_METHOD is executed 0 time.

The closest method to use with similar use-case in camel-QuarkusTestSupport`` is doAfterConstruct. But it is called in different way (in the test class with 3 test methods):

• With Lifecycle.PER_CLASS is executed 1 time.
• With Lifecycle.PER_METHOD is executed 3 time.

I'll add the difference into the documentation. I just wanted to point it out.

[jamesnetherton]

@JiriOndrusek Are we done with changes now?

[JiriOndrusek]

@JiriOndrusek Are we done with changes now?

Yes, the last 2 changes was the fix pointed by my last comment. Sorry for not stating clearly, that work is finished.

[ppalaga]

Thanks for the great work, @JiriOndrusek !

[zbendhiba]

Many thanks @JiriOndrusek for the great work !