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

bump ui test crate #3008

Merged
merged 7 commits into from
Sep 25, 2023
Merged

bump ui test crate #3008

merged 7 commits into from
Sep 25, 2023

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Aug 3, 2023

The recommended way to run tests locally is ./miri bless -- -- --quiet, which will show

  • progress bars
  • the currently running tests (allowing you to see which ones are still running towards the end of the test suite)
  • the output of the currently running tests (if they are slow). This means slow running tests can output lines to stderr and the last line will be shown after the test name and updated every few hundred milliseconds.

As a side effect this PR also fixes #2998 and only builds dependencies if any tests actually need them (this means that with the next ui_test update we'll be able to merge all our test suites).

Also fixes #3052.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

huh how did I manage to break mir-opt-level 4 tests?

@RalfJung
Copy link
Member

RalfJung commented Aug 3, 2023

./miri bless -- -- --quiet

That's a lot of dashes... why?^^ (Also it's ./miri test --bless now.)

(this means that with the next ui_test update we'll be able to merge all our test suites).

One reason we did the split was so that the pass tests can run even when cargo-miri is borked, so we might want to keep the split.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

One reason we did the split was so that the pass tests can run even when cargo-miri is borked, so we might want to keep the split.

We can keep the directory split, but we can run all tests in one big runner (without accidentally allowing dep tests in the non-dep folder). Maybe I should have said "merge the test runners" not "merge the test suites"

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

You can then still filter by directory, and the dependencies won't get built at all if the dep folder is not part of the filter

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

That's a lot of dashes... why?

idk yet 😆 something with the test runner

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

huh how did I manage to break mir-opt-level 4 tests?

uh... those are broken for me locally on master, too

@RalfJung
Copy link
Member

RalfJung commented Aug 3, 2023

One reason we did the split was so that the pass tests can run even when cargo-miri is borked, so we might want to keep the split.

We can keep the directory split, but we can run all tests in one big runner (without accidentally allowing dep tests in the non-dep folder). Maybe I should have said "merge the test runners" not "merge the test suites"

Ah, fancy. I guess I'll see how that works. :)

@RalfJung
Copy link
Member

RalfJung commented Aug 3, 2023

idk yet laughing something with the test runner

One set of -- I get, it's distinguishing cargo test arguments from arguments passed to the runner.
But the 2nd -- should not be needed.

Maybe we should use an env var scheme so that we can have ./miri test --quiet --bless? Anyway I'll need to see locally what it looks like with and without quiet anyway, I don't get yet why --quiet should be recommended (and if it is recommended, why it shouldn't be the default).

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

why it shouldn't be the default).

because it means CI will not show anything (same reason we don't use --quiet in CI right now, you'd just get a bunch of dots)

@RalfJung
Copy link
Member

RalfJung commented Aug 3, 2023

Well CI can use a non-default config. But honestly I prefer the version without the dots. ;) So I'm fine with non-quiet being the default.

I gave it quick shot locally. Some first feedback:

  • Now building the test runner takes longer than building Miri. 😂 might be worth investigating whether all these dependencies carry their weight. E.g. do we truly need clap? It pulls in syn and that's really slow.
  • The live output, seeing which tests are still running, is really neat. :D
  • Its non-quiet version however can be improved: the last test that still runs is one of the earliest ones, so what happens is that output just stops and nothing moves any more -- because the "this is still running" indicator is scrolled out already, outside my viewport. The tests that are still running should always be at the bottom.
  • I got some errors in fail tests. And when I scroll back up, I just see the same stuff printed over and over again. Like, when I search for tests/fail/both_borrows/buggy_as_mut_slice.rs in my terminal output it has a dozen hits. The same does not happen for pass tests so it does look like something went wrong. I mean obviously something went wrong because I am getting errors:
Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
The application panicked (crashed).
Message:  could not execute "/home/r/src/rust/miri/target/debug/miri" "--error-format=json" "-Dwarnings" "-Dunused" "-Zui-testing" "--target" "x86_64-unknown-linux-gnu" "--extern" "getrandom=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-ea1755338c1459a8.rlib" "--extern" "getrandom=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-ea1755338c1459a8.rmeta" "--extern" "getrandom_1=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-964643f2b0add773.rlib" "--extern" "getrandom_1=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-964643f2b0add773.rmeta" "--extern" "libc=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/liblibc-f255c48072fae688.rlib" "--extern" "libc=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/liblibc-f255c48072fae688.rmeta" "--extern" "num_cpus=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libnum_cpus-5080ae5f42796c75.rlib" "--extern" "num_cpus=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libnum_cpus-5080ae5f42796c75.rmeta" "--extern" "rand=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/librand-fa9573179d9c7292.rlib" "--extern" "rand=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/librand-fa9573179d9c7292.rmeta" "--extern" "page_size=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libpage_size-c5bca0befe654b9c.rlib" "--extern" "page_size=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libpage_size-c5bca0befe654b9c.rmeta" "--extern" "tokio=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libtokio-c962404ad4b80287.rlib" "--extern" "tokio=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libtokio-c962404ad4b80287.rmeta" "--extern" "miri_test_deps=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/miri-test-deps" "-L" "/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps" "-L" "/home/r/src/rust/miri/target/ui/miri/debug/deps" "-L" "/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug" "--out-dir" "/home/r/src/rust/miri/target/ui" "tests/fail/validity/invalid_char.rs" "--edition" "2021": Too many open files (os error 24)
Location: /home/r/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ui_test-0.13.0/src/lib.rs:620

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
The application panicked (crashed).
Message:  could not execute "/home/r/src/rust/miri/target/debug/miri" "--error-format=json" "-Dwarnings" "-Dunused" "-Zui-testing" "--target" "x86_64-unknown-linux-gnu" "--extern" "getrandom=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-ea1755338c1459a8.rlib" "--extern" "getrandom=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-ea1755338c1459a8.rmeta" "--extern" "getrandom_1=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-964643f2b0add773.rlib" "--extern" "getrandom_1=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-964643f2b0add773.rmeta" "--extern" "libc=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/liblibc-f255c48072fae688.rlib" "--extern" "libc=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/liblibc-f255c48072fae688.rmeta" "--extern" "num_cpus=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libnum_cpus-5080ae5f42796c75.rlib" "--extern" "num_cpus=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libnum_cpus-5080ae5f42796c75.rmeta" "--extern" "rand=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/librand-fa9573179d9c7292.rlib" "--extern" "rand=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/librand-fa9573179d9c7292.rmeta" "--extern" "page_size=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libpage_size-c5bca0befe654b9c.rlib" "--extern" "page_size=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libpage_size-c5bca0befe654b9c.rmeta" "--extern" "tokio=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libtokio-c962404ad4b80287.rlib" "--extern" "tokio=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps/libtokio-c962404ad4b80287.rmeta" "--extern" "miri_test_deps=/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/miri-test-deps" "-L" "/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug/deps" "-L" "/home/r/src/rust/miri/target/ui/miri/debug/deps" "-L" "/home/r/src/rust/miri/target/ui/miri/x86_64-unknown-linux-gnu/debug" "--out-dir" "/home/r/src/rust/miri/target/ui" "tests/fail/unaligned_pointers/unaligned_ptr_zst.rs" "-Zmiri-disable-validation" "-Cdebug-assertions=no" "--edition" "2021": Too many open files (os error 24)
Location: /home/r/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ui_test-0.13.0/src/lib.rs:620

I don't know where one test ends and the next one begins. "Too many open files" sounds like maybe it went a bit overboard with the concurrency or so?

  • The beginning of the output now looks like:
     Running unittests src/lib.rs (target/debug/deps/miri-73bfc5312ddef86b)

running 21 tests
test borrow_tracker::tree_borrows::perms::propagation_optimization_checks::access_transitions_progress_increasing ... ok
test borrow_tracker::tree_borrows::unimap::tests::consistency_small ... ok
test borrow_tracker::tree_borrows::perms::propagation_optimization_checks::all_transitions_idempotent ... ok
test borrow_tracker::tree_borrows::unimap::tests::consistency_large ... ok
test borrow_tracker::tree_borrows::unimap::tests::extend_to_length ... ok
test concurrency::range_object_map::tests::boundaries ... ok
test concurrency::range_object_map::tests::empty_map ... ok
test concurrency::range_object_map::tests::perfectly_overlapping ... ok
test borrow_tracker::tree_borrows::perms::propagation_optimization_checks::foreign_read_is_noop_after_write ... ok
test concurrency::range_object_map::tests::straddling ... ok
test eval::tests::windows_argv0_no_escape ... ok
test borrow_tracker::tree_borrows::tree::commutation_tests::all_read_accesses_commute ... ok
test concurrency::vector_clock::tests::test_equal ... ok
test range_map::tests::out_of_range_iter - should panic ... ok
test concurrency::vector_clock::tests::test_partial_order ... ok
test concurrency::range_object_map::tests::no_overlapping_inserts - should panic ... ok
test eval::tests::windows_argv0_panic_on_quote - should panic ... ok
test intptrcast::tests::test_align_addr ... ok
test range_map::tests::out_of_range_iter_mut - should panic ... ok
test range_map::tests::gaps ... ok
test range_map::tests::basic_insert ... ok

test result: ok. 21 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/compiletest.rs (target/debug/deps/compiletest-cdf3132f0a720797)
## Running ui tests in tests/pass against miri for x86_64-unknown-linux-gnu
   Compiler: /home/r/src/rust/miri/target/debug/miri "--error-format=json" "-Dwarnings" "-Dunused" "-Zui-testing" "--target" "x86_64-unknown-linux-gnu" "--out-dir" OUT_DIR
  tests/pass/arrays.rs ... ok
  Building dependencies ... ok
  tests/pass/align.rs ... ok
  tests/pass/associated-const.rs (stack) ... ok
  tests/pass/align_repeat_into_well_aligned_array.rs ... ok
  tests/pass/assume_bug.rs (stack) ... ok
  tests/pass/align_repeat_into_packed_field.rs ... ok
  tests/pass/async-fn.rs ... ok
  tests/pass/align_strange_enum_discriminant_offset.rs ... ok
  tests/pass/available-parallelism-miri-num-cpus.rs ... ok
  tests/pass/atomic-compare-exchange-weak-never-fail.rs ... ok
[...]
  tests/pass/loop-break-value.rs ... ok
  tests/pass/loops.rs ... ok
test result: ok. 306 tests passed, 5 ignored, 0 filtered out

libtest prints the test <name> ... ok at the beginning of the line, ui-test adds some spaces, that looks inconsistent. Also the Compiler: lines is indented more than the others. And finally, the test result has an empty line above it with libtest. And if we are trying to make it super compatible anyway, N tests passed should become N passed, and there should be ; between the numbers instead of ,. (Skipping 0 failed sounds good though.)

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

"Too many open files" sounds like maybe it went a bit overboard with the concurrency or so?

uh. I have 96 cores and I don't get this issue... weird. Not sure how to investigate this :/

@RalfJung
Copy link
Member

RalfJung commented Aug 3, 2023

Also filtering doesn't seem to work any more: ./miri test fail runs all the tests. (That might be why mir-opt=4 fails, it relies on filtering.)

On my next attempt I am getting some even wilder errors:

  tests/fail/data_race/dealloc_write_race2.rs ... FAILED
  tests/fail/data_race/dealloc_write_race_stack.rs ... FAILEDThe application panicked (crashed).
Message:  `tests/fail/function_calls/arg_inplace_mutate.rs` not found
Location: /home/r/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ui_test-0.13.0/src/status_emitter.rs:103

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.

That file definitely exists.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

That file definitely exists.

that's an internal check, not about the file. Uh... what XD I want your desktop's thread switching capabilities, they seem to be great at fuzzing

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

Also filtering doesn't seem to work any more: ./miri test fail runs all the tests. (That might be why mir-opt=4 fails, it relies on filtering.)

oh yea, that's the -- issue lol. I'll drop clap in favor of manual parsing again. But note that we'll still have syn because we need serde for parsing the json output of tests

@RalfJung
Copy link
Member

RalfJung commented Aug 3, 2023

You might be running into clap-rs/clap#5055.

@saethlin
Copy link
Member

saethlin commented Aug 3, 2023

uh. I have 96 cores and I don't get this issue... weird. Not sure how to investigate this :/

I don't know what OS yall are on, but MacOS has a default open files limit that is shockingly low.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

I don't know what OS yall are on, but MacOS has a default open files limit that is shockingly low.

I really don't want to start figuring out the open file limit. Maybe a few quiet retries (bounded to a hard limit) with some sleeps will make it work out?

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

libtest prints the test <name> ... ok at the beginning of the line, ui-test adds some spaces, that looks inconsistent.

I replicated compiletest-rs behaviour. I'll switch to mirroring libtest

@saethlin
Copy link
Member

saethlin commented Aug 3, 2023

I really don't want to start figuring out the open file limit.

I can look into this later, or we could invite someone else to look into it.

@RalfJung
Copy link
Member

RalfJung commented Aug 3, 2023

I'm on Linux. I've never seen "too many open files" before so retry to me files like it would paper over whatever is going on...

Also, Linux error code are awfully unspecific. Can I get it to tell me which operation raised the error? It might also appear when spawning too many threads too quickly or whatever.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

it's probably during child process spawning and setting up pipes to it (from your panic line and message).

@saethlin
Copy link
Member

saethlin commented Aug 3, 2023

Can I get it to tell me which operation raised the error?

I would/will figure this out by using strace. Probably something like strace -fk -otrace -eopen,openat ./miri test then the trace will be in a file called trace, and I'd look for the syscall that returned the error code in question. -f attaches to children, -k prints a backtrace. There's a flag to print only failing syscalls, but eh.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

Everything but the spurious failures and

Now building the test runner takes longer than building Miri. 😂 might be worth investigating whether all these dependencies carry their weight. E.g. do we truly need clap? It pulls in syn and that's really slow.

has been fixed now. I got rid of clap, but ui_test still needs around 8-10s to build on my machine (vs 30 for miri).

@RalfJung
Copy link
Member

RalfJung commented Aug 3, 2023

Nice. :)

