Skip to content

test(axum-http-server): add package-local transport tests (#1348) - #2129

Closed
josecelano wants to merge 48 commits into
torrust:developfrom
josecelano:1348-add-tests-axum-http-server
Closed

test(axum-http-server): add package-local transport tests (#1348)#2129
josecelano wants to merge 48 commits into
torrust:developfrom
josecelano:1348-add-tests-axum-http-server

Conversation

@josecelano

@josecelano josecelano commented Sep 2, 2026

Copy link
Copy Markdown
Member

Summary

Adds and refines the package-local Axum HTTP server test safety net for bencoded announce/scrape response mapping, authentication-key extraction, request-ID middleware, listener lifecycle, and readable server-start scenarios.

The accompanying coverage evidence records an increase from 93.82% to 95.07% lines, 91.66% to 92.99% regions, and 89.54% to 90.86% functions (test-inclusive package-source measurement).

Tracking correction

Closes #2136.

Supersedes #2129, which was opened against #1348 before historical package-name changes were reconciled. Issue #1348 remains the separate udp-core testing subissue; #1349 remains the separate http-core testing subissue.

Validation

  • cargo test -p torrust-tracker-axum-http-server (34 unit tests, 55 integration tests)
  • linter all
  • mandatory pre-commit gate

Copilot AI lite review requested due to automatic review settings September 2, 2026 07:17
@josecelano josecelano self-assigned this Sep 2, 2026
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.61111% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.24%. Comparing base (67c437e) to head (0236a53).
⚠️ Report is 26 commits behind head on develop.

Files with missing lines Patch % Lines
packages/axum-http-server/src/server.rs 96.46% 3 Missing and 1 partial ⚠️
...ttp-server/src/v1/extractors/authentication_key.rs 91.66% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2129      +/-   ##
===========================================
+ Coverage    84.05%   84.24%   +0.18%     
===========================================
  Files          350      351       +1     
  Lines        27958    28316     +358     
  Branches     27958    28316     +358     
===========================================
+ Hits         23501    23854     +353     
- Misses        4063     4069       +6     
+ Partials       394      393       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The PR/docs claim to close/track GitHub issue #1348 for axum-http-server, but #1348 is for udp-tracker-core, so the issue references need to be reconciled before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds fast, package-local transport tests to torrust-tracker-axum-http-server (response bencode adapters, request-id middleware behavior, and a lifecycle regression around failed registration cleanup), and records completion evidence in the local issue/EPIC specs.

Changes:

  • Add in-process router tests asserting x-request-id propagation and UUID generation.
  • Add handler-level tests that decode/validate bencoded announce (compact/non-compact) and scrape responses.
  • Add a lifecycle regression test ensuring the TCP listener is released after service registration fails; update local issue/EPIC documentation and enable tower utilities for test support.
File summaries
File Description
packages/axum-http-server/src/v1/routes.rs Adds in-process router tests for request-id propagation/generation.
packages/axum-http-server/src/v1/handlers/announce.rs Adds bencode decode assertions for compact vs non-compact announce responses.
packages/axum-http-server/src/v1/handlers/scrape.rs Adds bencode decode assertions for scrape responses built from domain data.
packages/axum-http-server/src/server.rs Adds regression test for “registration fails → listener released” behavior.
packages/axum-http-server/Cargo.toml Enables tower util feature to support oneshot in tests.
docs/issues/open/1348-1347-add-tests-axum-http-server/ISSUE.md Marks tasks/ACs complete and records verification/coverage evidence.
docs/issues/open/1347-overhaul-packages-testing/EPIC.md Updates EPIC subissue status/notes to reflect this work as done.
Review details

Suppressed comments (2)

packages/axum-http-server/src/v1/handlers/announce.rs:336

  • to_bytes(..., usize::MAX) disables the body-size guard and can turn an unexpected regression into an OOM/hang during tests. Use a bounded limit that comfortably exceeds the expected announce response size.
        let body = to_bytes(response.into_body(), usize::MAX)

packages/axum-http-server/src/v1/handlers/announce.rs:363

  • to_bytes(..., usize::MAX) disables the body-size guard and can turn an unexpected regression into an OOM/hang during tests. Use a bounded limit that comfortably exceeds the expected announce response size.
        let body = to_bytes(response.into_body(), usize::MAX)
  • Files reviewed: 7/7 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

| Order | Issue | Local Spec | Status | Notes |
| ----- | ----------------------------------------------------- | --------------------------------------------------------------------- | ------ | -------------------------------------------------------------- |
| 1 | #1348 - Add tests to the axum-http-server package | `docs/issues/open/1348-1347-add-tests-axum-http-server/ISSUE.md` | TODO | Existing subissue; package-level test work. |
| 1 | #1348 - Add tests to the axum-http-server package | `docs/issues/open/1348-1347-add-tests-axum-http-server/ISSUE.md` | DONE | Added fast package-local response, request-ID, and lifecycle tests; verification and final review evidence recorded. |

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed: GitHub issue #1348 is titled Overhaul Packages Testing: Add tests for udp-tracker-core package, so the current EPIC row and local axum-http-server spec are incorrectly associated with it. No replacement GitHub issue exists for the axum-http-server work, and changing the number without the intended issue would create another incorrect reference. This requires maintainer clarification or creation of the intended subissue; the thread remains open.


- 2026-09-01 18:00 UTC - GitHub Copilot - Created a repository-local folder-style specification from GitHub issue #1348 and EPIC #1347. - https://github.com/torrust/torrust-tracker/issues/1348
- 2026-09-01 18:00 UTC - User/maintainer - Clarified that the work should increase the recorded coverage baseline by testing critical behavior, prioritize fast unit tests close to package code, and retain or add valuable package-level integration and end-to-end tests. - https://github.com/torrust/torrust-tracker/issues/1348
- 2026-09-01 - GitHub Copilot - Measured package-source coverage with `cargo llvm-cov -p torrust-tracker-axum-http-server --all-features --json`, summing the `summary` objects for files beneath `packages/axum-http-server/src/`: baseline: 1,229 lines / 1,153 covered (93.82%), 1,763 regions / 1,616 covered (91.66%), and 153 functions / 137 covered (89.54%). Final: 1,441 lines / 1,368 covered (94.93%), 2,072 regions / 1,930 covered (93.15%), and 172 functions / 156 covered (90.70%). These package-source totals include test code and are not a production-only coverage measurement; the new tests directly assert previously high-level-only response-adapter, request-ID middleware, and registration-cleanup behavior.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed the same tracking mismatch: #1348 is the open udp-tracker-core subissue, not an axum-http-server subissue. No intended replacement issue was found. The specification and PR metadata cannot be reconciled safely without maintainer clarification or a new issue, so no documentation change was made and this thread remains open.

// Act
let response = super::build_response(&announce_request, sample_announce_data());
let status = response.status();
let body = to_bytes(response.into_body(), usize::MAX)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied locally: announce response decoding now uses a named 64 KiB limit rather than usize::MAX. cargo fmt --all -- --check, cargo test -p torrust-tracker-axum-http-server (34 unit and 55 integration tests), and linter all pass. Per the requested no-commit/no-push constraint, this change is not on the PR head, so the thread remains open.

// Act
let response = super::build_response(scrape_data);
let status = response.status();
let body = to_bytes(response.into_body(), usize::MAX)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied locally: scrape success and error response decoding now use a named 64 KiB limit rather than usize::MAX. cargo fmt --all -- --check, cargo test -p torrust-tracker-axum-http-server (34 unit and 55 integration tests), and linter all pass. Per the requested no-commit/no-push constraint, this change is not on the PR head, so the thread remains open.

torrust-tracker-primitives = { version = "3.0.0", path = "../primitives" }
torrust-tracker-swarm-coordination-registry = { version = "0.1.0", path = "../swarm-coordination-registry" }
tower = { version = "0", features = [ "timeout" ] }
tower = { version = "0", features = [ "timeout", "util" ] }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied locally: the production tower dependency now enables only timeout, while test-only tower::ServiceExt::oneshot is supplied by a dev-dependencies entry with util. cargo fmt --all -- --check, cargo test -p torrust-tracker-axum-http-server (34 unit and 55 integration tests), and linter all pass. Per the requested no-commit/no-push constraint, this change is not on the PR head, so the thread remains open.

Comment thread packages/axum-http-server/src/server.rs Outdated
Comment on lines +558 to +560
let reserved_listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).expect("reserve HTTP listener address");
let bind_to = reserved_listener.local_addr().expect("read reserved listener address");
drop(reserved_listener);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied locally: renamed the dropped socket to available_listener and updated its expectation messages, making clear it only selects an available port before release. cargo fmt --all -- --check, cargo test -p torrust-tracker-axum-http-server (34 unit and 55 integration tests), and linter all pass. Per the requested no-commit/no-push constraint, this change is not on the PR head, so the thread remains open.

@josecelano
josecelano requested a review from a team as a code owner September 2, 2026 09:33

@josecelano josecelano left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Processed all six unresolved Copilot threads. Four minimal implementation fixes are validated locally but intentionally remain uncommitted and unpushed at the requester’s direction, so their threads remain open. Two documentation threads identify a genuine #1348 tracking mismatch and remain open pending maintainer clarification or creation of the intended axum-http-server issue. See the individual thread replies for decisions and validation.

| Order | Issue | Local Spec | Status | Notes |
| ----- | ----------------------------------------------------- | --------------------------------------------------------------------- | ------ | -------------------------------------------------------------- |
| 1 | #1348 - Add tests to the axum-http-server package | `docs/issues/open/1348-1347-add-tests-axum-http-server/ISSUE.md` | TODO | Existing subissue; package-level test work. |
| 1 | #1348 - Add tests to the axum-http-server package | `docs/issues/open/1348-1347-add-tests-axum-http-server/ISSUE.md` | DONE | Added fast package-local response, request-ID, and lifecycle tests; verification and final review evidence recorded. |

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed: GitHub issue #1348 is titled Overhaul Packages Testing: Add tests for udp-tracker-core package, so the current EPIC row and local axum-http-server spec are incorrectly associated with it. No replacement GitHub issue exists for the axum-http-server work, and changing the number without the intended issue would create another incorrect reference. This requires maintainer clarification or creation of the intended subissue; the thread remains open.


- 2026-09-01 18:00 UTC - GitHub Copilot - Created a repository-local folder-style specification from GitHub issue #1348 and EPIC #1347. - https://github.com/torrust/torrust-tracker/issues/1348
- 2026-09-01 18:00 UTC - User/maintainer - Clarified that the work should increase the recorded coverage baseline by testing critical behavior, prioritize fast unit tests close to package code, and retain or add valuable package-level integration and end-to-end tests. - https://github.com/torrust/torrust-tracker/issues/1348
- 2026-09-01 - GitHub Copilot - Measured package-source coverage with `cargo llvm-cov -p torrust-tracker-axum-http-server --all-features --json`, summing the `summary` objects for files beneath `packages/axum-http-server/src/`: baseline: 1,229 lines / 1,153 covered (93.82%), 1,763 regions / 1,616 covered (91.66%), and 153 functions / 137 covered (89.54%). Final: 1,441 lines / 1,368 covered (94.93%), 2,072 regions / 1,930 covered (93.15%), and 172 functions / 156 covered (90.70%). These package-source totals include test code and are not a production-only coverage measurement; the new tests directly assert previously high-level-only response-adapter, request-ID middleware, and registration-cleanup behavior.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed the same tracking mismatch: #1348 is the open udp-tracker-core subissue, not an axum-http-server subissue. No intended replacement issue was found. The specification and PR metadata cannot be reconciled safely without maintainer clarification or a new issue, so no documentation change was made and this thread remains open.

// Act
let response = super::build_response(&announce_request, sample_announce_data());
let status = response.status();
let body = to_bytes(response.into_body(), usize::MAX)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied locally: announce response decoding now uses a named 64 KiB limit rather than usize::MAX. cargo fmt --all -- --check, cargo test -p torrust-tracker-axum-http-server (34 unit and 55 integration tests), and linter all pass. Per the requested no-commit/no-push constraint, this change is not on the PR head, so the thread remains open.

// Act
let response = super::build_response(scrape_data);
let status = response.status();
let body = to_bytes(response.into_body(), usize::MAX)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied locally: scrape success and error response decoding now use a named 64 KiB limit rather than usize::MAX. cargo fmt --all -- --check, cargo test -p torrust-tracker-axum-http-server (34 unit and 55 integration tests), and linter all pass. Per the requested no-commit/no-push constraint, this change is not on the PR head, so the thread remains open.

torrust-tracker-primitives = { version = "3.0.0", path = "../primitives" }
torrust-tracker-swarm-coordination-registry = { version = "0.1.0", path = "../swarm-coordination-registry" }
tower = { version = "0", features = [ "timeout" ] }
tower = { version = "0", features = [ "timeout", "util" ] }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied locally: the production tower dependency now enables only timeout, while test-only tower::ServiceExt::oneshot is supplied by a dev-dependencies entry with util. cargo fmt --all -- --check, cargo test -p torrust-tracker-axum-http-server (34 unit and 55 integration tests), and linter all pass. Per the requested no-commit/no-push constraint, this change is not on the PR head, so the thread remains open.

Comment thread packages/axum-http-server/src/server.rs Outdated
Comment on lines +558 to +560
let reserved_listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).expect("reserve HTTP listener address");
let bind_to = reserved_listener.local_addr().expect("read reserved listener address");
drop(reserved_listener);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied locally: renamed the dropped socket to available_listener and updated its expectation messages, making clear it only selects an available port before release. cargo fmt --all -- --check, cargo test -p torrust-tracker-axum-http-server (34 unit and 55 integration tests), and linter all pass. Per the requested no-commit/no-push constraint, this change is not on the PR head, so the thread remains open.

