Make CLI integration tests automatically run the most up-to-date version of the CLI

[Michael-F-Bryan]

Something that always annoys me is that the CLI integration tests require you to remember to recompile the wasmer CLI in release mode every time you want to run the test suite. This tends to be quite a) error-prone and b) slow.

I'm introducing a wasmer-cli-shim executable so we can use the $CARGO_BIN_EXE_<name> environment variable cargo sets when compiling integration tests. This will make sure we always run an up-to-date CLI executable because wasmer-cli-shim will be recompiled whenever wasmer-cli changes.

As a happy side-effect, this means the old run_unstable tests no longer need to do their defensive cargo run --package=wasmer-cli and can run the wasmer-cli-shim executable directly.

Fixes #4014.

[theduke]

I get the motivation, and it seems reasonable.

However, this does seem to break a bunch of assumptions and workflows.

• this makes us build the test CLI in a different manner than the "real CLI", because that is handled in the makefile, which could lead to drift in feature flags or various other compilation-affecting env vars/flags, which cause the integration to not exactly test the proper CLI

• Related to the above: integratin-cli specific dependencies could lead to version drift between the shim and the full CLI, which could affect test results due to slightly changed behaviour. Image for example the integration cli using a higher but compatible version of a dependencies, which doesn't get unified for some reason or another. Not a problem if everything was a workspace dependency, and probably a somewhat theoretical concern, because the CLI doesn't usually use exact versions, but it could still happen

• are there no tests exercising llvm at the moment? the feature is not enabled

• As far as I understand, the full build currently also handles c api related building and packaing, which is required to succeed for all the build-exe related tests to pass, this hides that fact, and doesn't obviate the need for running make in order to get the tests to run properly
  (note: I already suggested breaking out the build-exe tests into a separate step in the past, for somewhat related reasons)

• Prevents from running the API against a specific existing binary. no idea if that is useful

I'm tentatively in favour despite this, but I think we'll need to hear what @ptitSeb and @syrusakbary think, and if they are aware of any gotchas.

A lot of this can be mitigated by keeping the WASMER_PATH env var as an optional override, and only use the shim if it is not set.

[theduke]

The comment is a bit confusing.

Suggested change

	# This greatly speeds up debug builds because these crates are extremely slow
	# This greatly improves runtime performance of debug builds, because these crates are extremely slow

And yes, I'm perfectly aware who originally wrote that comment, so blame is on me.

[theduke]

Seeing different command names opposed to the the proper wasmer command would be somewhat confusing, wouldn't it?
Needs to be at least documented, maybe in the README.

[theduke]

Addendum: I think the CLI should keep using the make build-wasmer built binary.

This can be used to make local iteration easier regardless.

[Michael-F-Bryan]

Thanks for the input, @theduke!

In retrospect, I'm thinking it might not be a good idea to proceed with this PR.

The overall make test-integration-cli workflow is broken outside of CI for all intents and purposes anyway¹, but adding yet another way to build the CLI won't help the situation. This approach also isn't a perfect solution so we'd probably exchange one set of footguns for a different one.

For now, I might just keep running my cargo watch command which rebuilds and sets the correct feature flags.

this makes us build the test CLI in a different manner than the "real CLI", because that is handled in the makefile, which could lead to drift in feature flags or various other compilation-affecting env vars/flags, which cause the integration to not exactly test the proper CLI

Yeah, I guess we need to take a step back and ask what the integration tests are trying to achieve in the first place.

Are they testing our compiler implementations, our WASIX implementation, CLI sub-command, or all of the above?

are there no tests exercising llvm at the moment? the feature is not enabled

That's correct. We only run the integration test suite once, so CI only does integration tests against one compiler (I'm guessing cranelift?).

As far as I understand, the full build currently also handles c api related building and packaing, which is required to succeed for all the build-exe related tests to pass

Good point. In the long term, I'd prefer to split the create-exe tests out into a separate crate and build job (#3568).

Prevents from running the API against a specific existing binary. no idea if that is useful

I don't think I've ever had a concrete situation where I wanted this. It might be useful for a hypothetical WASIX spec test suite, but when you are wanting to test the CLI you'll always want to re-compile so you know what code is being tested.

Footnotes

1. • Using --nocapture means you get bombarded with so much output (I just did a run and it generated 40760 lines of output) that it's unusable
   • the Makefile and our feature flags like to recompile the world (even with sccache)
   • the code for setting up the C API and running create-exe tests has never worked on any of my Windows/Linux/MacOS machine - you always get #include "wasmer.h" errors
   • we use --test-threads=1 because of The wasmer-integration-tests-cli test suite isn't hygienic #4015 so the tests themselves are really slow
   • I could go on for hours...
   ↩