feat: add CLI tests

[willemneal]

Currently the docs have example cli commands for the examples and this provides tests to ensure that the docs are accurate.

[gitpod-io[bot]]

[paulbellamy]

The tests themselves look fine. But they feel like cli tests, so it's odd they're in the examples repo. Feels like they should be in the soroban-tools repo.

[tomerweller]

I agree with @paulbellamy that these tests don't naturally fit in the examples repo. I think that in a perfect world they would be in the docs repo, as the docs are what is actually being tested. However, the docs don't currently have machinery to facilitate that. So for now having them here sounds like a reasonable compromise to me.

I suggest the following:

1. rename cli-tests to docs-tests or docs-cli-tests to reflect the fact that we're testing docs, not the cli
2. add a short README to this new tests package to explain what it does
3. add a rustdoc at the top of every test file that links to the relevant doc it's testing

Thoughts @willemneal @chadoh?

[willemneal]

I think doc-tests is a good name for now. Ideally we would make the examples a git submodule and then the docs repo could host the tests there.

[tomerweller]

I do wonder if there's a more ergonomic way to write these tests. Constructing these commands in code is somewhat painful and will be painful to update.

Is there a way to use the cli's parser to parse a single command string instead of constructing it by code? I think this will make writing and updating these tests easier - just copy&paste the cli command.

[chadoh]

@tomerweller Willem and I would rather have the ability to move somewhat in the other direction.

• https://github.com/stellar/soroban-tools/pull/443

^ this would allow us to:

• depend on a specific version of the CLI
• write Rust tests that execute Rust code, rather than calling out to an external CLI process
• speed up the tests

What do you think?

[chadoh]

I suggest the following:

• rename cli-tests to docs-tests or docs-cli-tests to reflect the fact that we're testing docs, not the cli
• add a short README to this new tests package to explain what it does
• add a rustdoc at the top of every test file that links to the relevant doc it's testing

All done.

Looks like someone needs to approve running the workflow so tests actually run.

[tomerweller]

write Rust tests that execute Rust code, rather than calling out to an external CLI process

I totally agree about writing tests in rust and not calling an external CLI process.

My question/wonder was whether there's a way in code to construct the cli command, using the library, instead of "manually" constructing it.

So instead of this

TestEnv::with_default(|e| {
e.new_cmd("contract")
  .arg("invoke")
  .arg("--wasm")
  .arg(&WASM.path())
  .args(["--id", "1"])
  .args(["--fn", "increment"])
  .args(["--"])
  .args(["--incr", "5"])
  .assert()
  .stderr("")
  .stdout("5\n");
});

One could potentially write something more "similar" to this:

TestEnv::with_default(|e| {
  e.new_cmd_pasre("contract invoke --wasm {} --id 1 --fn increment -- --incr 5", &WASM.path())
    .assert()
    .stderr("")
    .stdout("5\n");
});

This makes an assumption that there's an available parse function (or that one could be exposed) in the cli lib. Intuitively I think there should be one because the soroban-cli indeed parses commands from the cli that take that shape (but my intuition is often wrong). What's the feasibility of this?

[willemneal]

The idea would be to allow constructing an invoke, or any command, and then call a run function that returns the output.

We have tests in the cli to handle testing cli parsing but we can skip that part and have proper types.

[chadoh]

@tomerweller @tsachiherman @paulbellamy this now uses the v0.7.1 versions of both soroban-cli and soroban-test. I'd love another round of review.

soroban-test's "new way" of doing this is a little frustrating still, because soroban-cli does not allow running commands in a consistent way. @willemneal's thought on how to fix this is to make a generic async run function that all commands implement, so that the tests can be written in a consistent asynchronous way. This would also allow us to make the tests run against an RPC node down the line, potentially running the exact same tests and only setting an environment variable.

However, even without this, we believe that these tests are a great start. I think it's time to merge this PR; we can open new ones to update these tests as the testing library and CLI evolve.

[chadoh]

@paulbellamy can you approve the workflow to run?

[paulbellamy]

Nope, I don't have write access to this repo. Maybe @leighmcculloch can?

[tsachiherman]

Just did. Could you address the conflicts ?

[paulbellamy]

@chadoh Seems like the test runner is failing to launch the soroban cli. Is it actually being built where expected?

Edit: Seems like the .github/workflows/rust.yml probably needs to cargo install --locked soroban-cli before running the tests. Good luck getting it to install the version matching whats in test/doc-tests tho 🤷

[paulbellamy]

Aaaaaaand the top-level Cargo.toml was removed, so there's no more make build-test-wasms, and this is broken now. :(