Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit tests for Tracker #210

Merged

Conversation

josecelano
Copy link
Member

@josecelano josecelano commented Mar 2, 2023

I'm double-checking the new Axum HTTP tracker and adding more tests.

@josecelano josecelano linked an issue Mar 2, 2023 that may be closed by this pull request
@josecelano
Copy link
Member Author

josecelano commented Mar 2, 2023

Hi @da2ce7 @WarmBeer, I've just realized we have a duplicate struct:

/// Swarm statistics for one torrent.
/// Swarm metadata dictionary in the scrape response.
/// BEP 48: <https://www.bittorrent.org/beps/bep_0048.html>
#[derive(Debug, PartialEq, Default)]
pub struct SwarmMetadata {
    pub complete: u32,   // The number of active peers that have completed downloading (seeders)
    pub downloaded: u32, // The number of peers that have ever completed downloading
    pub incomplete: u32, // The number of active peers that have not completed downloading (leechers)
}

/// Swarm statistics for one torrent.
/// Alternative struct for swarm metadata in scrape response.
#[derive(Debug, PartialEq, Default)]
pub struct SwarmStats {
    pub completed: u32, // The number of peers that have ever completed downloading
    pub seeders: u32,   // The number of active peers that have completed downloading (seeders)
    pub leechers: u32,  // The number of active peers that have not completed downloading (leechers)
}

I introduced the SwarmMetadata to use the same names as in the BEP 48. I think SwamStats is a better name and the names for attributes as well.

I would keep both and use SwarmMetadata only on the scrape context and SwamStats for the rest of the application. Maybe we could use SwarmMetadata when we build the final response.

On the other hand, It could be confusing to use two different names, although the BitTorrent specification (BEPs) do that all the time. The SwarmMetadata.complete != SwamStats.completed is especially misleading.

@josecelano josecelano added the Quality & Assurance Relates to QA, Testing, and CI label Mar 2, 2023
@josecelano josecelano force-pushed the issue-207-axum-http-tracker-add-tests branch from 525cd30 to fad6834 Compare March 3, 2023 13:31
@josecelano josecelano changed the title Axum HTTP tracker: double-check and add tests Axum HTTP tracker: add tests for Tracker functions Mar 3, 2023
@josecelano josecelano changed the title Axum HTTP tracker: add tests for Tracker functions Add unit tests for Tracker Mar 3, 2023
@josecelano josecelano force-pushed the issue-207-axum-http-tracker-add-tests branch from 0606799 to 71589c4 Compare March 3, 2023 17:35
@josecelano josecelano force-pushed the issue-207-axum-http-tracker-add-tests branch from 71589c4 to 7fb92b5 Compare March 3, 2023 17:47
@josecelano josecelano marked this pull request as ready for review March 3, 2023 17:55
@josecelano
Copy link
Member Author

Hi @da2ce7 @WarmBeer, this is ready to review. Finally, I've added only unit tests for the core Tracker. I will open new PRs to add more unit tests for the new HTTP Axum implementation. In general, I think we should increase the unit tests and decrease the integration tests.

@josecelano josecelano merged commit 6a15202 into torrust:develop Mar 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Quality & Assurance Relates to QA, Testing, and CI
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add unit tests for Tracker
1 participant