Consider ignore-by-default for tests that exercise optional external tools

[bstrie]

For the purpose of making the codebase friendlier to developers, it's nice to be able to tell them that a git clone && cargo build is all they need to get started sending PRs. We also want to encourage developers to pass cargo test before submitting a PR, for the sake of our CI credits and reviewer resources. However, currently a cargo test build of the full repository will fail if either redis (for the optional redis store) or ganache (for the optional ethereum settlement engine) is not installed. Going forward I expect that future settlement engines and stores will likely require similar external tools to be installed first, and it may be impractical to ask every developer to install every optional external dependency. There are a few options:

1. Tell people not to run cargo test from the root of the repository, but instead from within the specific crate that they're working on. This way, rather than running the tests from all crates in the workspace, only the tests from that specific crate will be run (for which, presumably, the developer will want to have the associated dependencies installed).

2. Use the ignore test attribute on any individual tests which require external dependencies. This attribute is used to annotate tests which are not expected to be executed during ordinary runs of the test suite. Tests which are ignored in this way are noted in the summary at the very end of the cargo test output:

   test result: ok. 1 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out

   CI would then be configured to run ignored tests as well. The upside is that typical developers will be able to cleanly pass an invocation of cargo test, while the downside is that developers that actually do want to exercise those tests will be required to remember to run them on their own.

3. Move all optional components into their own separate repositories (or at minimum one separate repo for all of them (or possibly one repo per "axis" of options: one for stores, one for settlement)), so that users simply interested in working on the main interledger crate only need to worry about its required dependencies (and any optional dependencies that they explicitly enable via feature flags), whereas people working on optional dependencies don't have to worry about remembering passing flags while testing. We could carefully document in the readmes of each repo that interledger-rs is the main repo that developers should be concerned with, to avoid a pitfall of the microrepository approach encountered in the JS implementation (and all required dependencies could remain in this repo, although then we might have to determine whether crates like interledger-btp technically count as required...).

[tarcieri]

I'm a big fan of #[ignore] for this purpose

[bstrie]

@tarcieri Indeed, #ignore is the easiest way forward right now, although I've also just added a third option to consider which would be more discussion and more work to implement but might be more satisfying in some ways.

[bstrie]

One thing that's attracting me to the idea of keeping optional features out-of-tree is that I've been trying to keep an eye on (and ideally reduce) our large number of Rust dependencies as well, and I'm concerned that a future version of interledger-rs with a lot of stores and settlement layers is going to make git clone && cargo build take an intractably long time; note that the recent PR adding an Ethereum settlement layer took the number of crates that need to be built during cargo build from 332 to 404, which some might find unreasonable (there's no clear line for the number of transitive dependencies a project "should" have, but my gut tells me that large projects should max out around 150, maybe 200 if those dependencies are especially mature).

[tarcieri]

@bstrie as someone who has strived to keep the number of transitive dependencies of Abscissa as low as possible, it's really hard to say. Re: 150, that's fewer than the total number of transitive dependencies of crossbeam, for example.

When counting that number, I'd perhaps subtract the transitive dependencies of widely used ecosystem crates which themselves have large numbers of transitive dependencies.

[bstrie]

@tarcieri I'm curious if we're both looking at different metrics. In my case, when I check out a fresh copy of interledger-rs and do cargo check (I'd do a build, but I'm on battery now :P), the running total of crates being compiled according to the cargo output is 404. When I do the same for crossbeam, I get 19 (which presumably also includes crossbeam's dev-dependencies, which we wouldn't be getting in our own usage unless I'm drastically mistaken about when dev-dependencies get pulled in). By what metric do you get 150?

[tarcieri]

Whoops sorry, seems I was looking at the wrong property in an index of these things, it's >150 inverse dependencies, not transitive dependencies.

[bstrie]

No problem. :) Out of curiosity I looked at Crossbeam's Cargo.lock to see how many packages it contains and the answer is 73, which I presume encompasses what you'd be compiling if you enabled all features. Curiously for a fresh copy of interledger-rs the number of packages in Cargo.lock is 343, which is... less than the number that we compile?

[tarcieri]

@bstrie there are several cases where that may happen, e.g. dependencies which are gated on by OS in Cargo.toml, but are not explicitly tagged as optional = true will still show up in Cargo.lock regardless of what OS you're on (dirs-rs is a good example of this).

[bstrie]

@tarcieri (Offtopic, but regarding aggressive dependency minimization I found this recent hack of Tokio's pretty interesting: https://github.com/tokio-rs/tokio/commit/b89ed00a0d0b5ffa8fd3fa3a1faee31b5693cd64 )

[tarcieri]

@bstrie I've done the same in several projects. getrandom suits my needs for cryptography, and avoids rand's many dependencies

[bstrie]

This is far less of a priority these days now that the settlement-engines live in their own repo, since those were the tests that took the most time (settling things on blockchains and etc.). Still might be worth doing for tests that hit the network.