-
Notifications
You must be signed in to change notification settings - Fork 65
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
Add simple integration test & infrastructure #24
Conversation
Turns this repository into a cargo workspaces and adds a sub-crate with the wonderful name 'simple-human-panic-test' that lives in the new `tests/` directory. This should be of no consequence to the actual crate itself or its deployment to crates.io, but allows us to easily test how human-panic works when in an external crate. Especially, we can set up the sub-crate `Cargo.toml` files in a way that we can easily test that human-panic's output does not cause panics in humans. For that, I've also added a simple integration test to the `simple` test crate. It uses assert_cli to run the crate's main binary and assert its output contains certain strings (as well as that it exits with a panic- typical exit code). By the way: One benefit of using cargo workspaces is that sub-crates share a lock file and a `target/` directory. So, we can add lots of test crates without worrying of having to recompile dependencies all the time. You can easily run the tests across all sub-crates with `cargo test --all`. In general, most cargo subcommands support `--all`, and I've added it in a few places in the CI config.
for great ~~justice~~ formatting
Cargo.toml
Outdated
|
||
[workspace] | ||
members = [ | ||
"tests/simple", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of "simple" having a clearer specifier might be useful. Having "simple" as a category isn't very clear on what should be inside of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's meant as a first step that tests simple, obvious things :D What would you call it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps naming it "tests/single_panic" or something might be clearer as to what it's testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thank you 👍
chore: Ensure pre-commit gets non-system Python
Turns this repository into a cargo workspaces and adds a sub-crate with the wonderful name 'simple-human-panic-test' that lives in the new
tests/
directory. This should be of no consequence to the actual crate itself or its deployment to crates.io, but allows us to easily test how human-panic works when in an external crate. Especially, we can set up the sub-crateCargo.toml
files in a way that we can easily test that human-panic's output does not cause panics in humans.For that, I've also added a simple integration test to the
simple
test crate. It uses assert_cli to run the crate's main binary and assert its output contains certain strings (as well as that it exits with a panic- typical exit code). By the way: One benefit of using cargo workspaces is that sub-crates share a lock file and atarget/
directory. So, we can add lots of test crates without worrying of having to recompile dependencies all the time.You can easily run the tests across all sub-crates with
cargo test --all
. In general, most cargo subcommands support--all
, and I've added it in a few places in the CI config.This is the first step towards #18 but I'd like to add more tests before closing this issue.