feat: CLI get command improvements

[faassen]

This PR focuses on creating tests for the iroh get CLI command. The Iroh API is also improved along the way.

This required more work in the underlying API to make it more testable. What's tested here is everything except the actual IPFS behavior -- the focus is on the behavior of the CLI and API and its interactions with the filesystem.

trycmd tests

In iroh/tests/cmd you can see now quite a few .trycmd files. These describes the command-line interaction. Stuff behind $ is a command, there's a special ? failed` indicator if the command is considered to have failed, and the output of the command is shown.

New are the .out and .in directories with the same names as the .trycmd files. These describe the filesystem before the command runs (may be missing), and the filesystem after the command has run. This way we can describe the effects of the iroh get command - directories and files are supposed to be created. We can also test failure scenarios where we refuse to overwrite a directory that already exists. n0-computer/beetle#180 tracks various test cases.

@b5 is probably most interested in this bit

get_stream

The get high level CLI method has been removed from the mockable Api trait (read on to see where it went). Instead, a more low level but still useful API method get_stream has been added to the Api trait. This gets a stream of relative paths and OutType, describing the directories and files you can create.

This is a refactored version of the code @ramfox originally wrote and was in getadd.rs previously. The big difference is that it returns relative paths and doesn't calculate final destination paths -- that's up to the user of the API.

No async_trait macro for Api trait

The interactions between the Api trait, mockall macro and async_trait were getting so hairy I couldn't figure out how to express things anymore once I wanted to add get_stream. For my sanity and also to learn better how this really works underneath, I've rewritten the Api trait to describe itself explicitly in terms of (Local)BoxFuture and (Local)_BoxStream. It's more verbose but functionally equivalent and I could express what I wanted.

ApiExt trait

The ApiExt trait is a trait that implements the high level get command. It puts everything together: it handles various error conditions, accesses the stream and then writes the stream to the filesystem. It's basically iroh get.

The ApiExt trait is solely intended to contain default trait methods, and is automatically available when the Api contract is fulfilled (if you use it).

I factored out save_get_stream from getadd.rs to be solely concerned with turning a stream into files and directories on the files system. That makes it possible to test its behavior in isolation.

test fixture

The get test fixture now mocks get_stream and returns a fake stream made from a Vec. This defines the actual stuff that the CLI writes to disk.

relative_path

I now depend on the relative-path crate because what the get stream returns are clearly relative paths, and I want to force the user to do something with them for being able to actually write stuff. I want to expand the use of relative paths in the CLI where it makes sense in the future, as the use is now only minimal. It should help make it really explicit where the CLI is going to read and write.

[faassen]

Hm, get_cid_directory_overwrite_explicit_failure.trycmd is failing in CI but it passes locally if I run the tests for the iroh CLI crate specifically or when I run them for all crates.

This test checks that iroh get fails because the directory it is trying to work to already exists; the input directory structure is described in get_cid_directory_overwrite_explicit_failures.in. But instead it succeeds.

Oddly enough I have a very similar test that does work in CI; get_cid_directory_overwrite_failure.

Update: that's odd, for some reason the directories don't show up in git, as if they're ignored somehow.

Wow. It's because I named the directory foo and foo is in .gitignore!

I resolved it by renaming the said things away from foo, but nonetheless I think we should make foo not be in .gitignore.

[faassen]

One thing I realized is that the fixture stream I created may not actually reflect a real world stream; I'm unclear how it works when directories are wrapped or not. I'm going to experiment a bit with real-world streams.

[faassen]

Okay, after some manual testing I see I definitely broke the correct behavior. Withdrawing this from review until I fix it.

I tried to convert to draft but currently that button is causing a 404 on the github API for me, so let's just consider it a draft. :)

[faassen]

Okay, I have resolved the underlying issues; it turns out that the stream was not returning what I expected. I've adjusted it so it does.

I've also written tests for the single unwrapped file scenario.

[faassen]

Just for FYI: it's now ready for review again.

[faassen]

@ramfox can you have another look to see whether it does the right thing? I went for a slightly different approach than you did to handle the ipfs tail paths, but it appears to work.

[ramfox]

just come request for changes around comments!