retis-org / retis

Tracing packets in the Linux networking stack & friends
https://retis.readthedocs.io/en/stable/
100 stars 14 forks source link

cli: return an errors if testing #260

Closed amorenoz closed 1 year ago

amorenoz commented 1 year ago

Fixes: #259

Instead of aborting the program, return the error so that tests can check against them.

atenart commented 1 year ago

I'm wondering why the initial issue was not seen on release v1.1. Do you know what introduced this behavior?

amorenoz commented 1 year ago

You mean that on v1.1 "--help" and "--version" were not raised as errors?

atenart commented 1 year ago

You mean that on v1.1 "--help" and "--version" were not raised as errors?

I'm not 100% sure of which versions are impacted, but testing a few builds on my machine,

v1.0:

$ ./target/release/retis --version
retis 1.0.0 ("pizza")

(IIRC the packaged v1.0 had not the issue).

v1.1:

$ ./target/release/retis --version
retis 1.1.0 ("pizza margherita")

(But the packaged v1.1 had the issue).

main w/ 3c651d75f971 reverted:

$ ./target/release/retis --version
Error: retis 1.1.0 ("pizza margherita")
amorenoz commented 1 year ago

It seems clap's behavior changed. The following code:

use clap::Command;

fn main()  {
    let arg_vec = vec!["my_prog", "--help"];
    let matches = Command::new("myprog")
        .ignore_errors(true)
        .try_get_matches_from(arg_vec);

    println!("Matches {:?}", matches);
}

Yields different results when built against clap 3.2.25 (used in retis 1.1.0) and 4.3.19 (used by retis's main).

3.2.25:

Matches Ok(ArgMatches { valid_args: [help], valid_subcommands: [], disable_asserts: false, args: {}, subcommand: None })

4.3.19:

Matches Err(ErrorInner { kind: DisplayHelp, context: FlatMap { keys: [], values: [] }, message: Some(Formatted(StyledStr("\u{1b}[1m\u{1b}[4mUsage:\u{1b}[0m \u{1b}[1mmy_prog\u{1b}[0m\n\n\u{1b}[1m\u{1b}[4mOptions:\u{1b}[0m\n  \u{1b}[1m-h\u{1b}[0m, \u{1b}[1m--help\u{1b}[0m  Print help\n"))), source: None, help_flag: Some("--help"), styles: Styles { header: Style { fg: None, bg: None, underline: None, effects: Effects(BOLD | UNDERLINE) }, error: Style { fg: Some(Ansi(Red)), bg: None, underline: None, effects: Effects(BOLD) }, usage: Style { fg: None, bg: None, underline: None, effects: Effects(BOLD | UNDERLINE) }, literal: Style { fg: None, bg: None, underline: None, effects: Effects(BOLD) }, placeholder: Style { fg: None, bg: None, underline: None, effects: Effects() }, valid: Style { fg: Some(Ansi(Green)), bg: None, underline: None, effects: Effects() }, invalid: Style { fg: Some(Ansi(Yellow)), bg: None, underline: None, effects: Effects() } }, color_when: Auto, color_help_when: Auto, backtrace: None })
amorenoz commented 1 year ago

I think the previous behavior is the buggy one so they must have fixed it.

amorenoz commented 1 year ago

Actually: https://github.com/clap-rs/clap/pull/4988

atenart commented 1 year ago

Thanks for the links. I'm still wondering why my local build of v1.1.0 did not had the issue while the build in copr had it, but I'm assuming the version of clap used might have been slightly different for some reason.