gnodet-bot commented on code in PR #28000:
URL: https://github.com/apache/camel/pull/28000#discussion_r4302146924
##########
docs/user-manual/modules/ROOT/pages/jbang-commands/camel-jbang-test-agent-run.adoc:
##########
@@ -0,0 +1,43 @@
+
+// AUTO-GENERATED by camel-package-maven-plugin - DO NOT EDIT THIS FILE
+= camel test agent run
+
+Runs tests on the agent server
+
+NOTE: This command is provided by the `test` plugin. Install it with `camel
plugin add test`.
+
+
+== Usage
+
+[source,bash]
+----
+camel test agent run [options]
+----
+
+
+
+== Options
+
+[cols="2,5,1,2",options="header"]
+|===
+| Option | Description | Default | Type
+| `--async` | Should the test engine print verbose test summary information. |
| boolean
Review Comment:
⚠️ **Wrong description for `--async`:** the description "Should the test
engine print verbose test summary information." is identical to `--verbose`'s
description (line 38). This is a copy-paste bug in the upstream Citrus
`AgentRun.java` (`@Option(names = "--async", description = "Should the test
engine print verbose test summary information.")`) — it was faithfully
reflected here by the new reflection-based extractor.
Since the generated docs are checked in, users will see the wrong
description. Consider either:
1. Tracking a bug upstream in Citrus to fix `AgentRun.java`, or
2. Adding a note in the PR/JIRA that these docs contain a known upstream
annotation bug.
##########
docs/user-manual/modules/ROOT/pages/jbang-commands/camel-jbang-test-agent-run.adoc:
##########
@@ -0,0 +1,43 @@
+
+// AUTO-GENERATED by camel-package-maven-plugin - DO NOT EDIT THIS FILE
+= camel test agent run
+
+Runs tests on the agent server
+
+NOTE: This command is provided by the `test` plugin. Install it with `camel
plugin add test`.
+
+
+== Usage
+
+[source,bash]
+----
+camel test agent run [options]
+----
+
+
+
+== Options
+
+[cols="2,5,1,2",options="header"]
+|===
+| Option | Description | Default | Type
+| `--async` | Should the test engine print verbose test summary information. |
| boolean
+| `--background` | When enabled the command is not blocking for the test
result response. | | boolean
+| `--dep` | Set of additional Maven dependencies that should be loaded with
the agent. | | String[]
+| `--engine` | Name of the test engine that is used ti run tests. One of
junit, junit-jupiter, junit4, testng, cucumber | | String
Review Comment:
🔤 **Typo in `--engine` description:** "is used **ti** run tests" should be
"is used **to** run tests". Same upstream Citrus annotation bug — appears in
`AgentRun.java`, `AgentStart.java`, and thus in both generated `.adoc` files
and the JSON metadata.
##########
tooling/maven/camel-package-maven-plugin/src/main/java/org/apache/camel/maven/packaging/PrepareCamelJBangCommandsMojo.java:
##########
@@ -375,6 +378,70 @@ private void processSubcommandLine(
}
}
+ /**
+ * The source that registers the commands, for the imports of their
classes, or null when it cannot be parsed.
+ */
+ private JavaSource<?> parseSource(File file) {
+ try {
+ return Roaster.parse(file) instanceof JavaSource<?> source ?
source : null;
+ } catch (Exception e) {
+ getLog().debug("Could not parse: " + file + " - " +
e.getMessage());
+ return null;
+ }
+ }
+
+ /**
+ * The command of a class: from its source when it is one of the command
classes, or else from its class on the
+ * classpath of the project when the source that registers it imports it
by name, such as a command of a library
+ * that a plugin adds as it is (camel test run is the Run of Citrus).
+ */
+ private CommandInfo commandInfo(
+ String className, Map<String, CommandInfo> commandsByClassName,
JavaSource<?> registering) {
+ CommandInfo info = commandsByClassName.get(className);
+ if (info == null && registering != null) {
+ String imported = importedByName(registering, className);
+ if (imported != null) {
+ info = parseCommandFromClass(imported);
+ }
+ }
+ return info;
+ }
+
+ /**
+ * Parses a command from the picocli annotations of its class on the
classpath of the project, read by reflection,
+ * as picocli is not a dependency of this plugin.
+ */
+ @SuppressWarnings("unchecked")
+ private CommandInfo parseCommandFromClass(String className) {
+ try {
+ ClassLoader loader = getProjectClassLoader();
+ Class<?> clazz = loader.loadClass(className);
+ Annotation command
+ = clazz.getAnnotation((Class<? extends Annotation>)
loader.loadClass("picocli.CommandLine$Command"));
+ if (command == null) {
+ return null;
+ }
+ CommandInfo info = new CommandInfo();
+ info.className = clazz.getSimpleName();
+ info.qualifiedName = clazz.getName();
+ info.name = (String) attribute(command, "name");
+ String description = String.join(" ", (String[])
attribute(command, "description"));
+ // without the hint to the help, as for a command of the sources
+ if (description.contains("(use")) {
+ description = description.substring(0,
description.indexOf("(use")).trim();
+ }
+ info.description = description.isEmpty() ? null : description;
+ info.hidden = (Boolean) attribute(command, "hidden");
+ info.deprecated = clazz.isAnnotationPresent(Deprecated.class);
+ info.options = new ArrayList<>();
+ parseOptionsFromClass(className, info.options);
+ return info;
+ } catch (Exception | LinkageError e) {
+ getLog().debug("Could not read the command of class: " + className
+ " - " + e.getMessage());
Review Comment:
💡 **Silent failure at build time:** when `parseCommandFromClass` fails
(class not on project classpath, linkage error), it falls back to
`getLog().debug()`. If a library command class is missing at the time the
plugin runs, the command will simply be absent from the generated docs with no
visible warning to the user.
Consider promoting to `warn` so developers notice documentation gaps at
build time without failing the build:
```suggestion
getLog().warn("Could not read the command of class: " +
className + " - " + e.getMessage());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
