fix(testenv): disable downloads (bitcoind and electrsd) for docs.rs builds in …

[riverKanies]

…crate testenv

Description

address this issue https://github.com/orgs/bitcoindevkit/projects/14/views/1?pane=issue&itemId=81939679&issue=bitcoindevkit%7Cbdk%7C1627
where docs.rs build is failing for bdk_testenv crate

Update: to get tests to pass RUSTFLAGS="--cfg docsrs" cargo +nightly test I had to add ignore flags for the docs env to all three blockchain client crates (bitcoid_rpc, electrum, esplora)

Notes to the reviewers

I was not able to reproduce the build issue locally, so this will have to be tested live unless someone else can reproduce the build error https://docs.rs/crate/bdk_testenv/0.10.0/builds/1377651

more details in this thread on discord https://discord.com/channels/753336465005608961/1265333904324427849/1304476756660719668

Bugfixes:

• I'm linking the issue being fixed by this PR

[notmandatory]

This should conditionally remove code when the "docsrs" attribute is enables:

#[cfg(not(docsrs))]

To test that tests still build and run with the docsrs attribute enabled try this command:

 RUSTFLAGS="--cfg docsrs" cargo +nightly test

[riverKanies]

RUSTDOCFLAGS="--cfg docsrs" cargo +nightly doc --no-deps --document-private-items --open
to see what this should look like on docs.rs

[riverKanies]

@notmandatory edited PR desc to include:
Update: to get tests to pass RUSTFLAGS="--cfg docsrs" cargo +nightly test I had to add ignore flags for the docs env to all three blockchain client crates (bitcoid_rpc, electrum, esplora)

this seems like a lot, but maybe it is what we want?

[notmandatory]

I think you're on the right track, we don't want to run any tests that use the bitcoind or electrsd crates since those are the ones that download and run those two daemons. I wonder if there's some cleaner way to ignore those tests. Open to ideas.

[riverKanies]

@notmandatory were you saying tuesday that you don't care to include the note in the docs with
#![cfg_attr(docsrs, feature(doc_cfg))]
and
#[cfg_attr(docsrs, doc(cfg(not(docsrs))))]
so I should just remove those? and just leave the #[cfg(not(docsrs))] attr on everything we don't want to compile?

[riverKanies]

the alternative that was mentioned is conditionally downloading the files when not running docs, like mock them out or something, but if the docs actually run the tests then that's not an option is it?

[notmandatory]

@notmandatory were you saying tuesday that you don't care to include the note in the docs with #![cfg_attr(docsrs, feature(doc_cfg))] and #[cfg_attr(docsrs, doc(cfg(not(docsrs))))] so I should just remove those? and just leave the #[cfg(not(docsrs))] attr on everything we don't want to compile?

Yes these should be removed since I don't think they do what we need.

[riverKanies]

they are just UI additions as described here https://discord.com/channels/753336465005608961/1304476756660719668/1306644061561688107 @notmandatory

[ValuedMammal]

Without having replicated the build environment used by docs.rs, my best guess is to somehow enable building bdk_testenv with a limited feature set, something like the following. This is based on the observation that the "download" feature in electrsd is responsible for triggering downloads during the build phase. Obviously we utilize the feature when testing the other blockchain crates but it isn't strictly needed to just document testenv. I don't think docs.rs is running any tests so it wouldn't make sense to cfg out any of the types or methods. You can check this works by doing a cargo clean and then cargo build -p bdk_testenv --no-default-features and see that no binary executables end up in the build outputs of the target/ dir

[dependencies]
bdk_chain = { path = "../chain", version = "0.20.0", default-features = false }
electrsd = { version = "0.28.0", default-features = false }

[features]
default = ["std", "download"]
download = ["electrsd/bitcoind_25_0", "electrsd/esplora_a33e97e1", "electrsd/legacy"]
std = ["bdk_chain/std"]
serde = ["bdk_chain/serde"]

[package.metadata.docs.rs]
no-default-features = true

[riverKanies]

@ValuedMammal yeah it wouldn't make sense for the tests to be run for docs, but they are being compiled, and just that requires the downloads it seems.

I think you're solution might work, we may just have to try and see. We could try your solution first since it's more minimal. Do you want to make a PR for it or should I?

Edit: looks like this causes tests to fail btw. I'll see if I can get them working...
nope. the solution I came to also involves conditional compilation flags in addition to the metadata, so doesn't seem better to me 😓

[notmandatory]

I agree with @ValuedMammal that we should be able to just disable the download features for electrsd and bitcoind. I was able to get the docs to build with no network access with these steps:

1. add new "download" feature

   [dependencies]
   ...
   electrsd = { version = "0.28.0", features = [ "legacy" ] }
   
   [features]
   default = ["std", "download"]
   download = ["electrsd/bitcoind_25_0", "electrsd/esplora_a33e97e1"]
   ...

2. turn off the local network on my computer
3. build docs with no-default-features, we should also be able to depoy to docsrs this way

   cargo doc --no-default-features
   

I also verified that if don't disable default features and my network is disabled the docs build fails.

[riverKanies]

@notmandatory doing this causes the same test to fail as valuedmammal's original suggestion
cargo test -p bdk_esplora --test async_ext

`failures:

---- test_async_update_tx_graph_stop_gap stdout ----
Error: you need to provide an env var BITCOIND_EXE or specify a bitcoind version feature

Caused by:
Called a method requiring a feature to be set, but it's not`

p.s i'm specifying this test bc it uses TestEnv

I went ahead and made a new PR for the suggested changes just in case I'm running tests wrong locally somehow #1722

and I confirmed that the solution in this PR does not work with network off 😅