Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No output when upgrading nextest to 0.9.86 #280

Closed
fredrik-jansson-se opened this issue Dec 19, 2024 · 15 comments · Fixed by #285
Closed

No output when upgrading nextest to 0.9.86 #280

fredrik-jansson-se opened this issue Dec 19, 2024 · 15 comments · Fixed by #285

Comments

@fredrik-jansson-se
Copy link

I'm running bacon 3.6.0 with nextest 0.9.86 (046e241b1 2024-12-12).

Since bacon 3.6.0 I no longer see any error output from failing tests:

 1  failed: parser::test::var
 no output

Using cargo test (from bacon) I get

 1  failed: parser::test::var
 thread 'parser::test::var' panicked at src/parser.rs:36:79:
 called `Result::unwrap()` on an `Err` value: Error { variant: ParsingError { positives: [identifier], negativ
 es: [] }, location: Pos(3), line_col: Pos((1, 4)), path: None, line: "var a = 2;", continued_line: None, pars
 e_attempts: None }

Running nextest outside bacon:

──── STDERR:             llvm::bin/llvm parser::test::var
thread 'parser::test::var' panicked at src/parser.rs:36:79:
called `Result::unwrap()` on an `Err` value: Error { variant: ParsingError { positives: [identifier], negatives: [] }, location: Pos(3), line_col: Pos((1, 4)), path: None, line: "var a = 2;", continued_line: None, parse_attempts: None }

Am I missing something or is there a setting I can't find to enable output from nextest in bacon?

@Canop
Copy link
Owner

Canop commented Dec 19, 2024

You mean the same configuration which was working before 3.6 isn't working with 3.6 ?

Standard configuration is

[jobs.nextest]
command = [
    "cargo", "nextest", "run",
    "--hide-progress-bar", "--failure-output", "final"
]
need_stdout = true
analyzer = "nextest"

@fredrik-jansson-se
Copy link
Author

Well, I don't have any custom bacon config.

@Canop

This comment was marked as outdated.

@fredrik-jansson-se

This comment was marked as outdated.

@Canop
Copy link
Owner

Canop commented Dec 19, 2024

Sorry for the log, it's not needed.

I updated cargo nextest to the last version and I can reproduce your problem. They changed the output :\

@fredrik-jansson-se
Copy link
Author

Got it, thanks for all help (again)!!

@Canop
Copy link
Owner

Canop commented Dec 19, 2024

If their format isn't stabilized, the nextest analyzer should probably be replaced to use their JSON output.

If anybody's willing to dive into this, they're welcome.

@fredrik-jansson-se
Copy link
Author

I can't make any promises, but sounds like a good challenge for the upcoming Holidays for me :)

@Canop
Copy link
Owner

Canop commented Dec 19, 2024

If you try this, and you have questions, you're welcome to come and chat on Miaou.

Two approaches seem possible: either make a new version of the existing nextest analyzer, or make a new nextest_json analyser or libtest_analyzer, similar to the cargo_json analyzer, to be used with cargo nextest run --message-format libtest-json (this would imply there's enough data in that JSON output, I didn't check).

@Canop Canop changed the title No output with nextest No output when upgrading nextest to 0.9.86 Dec 19, 2024
@sunshowers
Copy link

sunshowers commented Dec 21, 2024

Hi there, creator of nextest here.

I updated cargo nextest to the last version and I can reproduce your problem. They changed the output :\

Sorry about that! The human-readable output format, or in general anything nextest prints out to stderr, is explicitly not part of the stable guarantees -- I don't want to make too many tweaks to it, but there will likely be a few here and there.

The libtest-json format is functional but not quite full-fidelity, because cargo test doesn't support many of the features that nextest does. I would love to support a more full-featured machine-readable format, but that will require a fair bit of design and implementation work (see nextest issue #20). The work isn't on my employer's radar so I don't expect to get to it, but if you'd like to help out (even by just commenting there about what your needs are) it would be really helpful!

