Don't panic while panicking in CLI's tests #641
Conversation
Diesel CLI's tests will abort with no output if the tests for `diesel database create` fail to drop the database. (This is actually failing for many people because the code to drop it has diverged from CLI's behavior, which will be fixed separately). This change ensures that none of the `Drop` impls in our test suite will abort the process.
|
Thanks @dnagir for making me aware of this issue. |
| Ok(x) => x, | ||
| Err(e) => { | ||
| use ::std::io::{Write, stderr}; | ||
| if ::std::thread::panicking() { |
There was a problem hiding this comment.
FTR, don't compile these tests with panic=abort.
There was a problem hiding this comment.
Since tests fail by panicking, that would be bad in any project (I'll bet cargo test sets panic=unwind even if the project is configured with panic=abort)
| } else { | ||
| Err(self) | ||
| } | ||
| } |
There was a problem hiding this comment.
I've seen a few lines like these in the last few days. Maybe it is time to add the glorious boolinator as a dependency?
| conn.silence_notices(|| { | ||
| conn.execute(&format!("DROP DATABASE IF EXISTS {}", database)).unwrap(); | ||
| try_drop!(conn.execute(&format!("DROP DATABASE IF EXISTS {}", database)), "Couldn't drop database"); | ||
| }); |
There was a problem hiding this comment.
Memo to self: Open clippy issue for denying unwrap (and other panic-able fns) in drop
|
Travis fails in |
|
Rerunning. Will make it deterministic in a separate PR. |
Diesel CLI's tests will abort with no output if the tests for
diesel database createfail to drop the database. (This is actually failingfor many people because the code to drop it has diverged from CLI's
behavior, which will be fixed separately).
This change ensures that none of the
Dropimpls in our test suite willabort the process.