jj icon indicating copy to clipboard operation
jj copied to clipboard

tests: don't have cargo install test tools

Open martinvonz opened this issue 2 years ago • 4 comments

I noticed that cargo install --path . will install the new fake-editor and fake-diff-editor binaries. We don't want that, so let's move them into a separate crate. The tests still find them without needing any modification.

Checklist

  • [x] I have made relevant updates to CHANGELOG.md

martinvonz avatar Apr 20 '22 00:04 martinvonz

FWIW git-branchless puts helper binaries behind a feature flag which isn't enabled by default:

https://github.com/arxanas/git-branchless/blob/3595f3d151dffb974f7f6fddb500c79ee25a0431/git-branchless-lib/Cargo.toml#L25-L36

What enables the integration-test-bin feature?

martinvonz avatar Apr 20 '22 02:04 martinvonz

@martinvonz Me manually passing --feature integration-test-bin 😊

I guess what we're doing is different. Your binaries are supporting the relevant tests, but my binaries are the tests.

arxanas avatar Apr 20 '22 05:04 arxanas

@martinvonz Me manually passing --feature integration-test-bin 😊

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 https://github.com/rust-lang/cargo/issues/3670 has been fixed. I guess the problem is effectively the same as with using e.g. the git binary in tests. The difference is just that the binary can be built by cargo.

martinvonz avatar Apr 20 '22 15:04 martinvonz

Due to a recent build of jj, I'm now a proud owner of the fake-editor and fake-diff-editor binaries on my machine 😊

Sorry :( I don't know what the right solution to this problem is. I suppose we will also need a git binary on the path for tests in not too long (for running a git server to testing push/pull with, and to test GC, neither of which libgit2 supports AFAIK). For git, we can look for it on the path and have a $TEST_GIT override like I think git-branchless has. That doesn't work as well for these fake-* binaries, though.

martinvonz avatar Apr 29 '22 21:04 martinvonz

I ran into this when I tried to make jj installable with cargo binstall[^1].

The approach suggested in https://github.com/rust-lang/cargo/issues/2911#issuecomment-1483256987 seems somewhat promising, unless you've tried it already or it seems too hacky.

[^1]: For my own future reference, the magic incantantion for binstall seems to be --pkg-url="{ repo }/releases/download/v{ version }/jj-v{ version }-{ target }.{ archive-format }". Using jj instead of jj-cli is necessary. It's a bit surprising that I needed to add v before versions.

ilyagr avatar Sep 11 '23 06:09 ilyagr