tests: don't have cargo install test tools #216
Conversation
What enables the |
|
@martinvonz Me manually passing I guess what we're doing is different. Your binaries are supporting the relevant tests, but my binaries are the tests. |
I see. I'd really like to avoid having to do that, especially since every developer would need to remember to do the same. Maybe there's just no good solution until rust-lang/cargo#3670 has been fixed. I guess the problem is effectively the same as with using e.g. the |
Sorry :( I don't know what the right solution to this problem is. I suppose we will also need a |
I noticed that `cargo install --path .` will install the new `fake-editor` and `fake-diff-editor` binaries, which we don't want. It doesn't seem to work to put them in a separate crate and have a dev-dependency on it because `cargo` doesn't like dev-dependencies to binary targets. However, it seems that we can put them in `examples/` to be able to access them without having `cargo install` install them.
| @@ -140,7 +140,7 @@ impl TestEnvironment { | |||
|
|
|||
| /// Sets up the fake editor to read an edit script from the returned path | |||
| pub fn set_up_fake_editor(&mut self) -> PathBuf { | |||
| let editor_path = assert_cmd::cargo::cargo_bin("fake-editor"); | |||
| let editor_path = assert_cmd::cargo::cargo_bin("examples/fake-editor"); | |||
There was a problem hiding this comment.
Interesting, this really works? How does Cargo know that it needs to build the example before running the test?
There was a problem hiding this comment.
Yep, it seems that it does (the checks pass). I think you can have regular tests in the files in examples/, so maybe cargo test builds them for that reason. But that's just my guess.
There's an unstable feature that lets us depend on crates with just binary targets (no library). So once that's in stable, we can switch to that setup. Hopefully this solution doesn't break before then :)
There was a problem hiding this comment.
Oh, it actually doesn't work :( It works with cargo nextest run, however, which is why I didn't notice that it was broken with regular cargo test. I don't know why the GitHub actions pass.
|
I ran into this when I tried to make The approach suggested in rust-lang/cargo#2911 (comment) seems somewhat promising, unless you've tried it already or it seems too hacky. Footnotes
|
Among other things, this prevented `jj` from working with `cargo binstall`. The trick is taken from rust-lang/cargo#2911 (comment). We could now also remove `--bin jj` from the installation commands in `install-and-setup.md`, but I'm not sure we should. That argument makes it clear that the binary is `jj`, not `jj-cli`. Fixes jj-vcs#216.
Among other things, this prevented `jj` from working with `cargo binstall`. The trick is taken from rust-lang/cargo#2911 (comment). We could now also remove `--bin jj` from the installation commands in `install-and-setup.md`, but I'm not sure we should. That argument makes it clear that the binary is `jj`, not `jj-cli`. Fixes #216.
Among other things, this prevented `jj` from working with `cargo binstall`. The trick is taken from rust-lang/cargo#2911 (comment). We could now also remove `--bin jj` from the installation commands in `install-and-setup.md`, but I'm not sure we should. That argument makes it clear that the binary is `jj`, not `jj-cli`. Fixes jj-vcs#216.
I noticed that
cargo install --path .will install the newfake-editorandfake-diff-editorbinaries. We don't want that, solet's move them into a separate crate. The tests still find them
without needing any modification.
Checklist
CHANGELOG.md