Support intercepting the help message generated by a multiple-subcommand application

[jbduncan]

Currently, if one has a command-line application with multiple subcommands, and one wants to write a high-level help message to the console, they would write something like this:

String... args = ...;
CommandLine cmd = ...;
cmd.parseWithHandler(new RunLast(), System.out, args);

or if things are a bit more complicated (as is the case for me):

String... args = ...;
CommandLine cmd = ...;
try {
  List<CommandLine> parsedArgs = cmd.parse(args);
  new RunLast().handleParseResult(parsedArgs, System.out, Ansi.AUTO);
  System.exit(0);
} catch (ParameterException exception) {
  new DefaultExceptionHandler()
      .handleException(exception, System.err, Ansi.AUTO, args);
  System.exit(-1);
}

But sadly, this is not quite enough for me, because I need some way of "intercepting" the text sent to System.{out,err} so that I can assert on the contents of auto-generated help message at unit-test time.

The closest solution I've thought of so far is to use PrintWriters in place of System.{out,err} so that, at unit-test time, I can swap out System.{out,err} with a custom PrintWriter that diverts the text it receives to a StringWriter, so that I can query the text directly. But sadly, this doesn't work because RunLast::handleParseResult doesn't have an overload that accepts a PrintWriter instead of a PrintStream.

Does picocli have something built-in that would allow me to intercept the auto-generated help message of a command-line application with multiple subcommands?

If not, would providing an overload of IParseResultHandler::handleParseResult that accepts a PrintWriter be an idea that the picocli team would be interested in implementing?

[remkop]

About the snippet you provide with the try-catch, if you’re looking to control the exit code, one idea is to call the parseWithHandlers method with specialized handlers that extend RunLast and DefaultExceptionHandler to call System.exit after calling super.handle.... That would emphasize that providing exit codes is the intention. But that’s just an idea.

About testing, your approach of passing in a custom printStream is exactly how many of the tests in CommandLineHelpTest work. See for example testWithoutShowDefaultValues and following tests.

You may want to create a convenience method like below in the test:

private static String usageString(CommandLine commandLine, Help.Ansi ansi) throws UnsupportedEncodingException {
    ByteArrayOutputStream baos = new ByteArrayOutputStream();
    commandLine.usage(new PrintStream(baos, true, "UTF8"), ansi);
    String result = baos.toString("UTF8");
    return result;
}

Note that the output will contain line separators. To make your tests run in any environment, it may be useful to use%n for line separators and call String.format on the expected string before doing the comparison.

Would this work for you?

[remkop]

Actually, you've inspired me! Exiting with a given error code is probably a common use case. I'm thinking to provide two more convenience methods that decorate a IParseResultHandler or a IExceptionHandler.

public static IParseResultHandler exit(final int exitCode, final IParseResultHandler inner) {
    return new IParseResultHandler() {
        public List<Object> handleParseResult(List<CommandLine> parsedCommands, 
                                              PrintStream out, 
                                              Help.Ansi ansi) {
            inner.handleParseResult(parsedCommands, out, ansi);
            System.exit(exitCode);
            return null;
        }
    }
}
public static IExceptionHandler exit(int exitCode, IExceptionHandler inner) {
    return new IExceptionHandler() {
        public List<Object> handleException(ParameterException ex, 
                                            PrintStream out, 
                                            Help.Ansi ansi, 
                                            String... args) {
            inner.handleException(ex, out, ansi, ex);
            System.exit(exitCode);
            return null;
        }
    }
}

That way, client code that want to exit with an error code, like your application, could look like this:

cmd.parseWithHandler(exit(0, new RunLast()),
                     System.out, 
                     Help.Ansi.AUTO, 
                     exit(-1, new DefaultExceptionHandler()),
                     args);

[jbduncan]

About testing, your approach of passing in a custom printStream is exactly how many of the tests in CommandLineHelpTest work. See for example testWithoutShowDefaultValues and following tests.

Oh cool, thank you very much for pointing me in the direction of those tests!

It's a shame that I have to use new PrintStream([...]) directly and deal with that checked UnsupportedEncodingException. I'll probably just wrap the exception in an AssertionError, as it would certainly be a programming error on my side if I passed the PrintStream an invalid encoding - I think that's probably the closest I can get to a clean solution - but many thanks!

I'll test it and let you know if it works as a solution for me when I get into work next week. :)

Note that the output will contain line separators. To make your tests run in any environment, it may be useful to use%n for line separators and call String.format on the expected string before doing the comparison.

