Uploaded image for project: 'Maven'
  1. Maven
  2. MNG-6220

Add CLI options to control color output

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.5.2
    • Component/s: None
    • Labels:
      None

      Description

      Currently, the only way to enable/disable color output is to use the batch-mode or log-file options.

      If a user wants colored output but no interactivity (ie: jenkins environment with the ansicolor plugin), there is no CLI option combination to support the use-case.

      I propose to add an option to control output coloring directly.

      --color=enabled <- color output always enabled
      --color=disabled <- color output always disabled
      --color=auto <- current behavior (default)
      

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user mryan43 opened a pull request:

          https://github.com/apache/maven/pull/114

          MNG-6220 add color CLI option

          https://issues.apache.org/jira/browse/MNG-6220

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/mryan43/maven master

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/maven/pull/114.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #114


          commit 472a8866549ed1ee4e2dd53b8f4ff219c7d2f95a
          Author: mryan <ryan@shamu.ch>
          Date: 2017-04-18T08:57:23Z

          MNG-6220 add color CLI option


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user mryan43 opened a pull request: https://github.com/apache/maven/pull/114 MNG-6220 add color CLI option https://issues.apache.org/jira/browse/MNG-6220 You can merge this pull request into a Git repository by running: $ git pull https://github.com/mryan43/maven master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/maven/pull/114.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #114 commit 472a8866549ed1ee4e2dd53b8f4ff219c7d2f95a Author: mryan <ryan@shamu.ch> Date: 2017-04-18T08:57:23Z MNG-6220 add color CLI option
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user michael-o commented on the issue:

          https://github.com/apache/maven/pull/114

          I will comment shortly.

          Show
          githubbot ASF GitHub Bot added a comment - Github user michael-o commented on the issue: https://github.com/apache/maven/pull/114 I will comment shortly.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user michael-o commented on the issue:

          https://github.com/apache/maven/pull/114

          They are several issues. People are used to [GNU](https://www.gnu.org/software/grep/manual/grep.html#General-Output-Control) and [BSD](https://www.freebsd.org/cgi/man.cgi?query=grep&sourceid=opensearch) tools like grep. They use values `never`, `always`, `auto`. Having said that I do think we need to do this exactly the same way: [`grep.c`](https://github.com/freebsd/freebsd/blob/master/usr.bin/grep/grep.c#L627-L647). Option `always` requires special attention due to `isatty` in combination with `less(1)`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user michael-o commented on the issue: https://github.com/apache/maven/pull/114 They are several issues. People are used to [GNU] ( https://www.gnu.org/software/grep/manual/grep.html#General-Output-Control ) and [BSD] ( https://www.freebsd.org/cgi/man.cgi?query=grep&sourceid=opensearch ) tools like grep. They use values `never`, `always`, `auto`. Having said that I do think we need to do this exactly the same way: [`grep.c`] ( https://github.com/freebsd/freebsd/blob/master/usr.bin/grep/grep.c#L627-L647 ). Option `always` requires special attention due to `isatty` in combination with `less(1)`.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user michael-o commented on the issue:

          https://github.com/apache/maven/pull/114

          Two issues:

          1. Unfortunately, Commons CLI does not offer valid values. If it would, it would fail if the option value is wrong. I would do so too.
          2. You blindly `return` from `always`, but still forget that Jansi will ignore that because `isatty` returns false as soon as `stdout` is passed via pipe.

          Show
          githubbot ASF GitHub Bot added a comment - Github user michael-o commented on the issue: https://github.com/apache/maven/pull/114 Two issues: 1. Unfortunately, Commons CLI does not offer valid values. If it would, it would fail if the option value is wrong. I would do so too. 2. You blindly `return` from `always`, but still forget that Jansi will ignore that because `isatty` returns false as soon as `stdout` is passed via pipe.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mryan43 commented on the issue:

          https://github.com/apache/maven/pull/114

          Good idea to align the name of choices with grep, I just changed it.

          About "isatty", do you think we should set the jansi.force system property when --color is set to "always" ? https://github.com/fusesource/jansi/blob/2616142fda4425d779ac94a3d9bfa76412021b23/jansi/src/main/java/org/fusesource/jansi/AnsiConsole.java

          Show
          githubbot ASF GitHub Bot added a comment - Github user mryan43 commented on the issue: https://github.com/apache/maven/pull/114 Good idea to align the name of choices with grep, I just changed it. About "isatty", do you think we should set the jansi.force system property when --color is set to "always" ? https://github.com/fusesource/jansi/blob/2616142fda4425d779ac94a3d9bfa76412021b23/jansi/src/main/java/org/fusesource/jansi/AnsiConsole.java
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user michael-o commented on the issue:

          https://github.com/apache/maven/pull/114

          I think yes, grep doesn't do different. Please try the patch thoroughly!

          Show
          githubbot ASF GitHub Bot added a comment - Github user michael-o commented on the issue: https://github.com/apache/maven/pull/114 I think yes, grep doesn't do different. Please try the patch thoroughly!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mryan43 commented on the issue:

          https://github.com/apache/maven/pull/114

          I will test with less and jenkins and comment again with results.

          Show
          githubbot ASF GitHub Bot added a comment - Github user mryan43 commented on the issue: https://github.com/apache/maven/pull/114 I will test with less and jenkins and comment again with results.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mryan43 commented on the issue:

          https://github.com/apache/maven/pull/114

          Unfortunately, the system property is read only once in a static init block in the JansiConsole class before the cli main method is invoked, so my change doesn't work.

          We have a kind of chicken and egg situation and we need to decide if we prefer

          a) not setting up Jansi until arguments/config have been parsed and validated (and call MessageUtils.systemInstall(); later in the loggin() method)

          b) forcing jansi until arguments and config have been parsed (by adding System.setProperty( "jansi.force", "true" ); before MessageUtils.systemInstall(); in MavenCli#main and then eventually disable it later in the loggin() method

          What do you think ?

          Show
          githubbot ASF GitHub Bot added a comment - Github user mryan43 commented on the issue: https://github.com/apache/maven/pull/114 Unfortunately, the system property is read only once in a static init block in the JansiConsole class before the cli main method is invoked, so my change doesn't work. We have a kind of chicken and egg situation and we need to decide if we prefer a) not setting up Jansi until arguments/config have been parsed and validated (and call MessageUtils.systemInstall(); later in the loggin() method) b) forcing jansi until arguments and config have been parsed (by adding System.setProperty( "jansi.force", "true" ); before MessageUtils.systemInstall(); in MavenCli#main and then eventually disable it later in the loggin() method What do you think ?
          Hide
          rfscholte Robert Scholte added a comment -

          I really wonder if there's a need for this in the form of a commandline option. Arnaud HERITIER WDYT? My idea is that we need to introduce extension points for better CI support, just like we did with Maven 3 for better IDE support.

          Show
          rfscholte Robert Scholte added a comment - I really wonder if there's a need for this in the form of a commandline option. Arnaud HERITIER WDYT? My idea is that we need to introduce extension points for better CI support, just like we did with Maven 3 for better IDE support.
          Hide
          aheritier Arnaud HERITIER added a comment -

          Hi Robert Scholte, Manuel Ryan

          I agree that it might be interesting to have the ability to programatically activate / deactivate the color output but from my POV a command line option is too much. It might be an extension point like proposed by Robert or a system property but not more. End users (developers) won't really need it. This is only useful for IDE, CI, .. and others integrations like mentioned here.

          Show
          aheritier Arnaud HERITIER added a comment - Hi Robert Scholte , Manuel Ryan I agree that it might be interesting to have the ability to programatically activate / deactivate the color output but from my POV a command line option is too much. It might be an extension point like proposed by Robert or a system property but not more. End users (developers) won't really need it. This is only useful for IDE, CI, .. and others integrations like mentioned here.
          Hide
          mryan Manuel Ryan added a comment -

          Hello Robert Scholte and Arnaud HERITIER

          A system property or environment variable would be ok for CI use cases, but a programmatic extension point for this is a bit more cumbersome as it would need specific integration code in all CI systems when they already support cli options and env vars out of the box.

          We also need to consider how existing CLI options influence color (-B and -l). Configuring color behavior elsewhere could make it a harder to understand what is happening as some cli options would be starting to override system properties or vice-versa.

          Show
          mryan Manuel Ryan added a comment - Hello Robert Scholte and Arnaud HERITIER A system property or environment variable would be ok for CI use cases, but a programmatic extension point for this is a bit more cumbersome as it would need specific integration code in all CI systems when they already support cli options and env vars out of the box. We also need to consider how existing CLI options influence color (-B and -l). Configuring color behavior elsewhere could make it a harder to understand what is happening as some cli options would be starting to override system properties or vice-versa.
          Hide
          rfscholte Robert Scholte added a comment -

          Manuel Ryan Just to let you know: Arnaud is working on both Maven and Jenkins (Cloudbees).

          We should try to only keep a limit set of CLI options. Based on Arnauds answer there's no need for such an option. And to me that makes sense: the output for Maven (console) and Jenkins (webpage) is different, so Jenkins must do output parsing anyway to enable/disable color support.

          Show
          rfscholte Robert Scholte added a comment - Manuel Ryan Just to let you know: Arnaud is working on both Maven and Jenkins (Cloudbees). We should try to only keep a limit set of CLI options. Based on Arnauds answer there's no need for such an option. And to me that makes sense: the output for Maven (console) and Jenkins (webpage) is different, so Jenkins must do output parsing anyway to enable/disable color support.
          Hide
          aheritier Arnaud HERITIER added a comment -

          I'm interested to have the batch mode with color but a system property could be enough

          Show
          aheritier Arnaud HERITIER added a comment - I'm interested to have the batch mode with color but a system property could be enough
          Hide
          michael-o Michael Osipov added a comment -

          Robert Scholte, in my opinion there is. If you are used to $command --color=always | less -R you want to have this with Maven. Even if I redirect the Maven output, I'd like to page through the file with colors. Nobody forces you to invoke it, but you can if you need to. I will make it simply consistent with many other tools.

          Show
          michael-o Michael Osipov added a comment - Robert Scholte , in my opinion there is. If you are used to $command --color=always | less -R you want to have this with Maven. Even if I redirect the Maven output, I'd like to page through the file with colors. Nobody forces you to invoke it, but you can if you need to. I will make it simply consistent with many other tools.
          Hide
          rfscholte Robert Scholte added a comment -

          I don't think that colors are build specific properties. I can't think of a reason why you want to have colors for some builds and no colors for others. Instead it is a global settings for your environment/system. Hence, SystemProperties makes more sense to me.

          Show
          rfscholte Robert Scholte added a comment - I don't think that colors are build specific properties. I can't think of a reason why you want to have colors for some builds and no colors for others. Instead it is a global settings for your environment/system. Hence, SystemProperties makes more sense to me.
          Hide
          michael-o Michael Osipov added a comment -

          Can you elaborate? How would you implement mvn --color=always | less -R?

          Show
          michael-o Michael Osipov added a comment - Can you elaborate? How would you implement mvn --color=always | less -R ?
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user mryan43 opened a pull request:

          https://github.com/apache/maven-shared/pull/22

          Add color support auto detection in MessageUtils

          Linked to PR https://github.com/apache/maven/pull/114 and task https://issues.apache.org/jira/browse/MNG-6220

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/mryan43/maven-shared trunk

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/maven-shared/pull/22.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #22


          commit 6f3e71b3a2d9cc57b4a45a7d4bd354094cebcc5c
          Author: mryan43 <ryan@shamu.ch>
          Date: 2017-04-20T22:38:39Z

          add utility method to auto detect color support

          commit 6839dd1d45a71d0667c58460a4e5a0e9588ea1f7
          Author: mryan43 <ryan@shamu.ch>
          Date: 2017-04-21T07:08:06Z

          test for jansi availability when auto detecting color support


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user mryan43 opened a pull request: https://github.com/apache/maven-shared/pull/22 Add color support auto detection in MessageUtils Linked to PR https://github.com/apache/maven/pull/114 and task https://issues.apache.org/jira/browse/MNG-6220 You can merge this pull request into a Git repository by running: $ git pull https://github.com/mryan43/maven-shared trunk Alternatively you can review and apply these changes as the patch at: https://github.com/apache/maven-shared/pull/22.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #22 commit 6f3e71b3a2d9cc57b4a45a7d4bd354094cebcc5c Author: mryan43 <ryan@shamu.ch> Date: 2017-04-20T22:38:39Z add utility method to auto detect color support commit 6839dd1d45a71d0667c58460a4e5a0e9588ea1f7 Author: mryan43 <ryan@shamu.ch> Date: 2017-04-21T07:08:06Z test for jansi availability when auto detecting color support
          Hide
          rfscholte Robert Scholte added a comment -

          Let's do it like all colors are controlled as well:
          e.g.
          set MAVEN_OPTS=-Dstyle.color=enabled
          This is one option to enable it for every build.

          mvn | less -R
          or
          mvn -Dstyle.color=enabled | less -R

          Show
          rfscholte Robert Scholte added a comment - Let's do it like all colors are controlled as well: e.g. set MAVEN_OPTS=-Dstyle.color=enabled This is one option to enable it for every build. mvn | less -R or mvn -Dstyle.color=enabled | less -R
          Hide
          michael-o Michael Osipov added a comment -

          I still fail to see how you want to enfore Jansi to write color codes even if stdout is not attached to a terminal? the enum Style won't do this. Do you want to read it from MavenCli? Though, the property itself is fine for me.

          Show
          michael-o Michael Osipov added a comment - I still fail to see how you want to enfore Jansi to write color codes even if stdout is not attached to a terminal? the enum Style won't do this. Do you want to read it from MavenCli ? Though, the property itself is fine for me.
          Hide
          stephenc Stephen Connolly added a comment -

          I'd like to propose that we include this issue (however we end up fixing it) into scope for 3.5.1

          Show
          stephenc Stephen Connolly added a comment - I'd like to propose that we include this issue (however we end up fixing it) into scope for 3.5.1
          Hide
          cyrille@cyrilleleclerc.com Cyrille Le Clerc added a comment -

          The Jenkins Pipeline Maven Plugin would be very interested in having the colored output enable when the batch mode is enabled.
          We run Maven builds in batch mode to be not interactive but we would like to benefit of the colorized output.

          Reference https://issues.jenkins-ci.org/browse/JENKINS-44543

          Show
          cyrille@cyrilleleclerc.com Cyrille Le Clerc added a comment - The Jenkins Pipeline Maven Plugin would be very interested in having the colored output enable when the batch mode is enabled. We run Maven builds in batch mode to be not interactive but we would like to benefit of the colorized output. Reference https://issues.jenkins-ci.org/browse/JENKINS-44543
          Hide
          jglick@netbeans.org Jesse Glick added a comment -

          I was surprised to find that TERM=dumb mvn willfail still prints ANSI sequences, contrary to the expectations of Unix users.

          Show
          jglick@netbeans.org Jesse Glick added a comment - I was surprised to find that TERM=dumb mvn willfail still prints ANSI sequences, contrary to the expectations of Unix users.
          Hide
          michael-o Michael Osipov added a comment -

          Jesse, I discussed this in detail in a PR on GitHUb that our support is mediorce. Cited several tree(1) code spots how we should make our stuff idiotproof. Yes, TERM=dumb is not checked but should be: https://github.com/apache/maven/pull/114

          Show
          michael-o Michael Osipov added a comment - Jesse, I discussed this in detail in a PR on GitHUb that our support is mediorce. Cited several tree(1) code spots how we should make our stuff idiotproof. Yes, TERM=dumb is not checked but should be: https://github.com/apache/maven/pull/114
          Hide
          starwarsfan Yves Schumann added a comment -

          Fixing this issue with the next Maven version would be really great. I'm having developers asking for colored output on the Jenkins builds nearly every day...

          Show
          starwarsfan Yves Schumann added a comment - Fixing this issue with the next Maven version would be really great. I'm having developers asking for colored output on the Jenkins builds nearly every day...
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mryan43 commented on the issue:

          https://github.com/apache/maven/pull/114

          I feel stuck until someone with more experience of the maven code base update the version of maven-shared-utils to 3.2.1-SNAPSHOT on the default branch, anyone up to the task ?

          Show
          githubbot ASF GitHub Bot added a comment - Github user mryan43 commented on the issue: https://github.com/apache/maven/pull/114 I feel stuck until someone with more experience of the maven code base update the version of maven-shared-utils to 3.2.1-SNAPSHOT on the default branch, anyone up to the task ?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user michael-o commented on the issue:

          https://github.com/apache/maven/pull/114

          I don't might to pick this up, but it won't happen before Sep for personal priorities.

          Show
          githubbot ASF GitHub Bot added a comment - Github user michael-o commented on the issue: https://github.com/apache/maven/pull/114 I don't might to pick this up, but it won't happen before Sep for personal priorities.
          Show
          githubbot ASF GitHub Bot added a comment - Github user rfscholte commented on the issue: https://github.com/apache/maven/pull/114 Here's my proposal: https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=fd57754218e749305be1dd745fda9407960cf985
          Hide
          rfscholte Robert Scholte added a comment -
          Show
          rfscholte Robert Scholte added a comment - Fixed in 785bad693c60ad60d7b307af8fab9e9234ff57bd
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build maven-3.x #1663 (See https://builds.apache.org/job/maven-3.x/1663/)
          MNG-6220 Add CLI options to control color output Introduce (rfscholte: http://git-wip-us.apache.org/repos/asf/?p=maven.git&a=commit&h=785bad693c60ad60d7b307af8fab9e9234ff57bd)

          • (edit) maven-embedder/pom.xml
          • (edit) maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java
          • (edit) maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build maven-3.x #1663 (See https://builds.apache.org/job/maven-3.x/1663/ ) MNG-6220 Add CLI options to control color output Introduce (rfscholte: http://git-wip-us.apache.org/repos/asf/?p=maven.git&a=commit&h=785bad693c60ad60d7b307af8fab9e9234ff57bd ) (edit) maven-embedder/pom.xml (edit) maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java (edit) maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user agudian commented on the issue:

          https://github.com/apache/maven/pull/114

          @rfscholte: looks good! 👍

          Show
          githubbot ASF GitHub Bot added a comment - Github user agudian commented on the issue: https://github.com/apache/maven/pull/114 @rfscholte: looks good! 👍
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mryan43 commented on the issue:

          https://github.com/apache/maven/pull/114

          Great thanks @rfscholte !

          Show
          githubbot ASF GitHub Bot added a comment - Github user mryan43 commented on the issue: https://github.com/apache/maven/pull/114 Great thanks @rfscholte !
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mryan43 closed the pull request at:

          https://github.com/apache/maven/pull/114

          Show
          githubbot ASF GitHub Bot added a comment - Github user mryan43 closed the pull request at: https://github.com/apache/maven/pull/114
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mryan43 closed the pull request at:

          https://github.com/apache/maven-shared/pull/22

          Show
          githubbot ASF GitHub Bot added a comment - Github user mryan43 closed the pull request at: https://github.com/apache/maven-shared/pull/22
          Hide
          tom@bibbu.net Tom Wieczorek added a comment -

          I'm somehow not able to see any change when using -Dstyle.color=XXX with Maven 3.5.2. It always uses the auto detection. Can somebody confirm that it's working? I tried never at the terminal and had colors. I tried always when piping, and got no colors. I also tried some bogus value to trigger the exception, but to no avail.

          I tried both MAVEN_OPTS and as an argument to mvn.

          Apache Maven 3.5.2 (138edd61fd100ec658bfa2d307c43b76940a5d7d; 2017-10-18T09:58:13+02:00)
          Maven home: /opt/apache-maven-3.5.2
          Java version: 1.8.0_151, vendor: Oracle Corporation
          Java home: /usr/lib/jvm/java-8-oracle/jre
          Default locale: de_DE, platform encoding: UTF-8
          OS name: "linux", version: "4.10.0-37-generic", arch: "amd64", family: "unix"
          
          Show
          tom@bibbu.net Tom Wieczorek added a comment - I'm somehow not able to see any change when using -Dstyle.color=XXX with Maven 3.5.2. It always uses the auto detection. Can somebody confirm that it's working? I tried never at the terminal and had colors. I tried always when piping, and got no colors. I also tried some bogus value to trigger the exception, but to no avail. I tried both MAVEN_OPTS and as an argument to mvn . Apache Maven 3.5.2 (138edd61fd100ec658bfa2d307c43b76940a5d7d; 2017-10-18T09:58:13+02:00) Maven home: /opt/apache-maven-3.5.2 Java version: 1.8.0_151, vendor: Oracle Corporation Java home: /usr/lib/jvm/java-8-oracle/jre Default locale: de_DE, platform encoding: UTF-8 OS name: "linux", version: "4.10.0-37-generic", arch: "amd64", family: "unix"
          Hide
          dcendents Daniel Beland added a comment -

          It seems like it was not added to the maven 3.5.2 release.
          Was the pull request merged?

          Looking at the pull request, the parameter is '--color=always|never|auto': https://github.com/apache/maven/pull/114/files

          But when I check the source code for CLIManager on tag maven-3.5.2 it is not there: https://github.com/apache/maven/blob/maven-3.5.2/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java

          I'm a bit confused...

          Show
          dcendents Daniel Beland added a comment - It seems like it was not added to the maven 3.5.2 release. Was the pull request merged? Looking at the pull request, the parameter is '--color=always|never|auto': https://github.com/apache/maven/pull/114/files But when I check the source code for CLIManager on tag maven-3.5.2 it is not there: https://github.com/apache/maven/blob/maven-3.5.2/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java I'm a bit confused...
          Hide
          tom@bibbu.net Tom Wieczorek added a comment -
          Show
          tom@bibbu.net Tom Wieczorek added a comment - Daniel Beland : The PR was closed in favor of 785bad693c60ad60d7b307af8fab9e9234ff57bd .
          Hide
          dcendents Daniel Beland added a comment -

          I'm sorry to say but it is not working and the test is wrong.

          The test goes (MavenCliTest.java):

                  MessageUtils.setColorEnabled( true );
                  request = new CliRequest( new String[] { "-Dstyle.color=never" }, null );
                  cli.cli( request );
                  cli.properties( request );
                  cli.logging( request );
                  assertFalse( MessageUtils.isColorEnabled() );
          

          While the real code goes (MavenCli.java):

          try
                  {
                      initialize( cliRequest );
                      cli( cliRequest );
                      logging( cliRequest );
                      version( cliRequest );
                      properties( cliRequest );
                      localContainer = container( cliRequest );
                      commands( cliRequest );
                      configure( cliRequest );
                      toolchains( cliRequest );
                      populateRequest( cliRequest );
                      encryption( cliRequest );
                      repository( cliRequest );
                      return execute( cliRequest );
                  }
          

          cli.logging is called before cli.properties while the test was calling them in a different order.
          When I debug maven the userProperties is always empty and then default value of auto is assumed for the color logging.

          Show
          dcendents Daniel Beland added a comment - I'm sorry to say but it is not working and the test is wrong. The test goes (MavenCliTest.java): MessageUtils.setColorEnabled( true ); request = new CliRequest( new String [] { "-Dstyle.color=never" }, null ); cli.cli( request ); cli.properties( request ); cli.logging( request ); assertFalse( MessageUtils.isColorEnabled() ); While the real code goes (MavenCli.java): try { initialize( cliRequest ); cli( cliRequest ); logging( cliRequest ); version( cliRequest ); properties( cliRequest ); localContainer = container( cliRequest ); commands( cliRequest ); configure( cliRequest ); toolchains( cliRequest ); populateRequest( cliRequest ); encryption( cliRequest ); repository( cliRequest ); return execute( cliRequest ); } cli.logging is called before cli.properties while the test was calling them in a different order. When I debug maven the userProperties is always empty and then default value of auto is assumed for the color logging.
          Hide
          jglick@netbeans.org Jesse Glick added a comment -

          The usual risks of relying on unit tests: you get what you pay for. :-/ Robert Scholte can this be reopened? (I have no permissions to do so.)

          Show
          jglick@netbeans.org Jesse Glick added a comment - The usual risks of relying on unit tests: you get what you pay for. :-/ Robert Scholte can this be reopened? (I have no permissions to do so.)
          Hide
          jglick@netbeans.org Jesse Glick added a comment -

          I was directed to MNG-6296 which tracks the problem.

          Show
          jglick@netbeans.org Jesse Glick added a comment - I was directed to MNG-6296 which tracks the problem.
          Hide
          tom@bibbu.net Tom Wieczorek added a comment -

          Jesse Glick Thanks for pointing out. Just saw "Resolution Fixed" but not the link to the follow-up.

          Sorry for the noise.

          Show
          tom@bibbu.net Tom Wieczorek added a comment - Jesse Glick Thanks for pointing out. Just saw "Resolution Fixed" but not the link to the follow-up. Sorry for the noise.

            People

            • Assignee:
              rfscholte Robert Scholte
              Reporter:
              mryan Manuel Ryan
            • Votes:
              11 Vote for this issue
              Watchers:
              20 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development