I am still seeing errors, though new ones:

The application panicked (crashed).
Message:  called `Result::unwrap()` on an `Err` value: "SendError(..)"
Location: /home/r/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ui_test-0.14.0/src/status_emitter.rs:262

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.

Also there are some glitches while tests are running. "at " or so? Text that very briefly appears in a few places. After Ctrl-C only tiny parts of it are left

tests/pass/backtrace/backtrace-global-alloc.rs ⠒              at 
tests/pass/backtrace/backtrace-std.rs ⠒              at 
tests/pass/concurrency/sync.rs (stack) ⠐ 
tests/pass/panic/nested_panic_caught.rs ⠤    9:           0x3f56e1 - 
tests/pass/shims/time-with-isolation2.rs ⠄ 
tests/pass/concurrency/sync.rs (tree) ⠙                                                                                                                                                                            

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 3, 2023

Also there are some glitches while tests are running. "at " or so? Text that very briefly appears in a few places. After Ctrl-C only tiny parts of it are left

there shouldn't be. These lines are from filtered backtraces from when miri interprets a panic. Any test that takes longer than 1s starts putting its last line from stderr after the test spinner

Ctrl+C may cause random stuff to be left over, yes. A full run should clean it up tho

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 7, 2023

Why are there so many threads anyway? Are you not using a thread pool?