@Canop
Copy link
Owner

Canop commented Dec 21, 2024

Hello @sunshowers, thanks for coming here!

I suppose we'll try to see what we manage to obtain with the libtest-json format, or maybe the libtest-json-plus one.

Regarding the human-readable format, bacon doesn't need a lot of stability, just the ability to recognize a few line types which should be easy as we can rely on styling which is unlikely to appear otherwise. We'd just need, for each specific kind of line (location, start of stderr, etc.) a frozen ansi escape sequence and a basic shape.

If I'm unclear, an input line for bacon is an array of (csi, raw) strings, and recognizing a line is usually something like this:

const CSI_LINE_COL: &str = "\u{1b}[2m";
const CSI_ERROR: &str = "\u{1b}[31m";

/// Return true when the line is like
///    "67:52  error  Unnecessary escape character: \/  no-useless-escape"
fn is_line_col_error_line(content: &TLine) -> bool {
    let mut strings = content.strings.iter();
    let (Some(first), Some(second), Some(third), Some(fourth)) = (
        strings.next(),
        strings.next(),
        strings.next(),
        strings.next(),
    ) else {
        return false;
    };
    first.is_blank()
        && second.csi == CSI_LINE_COL
        && regex_is_match!(r"^\d+:\d+$", &second.raw)
        && third.is_blank()
        && fourth.csi == CSI_ERROR
        && fourth.raw == "error"
}

@sunshowers
Copy link

sunshowers commented Dec 24, 2024

Regarding the human-readable format, bacon doesn't need a lot of stability, just the ability to recognize a few line types which should be easy as we can rely on styling which is unlikely to appear otherwise. We'd just need, for each specific kind of line (location, start of stderr, etc.) a frozen ansi escape sequence and a basic shape.

Ah gotcha. So I guess the change that broke you was the one where we started producing Unicode characters.

There are a couple more changes I'd like to make to the output soon: another minor tweak to the Unicode, and indenting standard output/standard error to make sure it stands out.

Is there some way nextest can produce some kind of custom CSI escape that would help? I'm fine with that as a stopgap.

@Canop
Copy link
Owner

Canop commented Dec 24, 2024

@fredrik-jansson-se While there's no complete solution based on nextest's JSON output, I modified in PR #285 the previous analyzer

@sunshowers Please have a look at PR #285. I could modify the analyzer to support both versions. I think you might be able to define sequences of (csi, patterns) that have a low probability of changing (and if they change and there's an issue in bacon before the new version of nextest is released, this can be managed).

@fredrik-jansson-se
Copy link
Author

@Canop, sounds good! I didn’t get unstuck, so if you have a solution, please go with that. I’m afk for a couple of days for Xmas.

Happy holidays!

@Canop
Copy link
Owner

Canop commented Dec 24, 2024

I'll merge my PR, even if it's far from perfect, in order to relieve the problem for nextest users.

@Canop Canop closed this as completed in 2aefff4 Dec 24, 2024
bryceberger added a commit to jj-vcs/jj that referenced this issue Jan 24, 2025
bacon 3.6.0 -> 3.8.0, shows output with nextest 0.9.86
Canop/bacon#280
bryceberger added a commit to jj-vcs/jj that referenced this issue Jan 24, 2025
bacon 3.6.0 -> 3.8.0, shows output with nextest 0.9.86
Canop/bacon#280

Add python3 to the devshell to be able to run tools installed with uv
github-merge-queue bot pushed a commit to jj-vcs/jj that referenced this issue Jan 24, 2025
bacon 3.6.0 -> 3.8.0, shows output with nextest 0.9.86
Canop/bacon#280

Add python3 to the devshell to be able to run tools installed with uv
github-merge-queue bot pushed a commit to jj-vcs/jj that referenced this issue Jan 24, 2025
bacon 3.6.0 -> 3.8.0, shows output with nextest 0.9.86
Canop/bacon#280

Add python3 to the devshell to be able to run tools installed with uv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants