fix(tracker): replace address-keyed bootstrap containers with ordered vec

[josecelano]

Summary

Replaces HashMap<SocketAddr, Container> with Vec<(ConfigurationInstanceId, Container)> so that duplicate port-zero configuration entries each receive their own container.

Bootstrap fix

• AppContainer now stores HTTP and UDP instance containers in order-preserving Vecs keyed by (ConfigurationInstanceId, Container).
• Lookup methods take a configuration index instead of SocketAddr.
• Duplicate 0.0.0.0:0 entries no longer overwrite each other.

Test restructuring

• Replaces the old tests/stats.rs + tests/servers/ tree with tests/aggregate_stats_port_zero.rs and tests/aggregate_stats_fixed_ports.rs, following the binary-per-configuration convention.
• Each binary starts one tracker instance and runs scenario functions sequentially.
• Both binaries include HTTP and UDP aggregate-statistics scenarios using the tracker-client library.
• Splits tests/common/ into focused submodules: workspace.rs, announce.rs, statistics.rs.

Supporting changes

• Adds http_announce and udp_announce helpers to packages/test-helpers.
• Adds UDP dev-dependencies (torrust-tracker-client-lib, torrust-tracker-udp-protocol) to test-helpers.
• Updates deny.toml to allow test-helpers as a wrapper for protocol crates.
• Updates tests/AGENTS.md to reflect the new structure.

Validation

• Focused tests for primitives, HTTP server, REST API server, UDP server, and integration binaries.
• Full pre-commit checks: cargo machete, cargo deny check bans, linter all, documentation tests.
• Full pre-push checks: nightly format, check, doc, and stable test suite.

Scope

This PR deliberately does not implement duplicate-port-zero aggregate-statistics filtering (#2039) or registry metadata migration (#2041).

Refs #2035

[Copilot]

Pull request overview

This PR addresses #2035 by changing bootstrap container storage from an address-keyed map to an order-preserving list keyed by configuration instance identity, ensuring repeated 0.0.0.0:0 blocks do not overwrite each other. It also restructures integration tests into per-configuration binaries and introduces shared HTTP/UDP announce helpers in test-helpers.

Changes:

• Replace HashMap<SocketAddr, Container> with Vec<(ConfigurationInstanceId, Container)> for HTTP/UDP instance containers and switch lookups to configuration indices.
• Restructure integration tests into aggregate_stats_port_zero.rs and aggregate_stats_fixed_ports.rs, with shared helpers under tests/common/.
• Add HTTP/UDP announce helpers to packages/test-helpers and adjust dependency policy (deny.toml).

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tests/stats.rs	Removes legacy stats integration-test scaffold.
tests/servers/mod.rs	Removes legacy servers test module root.
tests/servers/api/mod.rs	Removes legacy API test module root.
tests/servers/api/contract/mod.rs	Removes legacy contract test module root.
tests/servers/api/contract/stats/mod.rs	Removes legacy stats contract scenarios (superseded by new binaries).
tests/common/workspace.rs	New workspace/startup + registar-based endpoint discovery helpers.
tests/common/statistics.rs	New REST API aggregate-statistics query helper.
tests/common/mod.rs	Splits common helpers into submodules and re-exports them.
tests/common/announce.rs	New wrappers delegating announces to test-helpers.
tests/aggregate_stats_port_zero.rs	New port-zero multi-instance aggregate-statistics integration test binary.
tests/aggregate_stats_fixed_ports.rs	New fixed-port multi-instance aggregate-statistics integration test binary.
tests/AGENTS.md	Updates documentation to match the new integration-test structure.
src/container.rs	Switches per-instance container storage to ordered vec keyed by configuration instance identity.
src/app.rs	Updates instance startup to resolve containers by configuration index (not bind address).
packages/test-helpers/src/udp.rs	Adds UDP announce helper performing connect→announce handshake.
packages/test-helpers/src/http.rs	Adds HTTP announce helper using tracker-client + protocol types.
packages/test-helpers/src/lib.rs	Exposes new http and udp helper modules.
packages/test-helpers/Cargo.toml	Adds client/protocol deps required by new helpers.
docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md	Updates spec metadata/links (but currently includes a stale removed test path).
docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md	Restructures evidence doc (but currently contains a duplicated heading artifact).
docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md	Updates spec metadata/plan (but currently includes a stale removed test path).
deny.toml	Allows torrust-tracker-test-helpers as an approved wrapper for protocol crates.
Cargo.lock	Locks added dependencies for torrust-tracker-test-helpers.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

❌ Patch coverage is 94.28571% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.97%. Comparing base (6af41e4) to head (8151e92).

Files with missing lines	Patch %	Lines
src/container.rs	92.85%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2044      +/-   ##
===========================================
+ Coverage    81.63%   81.97%   +0.34%     
===========================================
  Files          346      346              
  Lines        24735    24737       +2     
  Branches     24735    24737       +2     
===========================================
+ Hits         20192    20279      +87     
+ Misses        4238     4147      -91     
- Partials       305      311       +6     

☔ 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]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (10)

docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md:109

• These lines reference deferred test coverage living in tests/servers/api/contract/stats/mod.rs, but that file no longer exists after the test restructuring in this PR. This should be updated to point at the new integration-test binaries or phrased generically to avoid stale paths.

- Add the deferred aggregate-statistics cases in
  `tests/servers/api/contract/stats/mod.rs`: UDP enabled/disabled listeners on
  distinct fixed ports, then HTTP and UDP listeners with repeated port-zero
  bindings after bootstrap identity is available.

docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md:145

• The implementation plan item T10 references tests/servers/api/contract/stats/mod.rs, which was removed in this PR. Update the plan to reference the current integration-test structure under tests/ (or the new aggregate_stats_* binaries).

| T10 | TODO    | Add application tests            | In `tests/servers/api/contract/stats/mod.rs`, enable deferred UDP fixed-port coverage and add enabled/disabled HTTP/UDP repeated-port-zero coverage; each expects aggregate count `1`. |
| T11 | TODO    | Validate and document            | Run focused tests, `linter all`, and the manual protocol; update evidence and architecture docs.                                                                                       |

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:160

• The verification plan references the removed tests/servers/api/contract/stats/mod.rs and the removed cargo test --test stats command. Update these to the current integration-test binaries added in this PR.

- Focused regression tests for `AppContainer` and startup jobs after prerequisites land.
- `tests/servers/api/contract/stats/mod.rs`: enabled/disabled HTTP listeners on distinct fixed
  ports must produce aggregate `tcp4_announces_handled == 1`.
- Defer the equivalent UDP fixed-port and repeated-port-zero HTTP/UDP aggregate-statistics tests
  to #2039; they assert listener-side metrics filtering rather than bootstrap configuration selection.
- `cargo test --test stats --test scaffold`.
- `linter all`.

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:108

• T5 is marked DONE and points to tests/servers/api/contract/stats/mod.rs, but that file was removed and there is currently no enabled/disabled HTTP aggregate-statistics regression under tests/ (no tcp4_announces_handled == 1 assertion). Either add the intended regression in the new test structure or mark this task as TODO.

| T5  | DONE    | Add HTTP statistics integration coverage                                                               | In `tests/servers/api/contract/stats/mod.rs`, add a fixed-port enabled/disabled HTTP test that expects aggregate count `1`.                                      |
| T6  | DONE    | Add bootstrap regressions                                                                              | Cover duplicate port-zero HTTP/UDP configuration-to-container correspondence without asserting aggregate metrics policy.                                         |

docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md:20

• The file currently has a duplicated header and Purpose section glued onto the end of the new paragraph ("…baseline evidence.# Event Metrics…"), which breaks Markdown formatting and repeats content.

Add the exact tracker configuration, commands, observed REST statistics, and
post-change comparison for each probe here. Do not overwrite baseline evidence.# Event Metrics Normalization Evidence

docs/issues/open/2039-normalize-per-instance-event-metrics-policy/ISSUE.md:23

• This issue spec references tests/servers/api/contract/stats/mod.rs, but that file was removed in this PR. The related-artifacts list should point at the new integration test binaries instead.

This issue also appears in the following locations of the same file:

• line 106
• line 144

    - evidence.md
    - tests/servers/api/contract/stats/mod.rs
    - packages/events/src/bus.rs

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:24

• This issue spec lists tests/servers/api/contract/stats/mod.rs as a related artifact, but that path no longer exists after the test restructuring in this PR.

This issue also appears in the following locations of the same file:

• line 107
• line 154

    - evidence.md
    - tests/servers/api/contract/stats/mod.rs
  related-issues:

tests/aggregate_stats_port_zero.rs:119

• Parsing the UDP tracker address by slicing the URL string (url.as_str()[6..]) is brittle and will break if the URL formatting changes (e.g., different scheme length, future URL normalization). Prefer using Url's host/port accessors or socket_addrs.

    for url in &udp_urls {
        let addr: std::net::SocketAddr = url.as_str()[6..].parse().expect("UDP URL must parse to SocketAddr");
        common::udp_announce(addr, &info_hash, &peer_id, 17548).await;

tests/aggregate_stats_fixed_ports.rs:121

• Parsing the UDP tracker address by slicing the URL string (url.as_str()[6..]) is brittle and will break if the URL formatting changes. Prefer using Url's host/port accessors or socket_addrs.

    for url in &udp_urls {
        let addr: std::net::SocketAddr = url.as_str()[6..].parse().expect("UDP URL must parse to SocketAddr");
        common::udp_announce(addr, &info_hash, &peer_id, 17548).await;

tests/aggregate_stats_port_zero.rs:58

• These aggregate-stats tests exercise only the “all listeners enabled” case, which would have passed even before the duplicate-port-zero bootstrap fix (both instances still start; the bug was per-instance configuration selection). To regress the actual bug, add an application-level test where two HTTP tracker blocks share the same bind address but have different tracker_usage_statistics values and assert that only the enabled listener contributes to the aggregate count (expected tcp4_announces_handled == 1).

/// Configuration: two HTTP listeners on port zero, both enabled.
const PORT_ZERO_CONFIG: &str = r#"
    [metadata]
    app = "torrust-tracker"
    purpose = "configuration"
    schema_version = "2.0.0"

    [logging]
    threshold = "off"

    [core]
    listed = false
    private = false

    [core.database]
    driver = "sqlite3"
    path = "{STORAGE_PATH}/sqlite3.db"

    [[http_trackers]]
    bind_address = "0.0.0.0:0"
    tracker_usage_statistics = true

    [[http_trackers]]
    bind_address = "0.0.0.0:0"
    tracker_usage_statistics = true

    [[udp_trackers]]
    bind_address = "0.0.0.0:0"
    tracker_usage_statistics = true

    [[udp_trackers]]
    bind_address = "0.0.0.0:0"
    tracker_usage_statistics = true

[Copilot]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (6)

src/container.rs:22

• http_tracker_container/udp_tracker_container take both index and bind_address, but bind_address is only used for error formatting. This makes the API easy to misuse (caller can pass a mismatched address) and forces call sites to provide redundant data.

#[derive(thiserror::Error, Debug, Clone)]
pub enum Error {
    #[error("There is no HTTP tracker container at index {index} (bind_address: {bind_address})")]
    MissingHttpTrackerCoreContainer { index: usize, bind_address: SocketAddr },

    #[error("There is no UDP tracker container at index {index} (bind_address: {bind_address})")]
    MissingUdpTrackerCoreContainer { index: usize, bind_address: SocketAddr },

docs/issues/open/2039-normalize-per-instance-event-metrics-policy/evidence.md:26

• This file now contains two separate "## Purpose" sections; the second one (lines 21–26) duplicates the heading and makes the document harder to scan.

## Purpose

This file records the progressive manual baseline and post-change probes
required by the draft specification. Each code-changing task must have one
entry before its change and one entry after it.

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:109

• The implementation-plan row for T5 still references tests/servers/api/contract/stats/mod.rs, but that test file is removed in this PR. Also, no integration test in this PR covers the fixed-port enabled/disabled HTTP aggregate-statistics case mentioned here (grep shows no tracker_usage_statistics = false in tests/).

| T5  | DONE    | Add HTTP statistics integration coverage                                                               | In `tests/servers/api/contract/stats/mod.rs`, add a fixed-port enabled/disabled HTTP test that expects aggregate count `1`.                                      |
| T6  | DONE    | Add bootstrap regressions                                                                              | Cover duplicate port-zero HTTP/UDP configuration-to-container correspondence without asserting aggregate metrics policy.                                         |
| T7  | TODO    | Run and record local tracker probes                                                                    | Run fixed-port HTTP and duplicate-port-zero bootstrap scenarios locally; append configuration, commands, outputs, and comparisons to [evidence.md](evidence.md). |

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:160

• The verification-plan command still references the removed stats integration test binary (cargo test --test stats --test scaffold). This will no longer work after tests/stats.rs is deleted.

- `tests/aggregate_stats_fixed_ports.rs`: enabled/disabled HTTP listeners on distinct fixed
  ports must produce aggregate `tcp4_announces_handled == 1`.
- Defer the equivalent UDP fixed-port and repeated-port-zero HTTP/UDP aggregate-statistics tests
  to #2039; they assert listener-side metrics filtering rather than bootstrap configuration selection.
- `cargo test --test stats --test scaffold`.
- `linter all`.

tests/aggregate_stats_fixed_ports.rs:69

• This fixed-port integration-test binary only covers the all-enabled case (asserts aggregate count 2). Given the PR goal (duplicate port-zero configs keep their own per-listener config), it should also add the key regression: one HTTP listener with tracker_usage_statistics = false and one with true must yield an aggregate count of 1 (as documented in the #2035 spec).

/// Configuration: two HTTP listeners on distinct fixed ports,
/// all with `tracker_usage_statistics = true`.
const FIXED_PORT_CONFIG: &str = r#"
    [metadata]
    app = "torrust-tracker"
    purpose = "configuration"
    schema_version = "2.0.0"

    [logging]
    threshold = "off"

    [core]
    listed = false
    private = false

    [core.database]
    driver = "sqlite3"
    path = "{STORAGE_PATH}/sqlite3.db"

    [[http_trackers]]
    bind_address = "0.0.0.0:17091"
    tracker_usage_statistics = true

    [[http_trackers]]
    bind_address = "0.0.0.0:17092"
    tracker_usage_statistics = true

    [[udp_trackers]]
    bind_address = "0.0.0.0:17093"
    tracker_usage_statistics = true

    [[udp_trackers]]
    bind_address = "0.0.0.0:17094"
    tracker_usage_statistics = true

    [http_api]
    bind_address = "127.0.0.1:0"

    [http_api.access_tokens]
    admin = "MyAccessToken"

    [health_check_api]
    bind_address = "127.0.0.2:0"
"#;

tests/common/workspace.rs:85

• start_tracker_with_config returns as soon as any service registers (!map.is_empty()), but callers immediately assert on the full set of expected listeners (e.g., 2 HTTP + 2 UDP + APIs). This can race and make the integration tests flaky on slow machines.

    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
    loop {
        let entries = container.registar.entries();
        let map = entries.lock().await;
        if !map.is_empty() {
            break;

[Copilot]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (6)

src/container.rs:152

• Same issue as the HTTP variant: bind_address is not used for the lookup (only index), so it can drift from reality and produce confusing diagnostics. Prefer removing it from the signature/error or switching to an ID-based lookup that doesn't accept unrelated parameters.

    /// Return an error if there is no UDP tracker container at the given
    /// configuration index.
    pub fn udp_tracker_container(&self, index: usize, bind_address: SocketAddr) -> Result<Arc<UdpTrackerCoreContainer>, Error> {
        self.udp_tracker_instance_containers.get(index).map_or_else(
            || Err(Error::MissingUdpTrackerCoreContainer { index, bind_address }),
            |(_id, container)| Ok(container.clone()),
        )

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:160

• The verification plan still recommends cargo test --test stats ..., but tests/stats.rs is removed in this PR. Update this command to run the new integration-test binaries instead, otherwise the docs/spec will point to a non-existent test target.

- `tests/aggregate_stats_fixed_ports.rs`: enabled/disabled HTTP listeners on distinct fixed
  ports must produce aggregate `tcp4_announces_handled == 1`.
- Defer the equivalent UDP fixed-port and repeated-port-zero HTTP/UDP aggregate-statistics tests
  to #2039; they assert listener-side metrics filtering rather than bootstrap configuration selection.
- `cargo test --test stats --test scaffold`.
- `linter all`.

tests/common/workspace.rs:85

• start_tracker_with_config stops waiting as soon as any service registers (!map.is_empty()), but the integration tests immediately assert that multiple HTTP/UDP trackers + the REST API are discoverable. If the health-check (or any single service) registers first, this can exit early and make the suite flaky.

Consider waiting until the expected services are discoverable (based on the config) instead of only checking for a non-empty registar.

    let (container, jobs) = app::run().await;
    // Wait for services to register in the registar and bind to ports.
    // Polls the registar instead of using a fixed sleep to avoid
    // flakiness on slow machines and unnecessary delay on fast ones.
    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
    loop {
        let entries = container.registar.entries();
        let map = entries.lock().await;
        if !map.is_empty() {
            break;

src/container.rs:140

• bind_address is accepted by this API but not used for lookup (lookup is purely by index). This can lead to misleading error messages if a caller passes the wrong address, and it makes the API harder to use correctly.

Suggestion: remove the bind_address parameter entirely (and drop it from the error), or change the lookup to use ConfigurationInstanceId so the key and error payload are always consistent.

This issue also appears on line 146 of the same file.

    /// Return an error if there is no HTTP tracker container at the given
    /// configuration index.
    pub fn http_tracker_container(&self, index: usize, bind_address: SocketAddr) -> Result<Arc<HttpTrackerCoreContainer>, Error> {
        self.http_tracker_instance_containers.get(index).map_or_else(
            || Err(Error::MissingHttpTrackerCoreContainer { index, bind_address }),
            |(_id, container)| Ok(container.clone()),

tests/aggregate_stats_port_zero.rs:57

• This test binary is meant to prove that duplicate 0.0.0.0:0 configuration entries no longer overwrite each other, but the duplicated [[http_trackers]] and [[udp_trackers]] blocks are identical (tracker_usage_statistics = true for all). With identical blocks, the suite would still pass even if only the last container/config were used.

To make this a regression for #2035, consider making the duplicated blocks differ (e.g., set tracker_usage_statistics differently or vary another config field) and assert that app_container.http_tracker_instance_containers[idx].1.*_tracker_config reflects each block’s distinct value(s).

    [[http_trackers]]
    bind_address = "0.0.0.0:0"
    tracker_usage_statistics = true

    [[http_trackers]]
    bind_address = "0.0.0.0:0"
    tracker_usage_statistics = true

    [[udp_trackers]]
    bind_address = "0.0.0.0:0"
    tracker_usage_statistics = true

    [[udp_trackers]]
    bind_address = "0.0.0.0:0"
    tracker_usage_statistics = true

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:111

• This issue spec still references the removed stats-contract test location (tests/servers/api/contract/stats/mod.rs) and still suggests running cargo test --test stats ..., but tests/stats.rs is deleted in this PR.

Please update the references/commands to the new integration-test binaries (e.g. aggregate_stats_port_zero / aggregate_stats_fixed_ports), and ensure the spec’s stated expectations match what the new tests actually assert.

This issue also appears on line 155 of the same file.

| T5  | DONE    | Add HTTP statistics integration coverage                                                               | In `tests/servers/api/contract/stats/mod.rs`, add a fixed-port enabled/disabled HTTP test that expects aggregate count `1`.                                      |
| T6  | DONE    | Add bootstrap regressions                                                                              | Cover duplicate port-zero HTTP/UDP configuration-to-container correspondence without asserting aggregate metrics policy.                                         |
| T7  | TODO    | Run and record local tracker probes                                                                    | Run fixed-port HTTP and duplicate-port-zero bootstrap scenarios locally; append configuration, commands, outputs, and comparisons to [evidence.md](evidence.md). |
| T8  | BLOCKED | Land registry metadata migration                                                                       | #2041 must expose started-service canonical identity before final verification.                                                                                  |

[Copilot]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:158

• The automatic verification plan currently states that enabled/disabled HTTP listeners on fixed ports must yield tcp4_announces_handled == 1, but tests/aggregate_stats_fixed_ports.rs documents that this case is blocked by #2039 and does not assert it yet. To avoid misleading future readers, update this plan to describe the current enabled/enabled coverage and explicitly defer enabled/disabled policy assertions to #2039.

- `tests/aggregate_stats_fixed_ports.rs`: enabled/disabled HTTP listeners on distinct fixed
  ports must produce aggregate `tcp4_announces_handled == 1`.
- Defer the equivalent UDP fixed-port and repeated-port-zero HTTP/UDP aggregate-statistics tests
  to #2039; they assert listener-side metrics filtering rather than bootstrap configuration selection.

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:175

• This manual verification scenario expects the fixed-port enabled/disabled case to yield aggregate count 1, but the current test suite notes that enabled/disabled filtering is blocked by #2039. Either mark this scenario as deferred/blocked, or restate it as the enabled/enabled probe that #2035 can validate today (count 2).

| M3  | Run fixed-port enabled/disabled HTTP listeners locally.                             | The aggregate HTTP announce count is `1`.                                | TODO   | [evidence.md](evidence.md) |

tests/common/workspace.rs:85

• The startup wait loop breaks once the registar has 2 entries, but most configurations started by these integration tests register more than 2 services (e.g., HTTP API + health check + multiple HTTP/UDP listeners). This can let the function return before all tracker listeners are registered/bound, making http_tracker_urls/udp_tracker_urls intermittently return fewer URLs than expected.

    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
    loop {
        let entries = container.registar.entries();
        let map = entries.lock().await;
        if map.len() >= 2 {

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:107

• This task description says aggregate_stats_fixed_ports.rs provides an enabled/disabled HTTP aggregate-statistics case with expected count 1, but the test file explicitly notes that enabled/disabled filtering is blocked by #2039 and currently asserts the enabled/enabled case (count 2). The issue spec should reflect what is actually covered now vs what is deferred to #2039.

This issue also appears in the following locations of the same file:

• line 155
• line 175

| T5  | DONE    | Add HTTP statistics integration coverage                                                               | In `tests/aggregate_stats_fixed_ports.rs`, added fixed-port HTTP test. Aggregate count `1` blocked by #2039 (shared HTTP event bus).                             |

src/container.rs:21

• The new error messages say "container at index" without clarifying that this is the configuration-instance index (matching the order in the config). Since the public methods and docs refer to a "configuration index", the error text should match to avoid confusion when debugging.

    #[error("There is no HTTP tracker container at index {index}")]
    MissingHttpTrackerCoreContainer { index: usize },

    #[error("There is no UDP tracker container at index {index}")]
    MissingUdpTrackerCoreContainer { index: usize },

[Copilot]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

tests/common/workspace.rs:82

• The map.len() >= MIN_REGISTERED_SERVICES readiness gate can pass before the specific services the scenarios need are registered (e.g., if HTTP API + health check register first), which can make subsequent URL discovery/assertions flaky. Consider waiting on concrete predicates (e.g., http_api_url(container).await.is_some() plus minimum HTTP/UDP tracker counts), or make this helper accept expected counts/predicates per test binary.

    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
    loop {
        let entries = container.registar.entries();
        let map = entries.lock().await;
        if map.len() >= MIN_REGISTERED_SERVICES {

tests/aggregate_stats_port_zero.rs:47

• This suite is meant to guard against the duplicate 0.0.0.0:0 bootstrap collision, but both repeated HTTP/UDP blocks are identical (tracker_usage_statistics = true), so the test would still pass even if both instances accidentally shared the same configuration/container. Add a regression where the repeated blocks differ (any field) and assert the per-index AppContainer instance containers retain those distinct configs (e.g., inspect app_container.http_tracker_instance_containers[i].1.http_tracker_config and the UDP equivalent), without relying on aggregate-statistics filtering (#2039).

    [[http_trackers]]
    bind_address = "0.0.0.0:0"
    tracker_usage_statistics = true

    [[http_trackers]]

docs/issues/open/2035-fix-duplicate-port-zero-tracker-instance-bootstrap/ISSUE.md:158

• These Automatic Checks bullets describe an enabled/disabled fixed-port HTTP aggregate-statistics expectation (count == 1), but tests/aggregate_stats_fixed_ports.rs in this PR only covers the all-enabled case and asserts count == 2. This doc should match the current test suite, and keep enabled/disabled (count == 1) coverage explicitly deferred to #2039.

- Focused regression tests for `AppContainer` and startup jobs after prerequisites land.
- `tests/aggregate_stats_fixed_ports.rs`: enabled/disabled HTTP listeners on distinct fixed
  ports must produce aggregate `tcp4_announces_handled == 1`.
- Defer the equivalent UDP fixed-port and repeated-port-zero HTTP/UDP aggregate-statistics tests
  to #2039; they assert listener-side metrics filtering rather than bootstrap configuration selection.

[Copilot]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

tests/common/workspace.rs:93

• The startup wait only checks a minimum number of registered services (currently map.len() >= MIN_REGISTERED_SERVICES), which can be satisfied by non-tracker services (e.g., HTTP API + health check). Callers that immediately query http_tracker_urls/udp_tracker_urls can then observe zero/missing trackers, making these integration tests potentially flaky.

    loop {
        let entries = container.registar.entries();
        let map = entries.lock().await;
        if map.len() >= MIN_REGISTERED_SERVICES {
            break;

tests/common/mod.rs:24

• This comment says the compiler emits dead_code warnings for unused re-exports, but the suppression used below is #[allow(unused_imports)]. The warning being suppressed here is unused_imports; the dead_code suppression is handled in the individual submodules.

// Each integration-test binary compiles this module independently. Not all
// binaries call every re-exported function, so the compiler emits dead_code
// warnings for the binaries that don't. The attributes suppress those
// per-binary false positives without hiding genuine dead code.

tests/common/mod.rs:16

• The referenced decision record path is outdated: docs/issues/open/1419-allow-multiple-integration-tests-at-main-app-level.md doesn't exist; the current spec lives at docs/issues/open/1419-allow-multiple-integration-tests-at-main-app-level/ISSUE.md. This breaks the link for contributors reading these docs.

//! See `docs/issues/open/1419-allow-multiple-integration-tests-at-main-app-level.md`
//! for the full decision record.
mod announce;

[Copilot]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tests/common/workspace.rs:93

• The readiness gate in start_tracker_with_config can break too early because it only checks map.len() >= MIN_REGISTERED_SERVICES. With typical configs, the HTTP API + health check can satisfy this before HTTP/UDP tracker listeners have registered, making http_tracker_urls / udp_tracker_urls intermittently return fewer bindings and causing flaky asserts in the integration binaries. Consider gating on the expected number of tracker bindings (derived from container.http_tracker_instance_containers.len() / container.udp_tracker_instance_containers.len()) rather than total registry entries.

    loop {
        let entries = container.registar.entries();
        let map = entries.lock().await;
        if map.len() >= MIN_REGISTERED_SERVICES {
            break;

[josecelano]

ACK 8151e92