Uh, this thread issue is in miri not in ui_test. ui_test has exactly n_cores + 3 threads, where the +3 is just some threads that are mostly asleep and don't do much.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 7, 2023

You said this happens even without a ulimit for threads?

No, I have never seen this before. Only with a ulimit for processes

@RalfJung
Copy link
Member

RalfJung commented Aug 7, 2023

Uh, this thread issue is in miri not in ui_test. ui_test has exactly n_cores + 3 threads, where the +3 is just some threads that are mostly asleep and don't do much.

Yeah I've seen that. The reports I linked are this happening in rustc.

No, I have never seen this before. Only with a ulimit for processes

Threads are basically processes that share the address space on Linux, so I wouldn't be surprised if the process limit also includes threads.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 7, 2023

Threads are basically processes that share the address space on Linux, so I wouldn't be surprised if the process limit also includes threads.

oh that makes sense. I guess it'll be fine then if ppl reduce the number of threads or increase their ulimit -u. Orthogonally we could start figuring out how to harden everything against such spurious failures. This will need some work in the ecosystem combined with fancy testing under very low ulimits.

@RalfJung
Copy link
Member

RalfJung commented Aug 7, 2023

If the default limits are fine then I wouldn't worry about it.

@saethlin
Copy link
Member

saethlin commented Aug 7, 2023

