Skip to content

test: re-seed test_get_retry_with_alternatives_sparse_topology for current stdlib#4451

Merged
sanity merged 1 commit into
mainfrom
fix-3506-reseed
Jun 16, 2026
Merged

test: re-seed test_get_retry_with_alternatives_sparse_topology for current stdlib#4451
sanity merged 1 commit into
mainfrom
fix-3506-reseed

Conversation

@sanity

@sanity sanity commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Problem

test_get_retry_with_alternatives_sparse_topology (in
crates/core/tests/simulation_integration.rs) has been #[ignore]d since
PR #3488. Its hardcoded seed (0xBEEF_CAFE_0001) no longer produces the
intended sparse-but-reachable topology under the current freenet-stdlib:
the stdlib's enum layout changes bincode serialization sizes, which perturbs
the seeded RNG stream and shifts topology formation. Under the current stdlib
that seed leaves one GET requester (node-6 today; the issue originally observed
node-10/node-3) unable to reach any contract-caching node within HTL=2, so the
test's all-nodes-reachable assertion fails. While ignored, the test provides
zero coverage of the GET retry-with-alternatives path in a sparse topology.

Solution

Re-seed the test for the current stdlib layout and remove #[ignore]. The new
seed 0x3506_000B_0001 was found by an empirical search over the test's own
topology parameters (10 nodes, HTL=2, max_connections=6) and verified to
satisfy both constraints the test depends on:

  • Sparse — a PUT-only run of this exact topology caches the contract at
    exactly 2 of the 10 nodes (nodes 4 and 5), matching the issue's "2-3
    caching nodes" premise. So 8 of the 10 GET requesters genuinely have to use
    the retry-with-alternatives / k_closest re-query path.
  • Reachable + deterministic — all 10 GET requesters reach a caching node;
    confirmed 10/10 across repeated runs. Live logs show the expected
    get: attempt timed out; advancing retry events firing for the distant
    requesters.

The GET retry logic itself is unchanged. This is purely a seed/topology repair
in a single test function; no production code is touched.

Note: like every seed-coupled topology test, a future stdlib/topology change
could perturb the RNG stream again and require another re-seed. That is the
expected maintenance for these tests, not a regression in the retry logic
(which is also covered by test_get_routing_coverage_low_htl).

Testing

cargo nextest run -p freenet --features "simulation_tests,testing" \
  --test simulation_integration \
  -E 'test(test_get_retry_with_alternatives_sparse_topology)'

now passes (the test is no longer ignored). Sparseness was verified
out-of-band with a temporary PUT-only variant of the same network (PUT caches
at 2/10 nodes), and the full test was run multiple times with the chosen seed
to confirm 10/10 reachability every time. The temporary instrumentation used
for the seed search is not part of this diff.

Closes #3506

[AI-assisted - Claude]

