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

RUST_BACKTRACE=1 should be on by default for cargo test #1634

Closed
jimmycuadra opened this issue May 19, 2015 · 23 comments
Closed

RUST_BACKTRACE=1 should be on by default for cargo test #1634

jimmycuadra opened this issue May 19, 2015 · 23 comments

Comments

@jimmycuadra
Copy link
Contributor

Having to set this somewhat arcane environment variable in order to get a backtrace on a failing test is not a good user experience. I'm not sure if there are any plans to implement this option as a flag to rustc itself instead of an environment variable, or if there are any plans to make it on by default, but in any case I think Cargo should take care of turning it on for the user by default, at the very least when running tests.

@alexcrichton
Copy link
Member

Thanks for the report! This is a dupe of #1517, however, and I've explained there why I don't think this should be implemented in Cargo. For example I do not want RUST_BACKTRACE turned on by default for testing as it is quite noisy and most of the time I don't actually need a backtrace.

@jimmycuadra
Copy link
Contributor Author

Is there precedent in other languages for tests to hide backtraces by default? This seems quite unusual to me. Maybe it'd feel less noisy if rust-lang/rust#25621 is implemented.

@jimmycuadra
Copy link
Contributor Author

Also, taking into account your desire not to have Cargo set default options for rustc's test harness, maybe this is really a rustc issue.

@alexcrichton
Copy link
Member

I could see this either way as a Rust or Cargo issue, but for example languages like C do not have stack traces by default (as it requires a good deal of machinery to get a stack trace). In Rust it also requires a good deal of machinery to get a stack trace and you're not guaranteed that it exists. For example languages like Ruby and Python have a VM where the stack is explicit and there's a data structure to keep track of the information, but compiled languages like Rust, C and C++ use the native stack which is far less portable to inspect.

@wycats
Copy link
Contributor

wycats commented May 21, 2015

@alexcrichton how do you feel about non---release cargo runs?

@alexcrichton
Copy link
Member

I'm a little worried about too many different knobs for defaults, and I also feel that each knob will grow a cli switch at some point, which seems like somewhat overkill for this.

@alexcrichton
Copy link
Member

I'm going to close this for the reasons I listed above.

@wycats
Copy link
Contributor

wycats commented Jun 17, 2015

@alexcrichton How do you feel about converting the magic environment variables people have to learn into global command-line flags?

@alexcrichton
Copy link
Member

I still feel that generating a backtrace or not is orthogonal enough to a package manager that Cargo shouldn't be dealing with it. Cargo should certainly enable it to be specified one way or another (e.g. let the environment variable be inherited), but adding flags and/or logic to Cargo for all the bells and whistles in the standard distribution seems like the wrong direction to go in for me.

@wycats
Copy link
Contributor

wycats commented Jun 17, 2015

@alexcrichton the problem is that most people experience running Rust programs through Cargo, and having to learn about RUST_BACKTRACE=1 feels odd, to say the least. It doesn't seem that concerning to me to have a flag to Cargo (at minimum to cargo run) to enable the backtrace in a way that someone can discover by running --help

@jimmycuadra
Copy link
Contributor Author

Having cargo test and cargo run at all seems like a bigger divergence from strict package management than adding a flag to an existing command to me. I see why both commands make sense for Cargo. It just seems like an odd place to draw the line in the sand.

@wycats
Copy link
Contributor

wycats commented Jun 17, 2015

@jimmycuadra fwiw, cargo run seems pretty reasonable for a package manager in a compiled language, while I can see what you're saying about cargo test.

For me, the question is simply whether someone would reasonably think of the change as a configuration of the command they typed. cargo run --backtrace makes sense, while RUST_BACKTRACE=1 cargo run feels like you're plunging down into a lower abstraction to do what seems like a normal kind of thing.

@alexcrichton
Copy link
Member

@alexcrichton the problem is that most people experience running Rust programs through Cargo, and having to learn about RUST_BACKTRACE=1 feels odd, to say the least.

I agree, but the whole idea of an environment variable controlling backtraces also seems a little odd to me as well, and I'm not sure that adding a flag to cargo run would help much. If the goal was just to add some text to cargo run --help then we could just add some text about RUST_BACKTRACE.

