Some docs tests are very slow

[josecelano]

Some doc tests are very slow now.

cargo test --doc

These are the ones that take longer:

test src/servers/http/percent_encoding.rs - servers::http::percent_encoding::percent_decode_info_hash (line 28)
test src/servers/http/percent_encoding.rs - servers::http::percent_encoding::percent_decode_peer_id (line 59)
test src/servers/http/v1/query.rs - servers::http::v1::query::Query::get_param (line 33)
test src/servers/http/v1/query.rs - servers::http::v1::query::Query::get_param (line 46)
test src/servers/http/v1/query.rs - servers::http::v1::query::Query::get_param_vec (line 62) 
test src/servers/http/v1/query.rs - servers::http::v1::query::Query::get_param_vec (line 75)
test src/servers/http/v1/requests/announce.rs - servers::http::v1::requests::announce::Announce (line 32)
test src/servers/http/v1/responses/announce.rs - servers::http::v1::responses::announce::CompactPeer (line 207)
test src/servers/http/v1/responses/announce.rs - servers::http::v1::responses::announce::NormalPeer (line 155)
test src/servers/http/v1/responses/error.rs - servers::http::v1::responses::error::Error::write (line 29)
test src/servers/http/v1/responses/scrape.rs - servers::http::v1::responses::scrape::Bencoded (line 14)
test src/servers/http/v1/services/peer_ip_resolver.rs - servers::http::v1::services::peer_ip_resolver::invoke (line 62) 
test src/servers/http/v1/services/peer_ip_resolver.rs - servers::http::v1::services::peer_ip_resolver::invoke (line 84)

It can even crash your computer if you don't have enough memory. You can run them with:

cargo test -- --test-threads=1

It would be even slower, but you can execute them.

We have to find out why they are so slow now.

I guess the problem is:

Each doc-test generates a standalone binary for execution. Since these binaries are isolated from each other, they may independently recompile parts of your crate, especially if there is no mechanism to reuse already compiled artifacts.

For example, in this test:

/// ```rust
/// use std::str::FromStr;
/// use torrust_tracker::servers::http::percent_encoding::percent_decode_info_hash;
/// use bittorrent_primitives::info_hash::InfoHash;
/// use torrust_tracker_primitives::peer;
///
/// let encoded_infohash = "%3B%24U%04%CF%5F%11%BB%DB%E1%20%1C%EAjk%F4Z%EE%1B%C0";
///
/// let info_hash = percent_decode_info_hash(encoded_infohash).unwrap();
///
/// assert_eq!(
///     info_hash,
///     InfoHash::from_str("3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0").unwrap()
/// );
/// ```

We need to compile the main torrust_tracker package. FOr this particular case we can extract the functionality to a new package. In fact, it's duplicated. THe comment for the duplication:

/* code-review: this module is duplicated in torrust_tracker::servers::http::percent_encoding.
   Should we move it to torrust_tracker_primitives?
*/

But in other cases, we can also do the same. If, for some reason, we cannot do it, I would not compile the example for these slow cases.

cc @mario-nt @da2ce7

[josecelano]

I think I'm going to extract the duplicate code into a new package: torrust_http_protocol

These two modules:

• packages/tracker-client/src/http/url_encoding.rs
• src/servers/http/percent_encoding.rs

will become one in:

packages/
├── http_protocol       -> torrust-http-protocol
├── ...                 -> ...
└── tracker-client      -> bittorrent-tracker-client

console/
└── tracker-client      -> torrust-tracker-client

Realtes to: #753

This can be the first refactor. In the long term, we can consider using aquatic_http_protocol.

I think test execution times should decrease by removing the dependency on the main lib. If not, I will change the docs test to not compile and execute them. There are unit tests covering the same functionality.

cc @da2ce7

[josecelano]

It seems doc-tests are still slow, and they consume a lot of memory. Reported by @mario-nt.

This is in my PC:

[josecelano]

@mario-nt and I agreed on:

• Execute all tests one at a time with time to detect slow tests
• For slow tests:
  • Check if the problem is the same: the doc-test needs to compile the torrust-tracker
    • If the problem is the same
      • Extract the code to a subpackage
        • Check if there is a subpackage where we can move that code (the code with the doc-test)
          • If there is a package -> move to that package
          • If there is no package -> create a new package if it makes sense
    • If the problems is not the same -> do not compile the doc-test, leave as plain text.

[josecelano]

Hi @mario-nt @da2ce7, all the slow tests are in the src/servers/http/v1. They are slow because they use the main lib use torrust_tracker::.

time cargo test --doc src/servers/http/v1/services/peer_ip_resolver.rs
test src/servers/http/v1/services/peer_ip_resolver.rs - servers::http::v1::services::peer_ip_resolver::invoke (line 84) ... ok
test src/servers/http/v1/services/peer_ip_resolver.rs - servers::http::v1::services::peer_ip_resolver::invoke (line 62) ... ok
21 seconds

time cargo test --doc src/servers/http/v1/query.rs
test src/servers/http/v1/query.rs - servers::http::v1::query::Query::get_param_vec (line 75) ... ok
test src/servers/http/v1/query.rs - servers::http::v1::query::Query::get_param_vec (line 62) ... ok
test src/servers/http/v1/query.rs - servers::http::v1::query::Query::get_param (line 46) ... ok
test src/servers/http/v1/query.rs - servers::http::v1::query::Query::get_param (line 33) ... ok
24 seconds

time cargo test --doc src/servers/http/v1/responses/error.rs
test src/servers/http/v1/responses/error.rs - servers::http::v1::responses::error::Error::write (line 29) ... ok
19 seconds

time cargo test --doc src/servers/http/v1/responses/scrape.rs
test src/servers/http/v1/responses/scrape.rs - servers::http::v1::responses::scrape::Bencoded (line 14) ... ok
19 seconds

time cargo test --doc [src/servers/http](https://github.com/torrust/torrust-tracker/issues/753)/v1/responses/announce.rs
test src/servers/http/v1/responses/announce.rs - servers::http::v1::responses::announce::CompactPeer (line 207) ... ok
test src/servers/http/v1/responses/announce.rs - servers::http::v1::responses::announce::NormalPeer (line 155) ... ok
20 seconds

time cargo test --doc src/servers/http/v1/requests/announce.rs
test src/servers/http/v1/requests/announce.rs - servers::http::v1::requests::announce::Announce (line 32) ... ok
20 seconds

time cargo test --doc src/servers/http/v1/responses/announce.rs
test src/servers/http/v1/responses/announce.rs - servers::http::v1::responses::announce::CompactPeer (line 207) ... ok
19 seconds

Total: 123 seconds

123 seconds is not too long, but that's on my PC. On the GitHub runner, it tasks 3m 39s.

I think the good solution would be to move those files to the packages/http-protocol package. However, for some cases, it is not trivial because we also need to extract dependencies into new workspace packages (like crate::core)

For now, I will disable this doc test be replacing the following:

/// ```rust

With:

/// ```text

For the slow tests, and I will open a new issue to extract generic code in src/servers/http/ to packages/http-protocol. That would require to extract a new package tracker-core.