FileCheck

Hello,

I am not sold on FileCheck’s new behaviour. For failing tests in verbose mode, it first dump the actual error messages, followed by the annotated input file to FileCheck. The result is I can’t immediately see error messages if the input is more than just a few lines long, so I have to scroll all the way up to see the errors, then down again, etc.

I do see some advantages of dumping the input to FileCheck, but an improvement for me would be:

  • to dump the input first, then followed by the error message, so that I can the errors first, and then decide to scroll up if I am interested to do so.
  • dump it to a separate file (controlled with an option).

I am interested in changing the behaviour, because I think I find setting environment varibale “FILECHECK_OPTS=”–dump-input never"" inconvenient.

My 2 pennies.
Sjoerd.

+1. The new behavior makes sense if you break 1 test with 100 lines of output. It stops making sense if you break 20 tests with 1000s of lines of output. Some more aggressive dumping by default on buildbots may make sense

-Matt

For anybody viewing these failures through some sort of CI system, showing the error first then the input file is more useful for the same reasons you mentioned. Personally, I rarely run filecheck by hand from the command prompt, so your change would make my life worse. Granted, I’m just one person.

The point I’m trying to make is that I don’t think it’s clear-cut which order is better, so maybe we shouldn’t change it. I think it might be fine to add an option to swap the order, but I’d be very sad if it started dumping to some random file by default.

Thanks,

Christopher Tetreault

For anybody viewing these failures through some sort of CI system, showing the error first then the input file is more useful for the same reasons you mentioned. Personally, I rarely run filecheck by hand from the command prompt, so your change would make my life worse. Granted, I'm just one person.

I think a lot of people run filecheck by hand, at least everyone that does not have a proper* CI system set up.

The point I'm trying to make is that I don't think it's clear-cut which order is better, so maybe we shouldn't change it. I think it might be fine to add an option to swap the order, but I'd be very sad if it started dumping to some random file by default.

The order can be chosen with a flag and environment variable. We could even check if the output is a file or not to select a sensible default.

That said, I think the default should make sense for people that just download LLVM(, modify something,) and run the tests. If you set up your CI

you can add proper configuration to change this.

Cheers,

 Johannes

I would guess that in a CI system the order doesn’t matter much because you look at a webpage? I looked at some build bots today/yesterday that now also show this, and yeah, it’s fine either way, I was guessing.

My primary use-case is usage in a terminal, and displaying the errors first followed by all input makes this pretty unusable.

Thanks for bringing this discussion to the list so we can see a broader set of opinions on what the default should be.

I like Johannes’s suggestion that defaults should consider the new LLVM user. For that case, I think verbosity should be low, and diagnostics should mention how to increase verbosity.

For CI, I agree verbosity should be high because it’s not easy for many LLVM contributors to try again with higher verbosity.

For my personal usage, I don’t care what the default behavior is as long as behavior is easy to customize. FILECHECK_OPTS lets me do that per test suite run, or in my ~/.profile, or in my CI configs, etc.

Joel

The thing I use normally only shows the first N lines by default (I don’t know off hand what N is). Honestly, I don’t feel very strongly about the specific order, but it’s not useful when somebody proposes something on the list, and nobody voices any dissent (choosing instead to silently oppose the change). My requests would be:

As for the ordering issue… from the command line, I tend to set verbosity high, and so I pipe to a pager and search for “error:”. I prefer errors before the input dump then. My point is that it should be configurable if we change the default ordering.

Joel

Hi Chris,

The thing I use normally only shows the first N lines by default (I don’t know off hand what N is). Honestly, I don’t feel very strongly about the specific order, but it’s not useful when somebody proposes something on the list, and nobody voices any dissent (choosing instead to silently oppose the change). My requests would be:

  1. The order should be customizable via command line.
  2. By default, it should not dump things to multiple locations. If I ask for verbose output, I want to get blasted with all the stuff.
  3. The most important thing for me personally is to see the input to filecheck (I realize that this is in conflict with my earlier point. It’s early and I hadn’t had my coffee :blush: ). When I get a failure I want to be able to reproduce it in an IDE to use a debugger. Any change should not make this use case harder.

Personally, I do not find the argument that the defaults should be setup to be best for newcomers to be very compelling; we are talking about changing the behavior of a non-default option after all.

What do you mean by a “non-default option”? The default of -dump-input=never was recently changed to -dump-input=fail.

Joel

