torrust / torrust-tracker

A modern and feature-rich (private) BitTorrent tracker.
https://torrust.com
GNU Affero General Public License v3.0
372 stars 44 forks source link

Some docs tests are very slow #1097

Closed josecelano closed 2 days ago

josecelano commented 1 week ago

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 commented 4 days ago

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

These two modules:

will become one in:

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

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

Realtes to: https://github.com/torrust/torrust-tracker/issues/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