while RUST_BACKTRACE=1 cargo run feels like you're plunging down into a lower abstraction to do what seems like a normal kind of thing

This seems like a somewhat critical distinction to me, though, in that a backtrace for a Rust program is a lower abstraction and is not a normal kind of thing. The stack traces from Rust are very different than the stack traces from Ruby/Python, for example:

  • You very rarely have line numbers/filenames
  • The stack trace is not the actual stack trace due to inlining
  • Stack traces do not work on all platforms
  • Stack traces often have weird other symbols in them

Backtraces in Rust are just not a "super first class" utility which are expected to be 100% accurate, they're mostly just advisory. Along those lines I don't necessarily want all advisory debugging tools to be various Cargo flags.

@jaredly
Copy link

jaredly commented Aug 6, 2015

This is a huge frustration for new users :/
without them there is generally zero indication of where the thing failed...

I should amend that to say "if they're following the commonly done (though ill-advisable) practice of just unwrap()ing everything instead of putting expect()s in reasonable places."

@nikomatsakis
Copy link
Contributor

@alexcrichton

This seems like a somewhat critical distinction to me, though, in that a backtrace for a Rust program is a lower abstraction and is not a normal kind of thing.

Hmm, I don't think I agree with this. I think backtraces are pretty de rigeur for C/C++ programmers, and I've always experienced very reliable line numbers in that setting -- sure, when full optimization is enabled, things get a little bit iffy, but it's usually been enough for me to figure out what's going on.

