add no lib targets test #847
Conversation
suaviloquence
force-pushed
the
add-no-lib-targets-test
branch
from
3c297bf to
5df1844
Compare
|
|
|
Never mind, I read the diff wrong, need to figure out why 1.78 is giving different output. |
suaviloquence
force-pushed
the
add-no-lib-targets-test
branch
from
5df1844 to
0b7e2ee
Compare
|
(fixed in 7495147) |
| expression: "&output" | ||
| --- | ||
| exit_code: 1 | ||
| stderr: "error: no crates with library targets selected, nothing to semver-check\nnote: only library targets contain an API surface that can be checked for semver\nnote: skipped the following crates since they have no library target: bin_only\n" |
There was a problem hiding this comment.
This isn't the most readable output: the explicit \n are quite annoying, as are the extremely long line lengths.
Consider individually snapshotting the stderr and stdout into separate files that are stored verbatim, like rustc does with its ui tests? For example: /~https://github.com/rust-lang/rust/blob/fd8d6fbe505ecf913f5e2ca590c69a7da2789879/tests/ui/nll/issue-50716.stderr
There was a problem hiding this comment.
Ron output:
CommandOutput(
exit_code: Some(1),
stderr: "error: no crates with library targets selected, nothing to semver-check\nnote: only library targets contain an API surface that can be checked for semver\nnote: skipped the following crates since they have no library target: bin_only\n",
stdout: "",
)I think we can use the default insta::assert_snapshot! on the stdout and stderr, which will use their Display impls, which should print new lines.
There was a problem hiding this comment.
Consider doing a hybrid approach. Make the CommandOutput struct contain three fields: stdout, stderr, and "everything else" inside a single struct that can be ron-serialized and snapshotted that way.
Separately, consider looking at the "redactions" and "filters" features of insta to remove information that will be a source of non-determinism: https://insta.rs/docs/redactions/
There was a problem hiding this comment.
It looks like insta provides the insta-cmd crate which does exactly what we want. I refactored to use that.
There was a problem hiding this comment.
Wow, TIL! Nice find, that looks quite good ✨
suaviloquence
force-pushed
the
add-no-lib-targets-test
branch
from
0b7e2ee to
3ec951f
Compare
suaviloquence
force-pushed
the
add-no-lib-targets-test
branch
2 times, most recently
from
eefb456 to
028a923
Compare
suaviloquence
force-pushed
the
add-no-lib-targets-test
branch
from
028a923 to
81f76bb
Compare
src/snapshot_tests.rs
Outdated
| /// Running `cargo semver-checks --workspace` on a workspace that has library | ||
| /// targets should be an error. |
There was a problem hiding this comment.
typo, presumably?
| /// Running `cargo semver-checks --workspace` on a workspace that has library | |
| /// targets should be an error. | |
| /// Running `cargo semver-checks --workspace` on a workspace that | |
| /// doesn't have any library targets should be an error. |
There was a problem hiding this comment.
Is the addition of files under .../new/new/ intentional here?
Depends on #846. Adds a test when there are no library targets in a workspace, which is (currently) an error.