Yes, I would only worry about defaults on common dev systems. So like Ubuntu or whatever common Linux with available_parallelism() of 64 or 128 should work. Same with MacOS, which often has lower resource limits but similarly lower core counts.

Manually lowering the resource limits can break all kinds of software, including systemd. So let's handle users being creative if anyone reports a problem. Same deal with a 448-vCPU VM.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 8, 2023

Oh great I need a windows host to reproduce the i686-pc-windows-msvc failure.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 8, 2023

Found a windows host. CI passes, locally tests pass with ulimit -Su 1000 at 96 threads

Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks very nice here. :) Please squash all the "bump ui-test" commits though.

Comment on lines 124 to 127
std::env::var("MIRI_TEST_THREADS").map_or_else(
|_| std::thread::available_parallelism().unwrap(),
|threads| NonZeroUsize::new(threads.parse().unwrap()).unwrap(),
),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write this as a match, I find map_or_else much harder to read than a direct match

@bors
Copy link
Collaborator

bors commented Aug 15, 2023

☔ The latest upstream changes (presumably #2972) made this pull request unmergeable. Please resolve the merge conflicts.

@RalfJung RalfJung added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Aug 21, 2023
@oli-obk oli-obk force-pushed the ui_test_progress_bars branch 2 times, most recently from 684c291 to 80dd9a2 Compare September 22, 2023 15:46
@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 22, 2023

I squashed all the chained ui test bumps, but left the ones that had other changes in between. It also helps having individual commits to review the changes required by the bumps instead of skipping over too many versions at once

Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this locally for a bit and it looks great, thanks a ton for all the work you put into this. :) Just got some nits here.