Rust backtraces aren't quite as reliable as C++, obviously, but they've come a long way and are quite usable (at least on linux, I can't speak for other platforms). For example, I build rustc with -g and -O3 most of the time, and I find the backtraces quite complete. They have line numbers, they show which functions are inlined into which other functions, and in general they save me a lot of time.

I have found debugging unit tests in cargo to be quite annoying. I often get an error like "unwrap called on None" showing me some line in libstd, but I want to know which unwrap it was that failed! Now, I could plumb expect calls or something, but I'd rather the compiler just tell me. And since I expect to run tests in debug mode most of the time, the debuginfo ought to be pretty good.

I agree with you Alex that we can't add every option to cargo, but having backtraces readily accessible -- perfect or not -- feels pretty basic to me! If the backtraces aren't reliable enough, we should fix them.

@nikomatsakis
Copy link
Contributor

Now, I could plumb expect calls or something, but I'd rather the compiler just tell me.

Oh, and, if we have backtraces with line numbers, then I can just click on the relevant line in emacs and get taken there. Much nicer.

@jimmycuadra
Copy link
Contributor Author

I have found debugging unit tests in cargo to be quite annoying. I often get an error like "unwrap called on None" showing me some line in libstd, but I want to know which unwrap it was that failed! Now, I could plumb expect calls or something, but I'd rather the compiler just tell me. And since I expect to run tests in debug mode most of the time, the debuginfo ought to be pretty good.

This is the exact reason I even turn on backtraces most of the time. It's essentially useless information to tell me where in std the actual panic happened rather than where in my code the unwrap was called. Maybe having backtraces off by default wouldn't feel so wrong if the higher level error messages were better.

@alexcrichton
Copy link
Member

@nikomatsakis

I have found debugging unit tests in cargo to be quite annoying.

I totally agree that debugging without backtraces is quite annoying, but the issue here to me is just whether they're turned on by default or not. Right now a failing test with a backtrace certainly has some UI issues:

running 1 test
test foo ... FAILED

failures:

---- foo stdout ----
    thread 'foo' panicked at 'explicit panic', foo.rs:3

stack backtrace:
   1:        0x102e7cc95 - sys::backtrace::write::hf5ea20500b66cd24uns
   2:        0x102e802b3 - panicking::on_panic::hbe02cb0d925cad49iGw
   3:        0x102e75382 - rt::unwind::begin_unwind_inner::h12ba0ba9dffdecc2uow
   4:        0x102e2f58c - rt::unwind::begin_unwind::h1384026207784234921
   5:        0x102e2f4ee - foo::habf228f06f7efe55eaa
   6:        0x102e51d10 - boxed::F.FnBox<A>::call_box::h11070975173153378549
   7:        0x102e54c4f - boxed::F.FnBox<A>::call_box::h1372358435100094416
   8:        0x102e52517 - rt::unwind::try::try_fn::h5361545742513409503
   9:        0x102e81f98 - rust_try_inner
  10:        0x102e81f85 - rust_try
  11:        0x102e7bc05 - rt::unwind::try::inner_try::h480e3107f6a4b5b9nkw
  12:        0x102e52748 - boxed::F.FnBox<A>::call_box::h12826721353945353732
  13:        0x102e7eead - sys::thread::Thread::new::thread_start::hdb3d925f69c5da4aHIv
  14:     0x7fff94fd0059 - _pthread_body
  15:     0x7fff94fcffd6 - _pthread_start


failures:
    foo

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured

thread '<main>' panicked at 'Some tests failed', ../src/libtest/lib.rs:255
stack backtrace:
   1:        0x102e7cc95 - sys::backtrace::write::hf5ea20500b66cd24uns
   2:        0x102e80525 - panicking::on_panic::hbe02cb0d925cad49iGw
   3:        0x102e75382 - rt::unwind::begin_unwind_inner::h12ba0ba9dffdecc2uow
   4:        0x102e30ca6 - rt::unwind::begin_unwind::h17788042703409534955
   5:        0x102e32953 - test_main::he3317e9b11453a96I1a
   6:        0x102e39171 - test_main_static::h818c3572a7d0c571d4a
   7:        0x102e2f8ec - __test::main::h49e26b7832b9fc29Jaa
   8:        0x102e81f98 - rust_try_inner
   9:        0x102e81f85 - rust_try
  10:        0x102e80e83 - rt::lang_start::hd8f037e4034ec509fBw
  11:        0x102e2f94e - main

(e.g. the extra panic at the end). Beyond that I feel that Cargo silently turning them on for tests (and only tests) gives the wrong impression about backtraces in Rust code. Setting up `RUST_BACKTRACE=1 doesn't seem so bad and it's one of those things which you should probably learn early on in Rust (at least in my opinion)

@nikomatsakis
Copy link
Contributor

@alexcrichton

(e.g. the extra panic at the end)

Yes, that is silly.

Beyond that I feel that Cargo silently turning them on for tests (and only tests) gives the wrong impression about backtraces in Rust code. Setting up `RUST_BACKTRACE=1 doesn't seem so bad and it's one of those things which you should probably learn early on in Rust (at least in my opinion)

I'll be honest, I don't really know what impression you mean. I think you are referring to the idea that backtraces are special because they require infrastructure that is not always available; but I feel like this also describes memory allocation, stdout, dynamic linking, and a bunch of other stuff that we make available in libstd.

Now, I certainly agree that learning about RUST_BACKTRACE=1 is useful, and it'd be nice if it were more discoverable (I wonder if the printout should somehow say "Use RUST_BACKTRACE to attempt to dump a backtrace".)

I guess something like "cargo test --backtrace" could be a middle-ground, in that it's relatively discoverable, and the help for it can explain a bit more: "Dump a backtrace when a panic occurs. This is equivalent to setting RUST_BACKTRACE in the environment."

@nikomatsakis
Copy link
Contributor

That said, I think I would prefer cargo test --no-backtrace :)

@nikomatsakis
Copy link
Contributor

rlguarino pushed a commit to rlguarino/wert that referenced this issue Jun 4, 2021
Got stuck a bit on the inverted image coordinate system. U and V are
spacial, with positive Y up, while x and y are in the image coordinate
system, with positive Y down.

Background:
rust-lang/cargo#1634
@msdrigg
Copy link

msdrigg commented Aug 23, 2022

For anyone seeing this now and wondering if there is an easy work-around to set this by default, you can add a file in $WORKSPACE_ROOT/.cargo/config.toml with the contents

[env]
RUST_BACKTRACE = "1"

to automatically set this env variable in all cargo processes (including cargo test and cargo run).

@andrii-rubtsov
Copy link

Can also be set/overridden via aliases, for example, I put this in my $CARGO_HOME/config.toml:

[alias]
t = "test --config env.RUST_BACKTRACE=\"0\""
tt = "test --config env.RUST_BACKTRACE=\"1\""
ttt = "test --config env.RUST_BACKTRACE=\"full\""

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

No branches or pull requests

7 participants