Ensure ANSI is disabled in TracerTest regardless of environment (issu…

[dwalluck]

…e #1103)

[codecov-commenter]

Codecov Report

Merging #1166 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1166   +/-   ##
=========================================
  Coverage     94.29%   94.29%           
  Complexity      455      455           
=========================================
  Files             2        2           
  Lines          6661     6661           
  Branches       1792     1792           
=========================================
  Hits           6281     6281           
  Misses          102      102           
  Partials        278      278           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccd023e...9958d90. Read the comment docs.

[dwalluck]

When exactly should I expect ANSI to be turned on automatically? This is happening in some of our own code, where one person is testing it assuming no ANSI codes, and then I test it and the test fails due to ANSI codes in the output. Unfortunately, I don't know of a public API to just strip the codes. I do as you do here, more or less, and force -Dpicocli.ansi=false before the test.

[remkop]

Merged, thanks for the PR!

ANSI will be switched on for Unix/Mac, with an interactive console, and for Windows when Jansi is on the classpath, with an interactive console.

Then there are some details, which are documented here: https://picocli.info/#_heuristics_for_enabling_ansi

For JUnit tests, I would expect ANSI to be off by default because there is no interactive console, but that is a bit fragile.
Better to ensure by setting a system property like you did here.

[dwalluck]

For JUnit tests, I would expect ANSI to be off by default because there is no interactive console, but that is a bit fragile.
Better to ensure by setting a system property like you did here.

@remkop Is this Windows-only or is it more of an issue of not being able to determine if the shell is interactive or not (on Windows)?

I believe the default for ANSI is set to auto and it can use various variables (including perhaps TERM) to decide whether to enable it.

As you cannot control what a developer has his environment variables set to vs. the test/CI environment, I think the only safe and stable option is to force it off. Is it possible to force it off by simply passing -Dpicocl.ansi=off to the build tool (gradlew/mvn). I haven't tried this.

[remkop]

Is this Windows-only or is it more of an issue of not being able to determine if the shell is interactive or not (on Windows)? I believe the default for ANSI is set to auto and it can use various variables (including perhaps TERM) to decide whether to enable it.

Correct, the default for ANSI is AUTO. Sorry I was unclear. When I said "I expect ANSI to be off..." I meant that I expect AUTO to be evaluated to disabled.

FYI, picocli checks if System.console() returns null to guess if there is an interactive console. If null, then AUTO evaluates to disabled (except on Windows when Jansi is installed).

As you cannot control what a developer has his environment variables set to vs. the test/CI environment, I think the only safe and stable option is to force it off.

I completely agree.

Is it possible to force it off by simply passing -Dpicocl.ansi=off to the build tool (gradlew/mvn). I haven't tried this.

That may be possible, but I would not recommend it, because then your tests may pass or fail depending on whether -Dpicocl.ansi=off is set outside the test. It is more reliable to set this system property inside the test, like you did with this PR.

[dwalluck]

FYI, picocli checks if System.console() returns null to guess if there is an interactive console. If null, then AUTO evaluates to disabled (except on Windows when Jansi is installed).

I see that calcTTY() is what is checking for this. It's a little hard to debug given how many boolean clauses it evaluates. I am pretty sure I have System.console() == null since another test was actually testing for this by chance.

I am still trying to find out why it's turned on and I'll let you know if I find anything. If I understand your explanation, it is supposed to be off when doing a build.

[remkop]

Yes, it is checking a lot...

When ANSI is AUTO, it could be enabled even when System.console() == null when any of the following conditions is true:

• System.getenv("CLICOLOR_FORCE") is set to any value, other than "0"
• on Windows, if prior code has invoked AnsiConsole.systemInstall()
• on Windows, on pseudo-TTY like Cygwin or MSYS (env("TERM") starts with xterm or env("OSTYPE") != null)

static boolean ansiPossible() {
    if (forceDisabled())                          { return false; } // System.getenv("NO_COLOR") != null
    if (forceEnabled())                           { return true; } // getenv("CLICOLOR_FORCE") != null && !"0".equals(getenv("CLICOLOR_FORCE")
    if (isWindows() && isJansiConsoleInstalled()) { return true; } // 
    if (hintDisabled())                           { return false; } // "0".equals(getenv("CLICOLOR")) || "OFF".equals(getenv("ConEmuANSI"))
    if (!isTTY() && !isPseudoTTY())               { return false; } // console != null ; env("TERM"), env("OSTYPE")
    return hintEnabled() || !isWindows() || isXterm() || hasOsType(); //env("ANSICON"), env("CLICOLOR"), env("ConEmuANSI")
}