@josecelano

Copy link
Copy Markdown
Member Author

Superseded by #2137, which tracks the Axum HTTP server testing work under the correct subissue #2136. This PR incorrectly referenced #1348; #1348 remains the separate udp-core testing subissue. Please continue review on #2137.

@josecelano josecelano closed this Sep 3, 2026
josecelano added a commit that referenced this pull request Sep 4, 2026
…verage (#2136)

7079ad3 docs(testing): assess test levels per package (Jose Celano)
252ced5 style(docs): align table formatting (Jose Celano)
1cc0885 docs(issues): add HTTP integration test review handoff (Jose Celano)
fd28c9f docs(issues): link axum HTTP testing pull request (Jose Celano)
92682a0 style(axum-http-server): apply nightly import formatting (Jose Celano)
4e3a8ce docs(review): record PR 2137 Copilot audit (Jose Celano)
a7fa154 test(axum-http-server): bound failure response decoding (Jose Celano)
560ba3b docs(axum-http-server): correct coverage reference (Jose Celano)
000a3ca docs(testing): correct scenario pattern subissue (Jose Celano)
2eb8824 test(axum-http-server): tighten test support (Jose Celano)
29bd75e docs(issues): correct axum HTTP subissue identity (Jose Celano)
5af32cc docs(issues): update completed test plan navigation (Jose Celano)
8e3529d docs(issues): complete shared bootstrap assessment (Jose Celano)
7325e3a docs(issues): complete server test refactor plan (Jose Celano)
1c79f0f docs(issues): assess server lifecycle coverage boundaries (Jose Celano)
14f7d6e test(axum-http-server): name normal start scenario (Jose Celano)
7ad4723 test(axum-http-server): clarify server start scenarios (Jose Celano)
ac122a7 docs(testing): add causal initial-state scenario fixture pattern (Jose Celano)
716b3cd docs(issues): plan server test bootstrap cleanup (Jose Celano)
66854c6 docs(issues): assess health check job coverage (Jose Celano)
d41389b test(axum-http-server): document listener cleanup risk (Jose Celano)
2fc187c test(axum-http-server): clarify server lifecycle tests (Jose Celano)
727c8ec docs(issues): plan HTTP server test refactoring (Jose Celano)
e1e56e3 docs(testing): refresh extractor coverage evidence (Jose Celano)
5c96ab3 docs(issues): complete authentication key test plan (Jose Celano)
53e1a37 test(axum-http-server): cover authentication key failure response (Jose Celano)
4b8126c test(axum-http-server): cover valid authentication key (Jose Celano)
3e61d29 test(axum-http-server): clarify authentication key mapping (Jose Celano)
aa82e98 docs(axum-http-server): correct authentication error status (Jose Celano)
2dfbe8d docs(issues): plan authentication key extractor tests (Jose Celano)
7e69834 docs(issues): complete routes test refactor plan (Jose Celano)
704b3bb docs(axum-http-server): document coverage commands (Jose Celano)
cea63b3 docs(issues): assess routes test helper (Jose Celano)
227c571 test(axum-http-server): standardize request ID header (Jose Celano)
fbae24e docs(testing): define external coverage boundaries (Jose Celano)
7acc268 docs(issues): plan routes test refactoring (Jose Celano)
528020f docs(testing): refresh HTTP server coverage evidence (Jose Celano)
1b64231 docs(issues): complete scrape test refactor plan (Jose Celano)
87c6290 test(axum-http-server): simplify scrape test events (Jose Celano)
c67a122 test(axum-http-server): cover multi-file scrape response (Jose Celano)
caedaab test(axum-http-server): simplify scrape test setup (Jose Celano)
4ecd4bf test(axum-http-server): cover scrape failure response (Jose Celano)
1e114a2 test(axum-http-server): clarify scrape handler tests (Jose Celano)
0838633 docs(axum-http-server): correct scrape handler description (Jose Celano)
6211b07 docs(issues): plan scrape test refactoring (Jose Celano)
02095e3 docs(issues): complete announce test refactor plan (Jose Celano)
7a22b16 docs(issues): assess announce handler coverage (Jose Celano)
26d2724 docs(issues): record announce fixture assessment (Jose Celano)
7912d0f test(axum-http-server): clarify announce error tests (Jose Celano)
b8be911 docs(issues): plan announce test refactoring (Jose Celano)
ca0f477 docs(testing): refine package testing workflow (Jose Celano)
d1b7f2b docs(issues): record HTTP server coverage evidence (Jose Celano)
341d022 test(axum-http-server): clarify adapter test assertions (Jose Celano)
38b8596 test(axum-http-server): clarify announce response assertions (Jose Celano)
d3240f9 test(axum-http-server): refine announce response scenarios (Jose Celano)
a262dce docs(testing): catalog response fixture refactoring (Jose Celano)
04fb2bd test(axum-http-server): simplify announce response fixtures (Jose Celano)
a75a866 docs(issues): document test development loop (Jose Celano)
cc36fc8 test(axum-http-server): add package-local transport tests (Jose Celano)

Pull request description:

  ## Summary

  Strengthens package-local test coverage for the Axum HTTP tracker server.

  - Adds focused bencoded announce and scrape response-mapping coverage.
  - Adds authentication-key extraction and request-ID middleware coverage.
  - Covers listener-registration cleanup and clarifies lifecycle scenarios.
  - Records coverage evidence, refactor plans, and reusable test-design guidance.

  The package-source coverage evidence is test-inclusive and records an increase from 93.82% to 95.07% lines, 91.66% to 92.99% regions, and 89.54% to 90.86% functions.

  ## Tracking correction

  Closes #2136.

  Supersedes #2129, which was opened against #1348 before historical package-name changes were reconciled. Issue #1348 remains the separate `udp-core` testing subissue; #1349 remains the separate `http-core` testing subissue.

  ## Validation

  - `cargo test -p torrust-tracker-axum-http-server` (34 unit tests, 55 integration tests)
  - `cargo +nightly fmt --all -- --check`
  - `linter all`
  - mandatory pre-commit gate

ACKs for top commit:
  josecelano:
    ACK 7079ad3

Tree-SHA512: bcc5f1aaf782df3da831058eb1c2d66269121778f68613e98940f7c151a4cc9aa280c69c7405fc3fa9b2f83c00786ac83d09ba440d97e3134bac3d6bb500adf0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Overhaul Packages Testing: Add tests for axum-http-server package

2 participants