…rrent stdlib (#3506)

## Problem

`test_get_retry_with_alternatives_sparse_topology` has been `#[ignore]`d
since PR #3488. Its hardcoded seed (`0xBEEF_CAFE_0001`) no longer produces
the intended sparse-but-reachable topology under the current freenet-stdlib:
the stdlib's enum layout changes bincode serialization sizes, which perturbs
the seeded RNG stream and shifts topology formation. Under the current stdlib
that seed leaves one GET requester (node-6 today, originally node-10/node-3)
unable to reach any contract-caching node within HTL=2, so the test's
all-nodes-reachable assertion fails. The ignored test provides zero coverage
of the GET retry-with-alternatives path in a sparse topology.

## Solution

Re-seed the test for the current stdlib layout and remove `#[ignore]`. The new
seed `0x3506_000B_0001` was found by an empirical search over the same
topology parameters (10 nodes, HTL=2, max_connections=6) and verified to
satisfy both constraints the test depends on:

- Sparse: a PUT-only run of this exact topology caches the contract at
  exactly 2 of the 10 nodes (nodes 4 and 5), matching the issue's "2-3
  caching nodes" premise.
- Reachable + deterministic: all 10 GET requesters reach a caching node, so
  the 8 non-cacher requesters exercise the retry-with-alternatives /
  k_closest re-query path. Confirmed 10/10 across repeated runs.

The GET retry logic itself is unchanged; this is purely a seed/topology
repair, so no production code is touched.

## Testing

`cargo nextest run -p freenet --features "simulation_tests,testing"
--test simulation_integration -E
'test(test_get_retry_with_alternatives_sparse_topology)'` now passes
deterministically (the test is no longer ignored). Sparseness was verified
out-of-band with a PUT-only variant of the same network (PUT caches at 2/10
nodes), and the full test was run multiple times with the chosen seed to
confirm 10/10 reachability every time.

Closes #3506

[AI-assisted - Claude]
@github-actions

Copy link
Copy Markdown
Contributor

Rule Review: No issues found

Rules checked: git-workflow.md, code-style.md, testing.md
Files reviewed: 1 (crates/core/tests/simulation_integration.rs)

No rule violations detected.

The change is a clean, well-scoped test maintenance PR: it removes an #[ignore] on a simulation test that had been disabled due to a stdlib-induced RNG stream shift, replaces the old seed with one that produces the intended sparse-but-reachable topology under the current stdlib, and documents the full rationale (seed history, what was verified, and maintenance expectations for future stdlib bumps) both in the doc comment and inline. The commit message follows the test: conventional-commit prefix and the single-logical-change rule is satisfied.


Rule review against .claude/rules/. WARNING findings block merge.

@sanity

sanity commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Multi-model review (consolidated)

Risk tier: routing/topology is normally a Full-tier surface, but this change is test-only (a single const SEED value swap plus removing #[ignore] and updating the doc comment). No production code is touched.

External model pass — Codex (codex review --base origin/main)

The patch only re-enables a previously ignored simulation regression test with an updated deterministic seed and explanatory comments. I did not identify any actionable correctness issue in the changed lines.

(Codex also attempted to run the test itself but was blocked by the offline sandbox's crates.io resolution, which is an environment limitation, not a finding.)

Claude lenses (run serially; this PR was produced by a dispatched subagent that cannot spawn sub-reviewers)

  • Code-first: The only behavioral change is SEED: 0xBEEF_CAFE_0001 → 0x3506_000B_0001 and removing #[ignore]. Assertions, topology parameters (10 nodes, HTL=2, max_connections=6, min_connections=3) and the test body are unchanged. Matches the issue's stated intent (re-seed and un-ignore).
  • Testing: The retry path is genuinely exercised — a PUT-only run of this exact topology caches the contract at 2 of 10 nodes, so 8 GET requesters must use retry-with-alternatives. The all-nodes-reachable assertion is unchanged (not weakened to chase a pass).
  • Skeptical: Simulation is deterministic given the seed (Turmoil + seeded GlobalRng). Confirmed multiple clean 10/10 passes. The only non-completions seen locally were timeouts under an extremely loaded dev box (load avg ~80 from other concurrent builds), i.e. infra contention, not test-logic flakiness; the test completes in ~17–50s in isolation, well within the 300s sim budget.
  • Big-picture: No CI-chasing (assertion preserved), no removed tests, no re-added #[ignore]. No bug-prevention-patterns.md pattern applies (no channels / spawns / enum changes).

Verdict

No blocking findings from either the external model or the Claude lenses. Proceeding to CI; will enable auto-merge once CI is green on the current HEAD.

[AI-assisted - Claude]

@sanity sanity enabled auto-merge June 16, 2026 15:50
@sanity sanity added this pull request to the merge queue Jun 16, 2026
Merged via the queue into main with commit 0b2e5ac Jun 16, 2026
15 checks passed
@sanity sanity deleted the fix-3506-reseed branch June 16, 2026 16:13
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.

fix: update test_get_retry_with_alternatives_sparse_topology seed for stdlib 0.2.2

1 participant