gnodet commented on PR #21979:
URL: https://github.com/apache/camel/pull/21979#issuecomment-4055120538

   ## Review notes — potential downsides to consider
   
   ### 1. CLI backward compatibility
   The old `CitrusExecutionStrategy` forwarded *any* command to Citrus JBang. 
The new approach only exposes `init` and `run` as picocli subcommands. Users 
who relied on other Citrus JBang commands or Citrus-specific flags not 
re-implemented in `TestRun` would break or see them silently ignored.
   
   ### 2. Templates bundled & may drift
   The Citrus test templates (`citrus-yaml.tmpl`, `citrus-xml.tmpl`, etc.) are 
now embedded in the plugin rather than coming from Citrus JBang. If Citrus 
upstream updates their templates, ours won't pick up the changes — they'd need 
manual syncing.
   
   ### 3. Larger launcher footprint
   The test plugin was previously excluded from the launcher with the comment 
*"the citrus test plugin cannot be embedded"*. Now it's pre-installed, pulling 
`citrus-base` and its transitive dependencies into the launcher JAR.
   
   ### 4. System property side effects
   `TestRun.doCall()` calls `System.setProperty()` for user-provided `-p` 
properties but never cleans them up. In a long-running process or if the 
launcher is reused, these properties leak into subsequent operations.
   
   ### 5. Reduced Citrus feature coverage
   The old approach delegated entirely to Citrus JBang, so new Citrus features 
were automatically available. The new in-process `TestRun` reimplements test 
resolution and configuration — if Citrus adds new options, they won't be 
available until we update our code.
   
   ### 6. No test coverage
   `TestInit` and `TestRun` have no unit tests. Given that `TestRun` has 
non-trivial logic (directory scanning, path resolution, configuration 
building), some test coverage would be valuable.
   
   ---
   
   The architectural improvement (in-process vs spawning JBang) is the right 
direction. It would be good to coordinate with the Citrus project to ensure the 
in-process API (`TestEngine`, `TestRunConfiguration`) is the officially 
supported integration path.


-- 
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]

Reply via email to