elharo commented on code in PR #1062:
URL: https://github.com/apache/maven/pull/1062#discussion_r1140831831


##########
maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java:
##########
@@ -544,6 +544,19 @@ public void populatePropertiesOverwrite() throws Exception 
{
         assertThat(request.getUserProperties().getProperty("x"), is("false"));
     }
 
+    @Test
+    public void testPropertiesInterpolation() throws Exception {

Review Comment:
   There should also be a test for the exceptional case. 



##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -562,6 +566,21 @@ private void commands(CliRequest cliRequest) {
     // Maybe it's better to move some of those methods to separate class (SoC).
     void properties(CliRequest cliRequest) {
         populateProperties(cliRequest.commandLine, 
cliRequest.systemProperties, cliRequest.userProperties);
+
+        // now that we have properties, interpolate all arguments
+        BasicInterpolator interpolator = 
createInterpolator(cliRequest.systemProperties, cliRequest.userProperties);
+        CommandLine.Builder commandLineBuilder = new CommandLine.Builder();
+        cliRequest.commandLine.getArgList().stream()
+                .map(s -> {
+                    try {
+                        return interpolator.interpolate(s);
+                    } catch (InterpolationException e) {
+                        throw new RuntimeException("Unable to interpolate", e);

Review Comment:
   This should not be a runtime exception since it comes from input external to 
the program. InterpolationException might be OK, or perhaps a different checked 
exception. 
   
   If this means, the code doesn;t use lambdas, that's fine. 



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