Two questions:

  1. Has anyone thought about adding a new “make check” configuration / envvar that enables more verbosity, which CI would enable but normal humans haven’t? The CI configs could be set to dump the whole file.

  2. Instead of dumping the entire input by default, would it be reasonable to change the default “make check” to have FileCheck print the 10 lines before and after the mismatch? Most problems are close by the check failure. If you want to check extra fancy, dump the CHECK-LABEL region.

The combination of both seem like significant usability improvements for both CI and humans.

-Chris

In my experience, the entire CHECK-LABEL region is still way too much (e.g. MIR tests print a giant block of function information in the prolog). There needs to be a stricter line count clamping of some kind

-Matt

Hi Chris,

Two questions:

  1. Has anyone thought about adding a new “make check” configuration / envvar that enables more verbosity, which CI would enable but normal humans haven’t? The CI configs could be set to dump the whole file.

FILECHECK_OPTS is an env var that permits this now. For example, I often do:

FILECHECK_OPTS=‘-dump-input=fail -vv -color’ ninja check-all

Is that what you mean?

After the recent change, -dump-input=fail above is redundant, but maybe that default needs more discussion.

  1. Instead of dumping the entire input by default, would it be reasonable to change the default “make check” to have FileCheck print the 10 lines before and after the mismatch? Most problems are close by the check failure. If you want to check extra fancy, dump the CHECK-LABEL region.

The combination of both seem like significant usability improvements for both CI and humans.

Sounds reasonable to me.

Joel

Sure, what I’m actually advocating here is a pile of heuristics that work well for humans: e.g. dump the label region if it is 20 lines or less. If it is large, then look at where the last match and the fuzzy next match are, and include that, … etc. FileCheck has a lot of information that we’re not using and some elbow grease could make the default experience way nicer for humans.

-Chris

Hi Chris,

Two questions:

  1. Has anyone thought about adding a new “make check” configuration / envvar that enables more verbosity, which CI would enable but normal humans haven’t? The CI configs could be set to dump the whole file.

FILECHECK_OPTS is an env var that permits this now. For example, I often do:

FILECHECK_OPTS=‘-dump-input=fail -vv -color’ ninja check-all

Is that what you mean?

After the recent change, -dump-input=fail above is redundant, but maybe that default needs more discussion.

Nice - this nicely separates out the CI problem from the human problem, thanks!

-Chris

  1. Instead of dumping the entire input by default, would it be reasonable to change the default “make check” to have FileCheck print the 10 lines before and after the mismatch? Most problems are close by the check failure. If you want to check extra fancy, dump the CHECK-LABEL region.

In my experience, the entire CHECK-LABEL region is still way too much (e.g. MIR tests print a giant block of function information in the prolog). There needs to be a stricter line count clamping of some kind

Sure, what I’m actually advocating here is a pile of heuristics that work well for humans: e.g. dump the label region if it is 20 lines or less. If it is large, then look at where the last match and the fuzzy next match are, and include that, … etc. FileCheck has a lot of information that we’re not using and some elbow grease could make the default experience way nicer for humans.

This kind of info is stored in the input-line annotations produced by -vv (when combined with -dump-input). I think it’s just a matter of filtering lines based on those annotations.

Joel

We’re talking about verbose output right? Verbose isn’t the default.

We’re talking about verbose output right? Verbose isn’t the default.

I’m fairly certain the issue in this thread is just the verbosity of -dump-input=fail. Yes, -vv makes it even more verbose by annotating input lines with good matches, etc., but that’s not part of the “new behaviour” Sjoerd meant, I believe.

Joel

Sjoerd specifically said “in verbose mode”, which I interpret to mean “when passing -v or -vv”. If we’re discussing the default behavior, then that’s a separate issue. Regardless, my other points stand independent of that.

I’m referring to the default behavior that I, as an LLVM developer, see when I type “ninja check” after setting up an LLVM the the usual recommended cmake flags.

-Chris

Sorry if I wasn’t clear about my use case. In my daily dev work, I do many local "ninja check"s, or “llvm-lit” on a subdirectory as a quick(er) smoke test if I am making changes in that area (e.g. “llvm-lit …/llvm/test/CodeGen”). Nothing wrong here, as indeed nothing changed here. But in case of a test failure, I want to run just that test:

bin/llvm-lit …/llvm/test/CodeGen/my_test.ll

This only reports success/failure, and doesn’t show any cause for failure , so I run it in verbose mode with:

bin/llvm-lit -a …/llvm/test/CodeGen/my_test.ll

In a terminal, the new default behaviour of FileCheck has become pretty unusable IMHO.