-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Add tests for camel-console module to increase coverage #21103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add comprehensive test suite for the camel-console module: - Add AbstractDevConsoleTest base class with helper methods - Create SimpleDevConsoleTest parameterized test for 23 simple consoles - Create RouteBasedDevConsoleTest parameterized test for 8 route consoles - Add 16 new test files covering most DevConsole implementations - Add ~130 test methods testing both TEXT and JSON output formats - Add tests for Configurer classes (auto-generated property configurers) - Add camel-management as test dependency to enable JMX support - Coverage increased from ~9% to ~37% Note: Some console classes (ProcessorDevConsole, ProducerDevConsole, EventConsole) require full JMX MBean infrastructure that isn't easily available in unit tests without significant setup. Further coverage improvements would require integration tests with full JMX management.
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🤖 CI automation will test this PR automatically. 🐫 Apache Camel Committers, please review the following items:
|
| */ | ||
| protected String callText(DevConsole console) { | ||
| String out = (String) console.call(DevConsole.MediaType.TEXT); | ||
| assertNotNull(out, "TEXT output should not be null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checking that it is not null is increasing coverage but it is not testing efficiently the expected output.
For instance, if it is a stacktracetrace which is returned because of an error, or if an empty string is provided, it won't be notice
it applies to a lot of places in the the PR
core/camel-console/src/test/java/org/apache/camel/impl/console/BeanDevConsoleTest.java
Outdated
Show resolved
Hide resolved
| Assertions.assertTrue(out.contains("myBean")); | ||
| Assertions.assertTrue(out.contains("anotherBean")); | ||
| Assertions.assertTrue(out.contains("TestBean")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
potential improvement: By using assertj Assertions.assertThat(out).containsAll("myBean", "anotherBean", "TestBean"), it gives a clearer error message in case of failure. it allows to see each text expected in a single time
core/camel-console/src/test/java/org/apache/camel/impl/console/BeanDevConsoleTest.java
Outdated
Show resolved
Hide resolved
| StringReader reader = new StringReader(""); | ||
|
|
||
| List<JsonObject> result = ConsoleHelper.loadSourceAsJson(reader, 1); | ||
| Assertions.assertNull(result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better that it returns an empty list of null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, probably a good improvement to do in ConsoleHelper in the future.
...camel-console/src/test/java/org/apache/camel/impl/console/DefaultDevConsoleRegistryTest.java
Outdated
Show resolved
Hide resolved
...camel-console/src/test/java/org/apache/camel/impl/console/DefaultDevConsoleRegistryTest.java
Outdated
Show resolved
Hide resolved
core/camel-console/src/test/java/org/apache/camel/impl/console/RouteBasedDevConsoleTest.java
Outdated
Show resolved
Hide resolved
core/camel-console/src/test/java/org/apache/camel/impl/console/SimpleDevConsoleTest.java
Outdated
Show resolved
Hide resolved
- Remove log.info() calls from test helper methods to avoid slowness - Use AssertJ for clearer assertion error messages - Use assertInstanceOf instead of assertTrue(x instanceof Y) - Remove redundant assertNotNull before assertInstanceOf - Remove null checks for expectedJsonKey in parameterized tests - Use correct JSON keys that are always present in console output
50600c0 to
4344e5e
Compare
Add comprehensive test suite for the camel-console module:
Note: Some console classes (ProcessorDevConsole, ProducerDevConsole,
EventConsole) require full JMX MBean infrastructure that isn't easily
available in unit tests without significant setup. Further coverage
improvements would require integration tests with full JMX management.