SOLR-18281 Make PackageManager testable by printing to console through ToolRuntime#4502
Conversation
…rough ToolRuntime Replace the last two PackageUtils.print() call sites in PackageManager with runtime.print()/printSuccess()/printError(), then delete the now-unused method and its @SuppressForbidden System.out usage. Also replace duplicate RESET constant in PackageUtils.format() with CLIUtils.RESET.
dsmiley
left a comment
dsmiley
left a comment
There was a problem hiding this comment.
I like the packagemanager changes. But I don't claim to be a good reviewer to CLI stuff so take my approval with a grain of salt.
|
|
||
| public static String YELLOW = "\u001B[33m"; | ||
|
|
||
| public static String RESET = "\u001B[0m"; |
There was a problem hiding this comment.
what are these codes? I have no clue here
There was a problem hiding this comment.
the implication of my question is we need a comment with a URL
There was a problem hiding this comment.
These are ANSI terminal codes, a standard way to provide colored terminal output. For some reason Package manager uses colored output 🙈
There was a problem hiding this comment.
When we did the last pass of the CLI, this jumped out at me as something to rethink in the future. Maybe as part of the picocli migration we drop this (or double down and embrace it everywhere!).
There was a problem hiding this comment.
Agree. I'm also sure there exists a java library for TUI (Terminal User Interface) hanlding, if we want to properlty handle color, user input etc.
But I think perhaps we instead should stick to plain ASCII output as you say... Let's remember to cycle back to this..
3 participants
https://issues.apache.org/jira/browse/SOLR-18281
printSuccess()andprintError()to ToolRuntime (which prints green/red ANSI text)printGreen()withruntime.printSuccess()andprintRed()withruntime.printError()This insulates the PackageManager from raw console output by using the
ToolRuntimeproperly.And this makes the new
PackateToolTest.testDeployValidationMessages()assert tool output as designed.