IIRC, I'm already asserting in my tests that the line separators in my program's help message are equal to System.lineSeparator(), so hopefully my tests are cross-platform already. But thank you very much for the advice anyway. :)

[jbduncan]

I think providing exit(int, [...]) convenience methods would be a great idea! But they wouldn't completely meet my needs, because I'd ideally like my program to use System.out for the help message but to use System.err when the DefaultExceptionHandler needs to be invoked.

What do you think of providing a cmd.parseWithHandler([...]) overload like this?

cmd.parseWithHandler(exit(0, new RunLast()),
                     /* out= */ System.out,
                     /* err= */ System.err,
                     Help.Ansi.AUTO, 
                     exit(-1, new DefaultExceptionHandler()),
                     args);

[jbduncan]

Talking about the PrintStream and usageString() issue again...

I don't know if you've thought about this already, but I wonder if it would be good to provide a CommandLine::usage overload that accepts a PrintWriter instead of PrintStream, so that the whole charset-encoding issue can be dealt with behind-the-scenes instead of needing to provide a "UTF8" encoding and having to deal with the checked UnsupportedEncodingException. With such an overload, I think usageString() could look like this:

private static String usageString(CommandLine commandLine, Help.Ansi ansi) {
    StringWriter out = new StringWriter();
    commandLine.usage(new PrintWriter(out, true), ansi);
    String result = out.toString();
    return result;
}

[remkop]

I just thought of a way to address your requirements while also reducing the number of arguments passed to the parseWithHandler(s) methods (these methods already have too many arguments...)

• add constructors to the default handler implementations that take a target PrintStream or PrintWriter (also take an optional Help.Ansi parameter)
• add a method andExit(int) to the default handlers that returns a wrapper that invokes the handler first and then calls System.exit with the specified error code. Add this method to the default handlers instead of adding a static method on CommandLine.

User code, like your application, would look like this:

// production code
cmd.parseWithHandlers(new RunLast(System.out).andExit(0),
                      new DefaultExceptionHandler(System.err).andExit(-1),
                      args);

Test code could look like this:

// test code
StringWriter printWriter = new StringWriter();
cmd.parseWithHandlers(new RunLast(printWriter),
                      new DefaultExceptionHandler(printWriter),
                      args);
assertEquals(expected, printWriter.toString());

This looks a lot cleaner than the current API and I wish I had thought of it earlier...

This also makes it possible to have much simpler IParseResultHandler and IExceptionHandler interfaces:

// remove arguments: PrintStream and Help.Ansi 

// IParseResultHandler2
List<Object> handleParseResult(List<CommandLine> parsedCommands) throws ExecutionException;

// IExceptionHandler2
List<Object> handleException(ParameterException ex, String... args);

Unfortunately I can't think of another way to avoid breaking backwards compatibility than adding additional interfaces with different names. Darn, I really wish we could have had this conversation a week ago! :-)

[jbduncan]

Yes, it's really a shame that I only discovered this need a few days ago and (re-)discovered picocli a few days prior! :P

I've managed to resolve my testing needs not with a usageString([...]) method but instead by supplying ByteArrayOutputStreams to my program's entry point (defined by a static method called CommandLineEntryPoint.parseArgsAndExecute), like in the following JUnit 5 test snippet.

@Test
void theTest {
    ByteArrayOutputStream out = new ByteArrayOutputStream();
    ByteArrayOutputStream err = new ByteArrayOutputStream();
    ExitCode exitCode =
        CommandLineEntryPoint.parseArgsAndExecute(
            new PrintStream(out, true, "UTF8"), new PrintStream(err, true, "UTF8"));

    assertAll(
        () -> assertThat(out.toString()).contains(HIGH_LEVEL_USAGE_HELP),
        () -> assertThat(err.toString()).isEmpty(),
        () -> assertThat(exitCode.numericalExitCode()).isEqualTo(0));
}

[jbduncan]

So it's no longer urgent for me to have an overload of IParseResultHandler.handleParseResult that accepts a PrintWriter, but it would still be very nice to have. :)

[jbduncan]

If anything, I think it would be even nicer, and probably more important for the maintainability of my program at this point, to have implementation(s) of IParseResultHandler2 and IExceptionHandler2 as you've discussed already. :)

[jbduncan]

Thank you very much for all your help so far @remkop!

[remkop]

I finally got around to working on this.

Picocli 3.0 introduces these new classes and interfaces:

• IParseResultHandler2
• IExceptionHandler2
• AbstractHandler
• ParseResult