tests/compiletest.rs Show resolved Hide resolved
tests/compiletest.rs Show resolved Hide resolved
@@ -1,5 +1,5 @@
$DIR/backtrace-api-v0.rs:24:14 (func_d)
$DIR/backtrace-api-v0.rs:20:5 (func_c)
$DIR/backtrace-api-v0.rs:9:5 (func_b)
$DIR/backtrace-api-v0.rs:9:5 (func_b::<u8>)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fascinating, why does this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have absolutely no clue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I think I changed some default path filters and they may affect this but I don't know why)

Copy link
Member

@RalfJung RalfJung Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like somehow this filter no longer applies?

//@normalize-stderr-test: "::<.*>" -> ""

Copy link
Member

@RalfJung RalfJung Sep 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah wait, that's a stderr filter. Was there a bug previously where those applied to stdout as well?

EDIT: Yes, there was. So this is a bugfix in ui_test. Nice. :) Does the ui-test test suite have a test to ensure normalization is for stdout/stderr only?

@RalfJung
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 25, 2023

📌 Commit 4a3b941 has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Sep 25, 2023

⌛ Testing commit 4a3b941 with merge c004fc1...

@bors
Copy link
Collaborator

bors commented Sep 25, 2023

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing c004fc1 to master...

@bors bors merged commit c004fc1 into rust-lang:master Sep 25, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: Waiting for the PR author to address review comments
Projects
None yet
4 participants