There is a new simplified parseWithHandlers(IParseResultHandler2, IExceptionHandler2, String...) method.

Handlers that extend AbstractHandler can optionally control the output stream, error stream and exit code. For example:

class MyCommand implements Runnable {
    public void run() { ... }

    public void static main(String... args) {
        CommandLine cmd = new CommandLine(new MyCommand());
        cmd.parseWithHandlers(
                new RunLast().useOut(System.out).useAnsi(Help.Ansi.AUTO).andExit(123),
                new DefaultExceptionHandler().useErr(System.err).andExit(456), 
                args);
    }
}

This is now in master.

There is no convenience methods to specify a PrintWriter. I just discovered http://stefanbirkner.github.io/system-rules/#SystemErrAndOutRule and I'm hoping this would be a good way for CLI authors to test the stdout/stderr streams and exit codes.

Thoughts?

[jbduncan]

Thanks for taking the time to look into this @remkop. The new API looks pretty neat! I'll see if I can find the time in the near future to play around with it and let you know what I think.

Regarding stdout/stderr streams and exit code testing, I'm sure that http://stefanbirkner.github.io/system-rules/#SystemErrAndOutRule would be a very good way of doing it for those people using JUnit 4, but I'm personally using JUnit 5, which doesn't provide @Rules anymore but a different extension mechanism, so in my case at least, a different solution would be needed. :/

[remkop]

Thanks for the response. From your test code snippet I can guess how you test the stderr and stdout streams, but can I ask how you test the exit code in JUnit 5? I'm thinking to do a write-up of how to test picocli command line applications.

[jbduncan]

My solution to testing exit codes is admittedly not great.

What I do is I organise the entry point of my application like this:

public enum ExitCode {
  SUCCESS(0),
  ERROR(1);

  private final int numericalExitCode;

  ExitCode(int numericalExitCode) {
    this.numericalExitCode = numericalExitCode;
  }

  public int numericalExitCode() {
    return numericalExitCode;
  }
}

public final class Application {
  private Application() {}

  public static void main(String[] args) {
    ExitCode exitCode = execute(System.out, System.err, args);
    System.exit(exitCode.numericalExitCode());
  }

  @VisibleForTesting
  static ExitCode execute(PrintStream out, PrintStream err, String... args) {
    // Do useful stuff here!
  }
}

and then I write tests which test against Application.execute() instead of Application.main(), like so:

  @Test
  void main_noArgs_printsHelp() throws UnsupportedEncodingException {
    // given
    ByteArrayOutputStream out = new ByteArrayOutputStream();
    ByteArrayOutputStream err = new ByteArrayOutputStream();

    // when
    ExitCode exitCode =
        Application.execute(new PrintStream(out, true, "UTF8"), new PrintStream(err, true, "UTF8"));

    // then
    assertAll(
        () -> assertThat(out.toString()).isEmpty(),
        () -> assertContainsImportantHelpKeywords(err.toString()),
        () -> assertThat(exitCode.numericalExitCode()).isEqualTo(0));
  }

FYI, my tests are written using JUnit 5's Jupiter API assertions and Truth, and @VisibleForTesting comes from Guava.

(I used an enum for the ExitCodes since I believe that it makes things more readable, but I may very well be over-engineering things.)

Since I use JUnit Jupiter (part of JUnit 5) and thus I don't have access to https://stefanbirkner.github.io/system-rules/, I believe that I'm forced to write my exit code tests this way. I certainly don't see a way that I could migrate to the cool new API you've written for setting exit codes without downgrading from JUnit Jupiter to JUnit 4 just so I could use https://stefanbirkner.github.io/system-rules/. It's my hope that if or when https://stefanbirkner.github.io/system-rules/ is ported to JUnit Jupiter (stefanbirkner/system-rules#55), then I could look into migrating to the new exit-code-setting API.

[remkop]

Closing this ticket, since #307 (finally) added the requested CommandLine.usage(PrintWriter) methods. :-)

Please feel free to reopen or raise another one for any follow-up issues.

[jbduncan]

Great! Thank you very much @remkop. :)

I hope that my response above for exit code testing was helpful in some way? I've not yet observed any info in the User Guide that shows how one can test exit codes, but I observe from picocli's tests that the way you're doing it right now for picocli itself is with https://stefanbirkner.github.io/system-rules/.

[remkop]

Yes it certainly was helpful!
The user manual is already a bit biggish, so
How to test picocli command line applications will likely be a separate article.
Thanks again for the great feedback!