Skip to content

test: use a shared 30-second events stream hang guard - #699

Merged
thomasluizon merged 1 commit into
mainfrom
fix/ticket-1208-events-stream-hang-guard
Oct 4, 2026
Merged

thomasluizon merged 1 commit into
mainfrom
fix/ticket-1208-events-stream-hang-guard

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

Use one shared 30-second hang guard for all three stream completion waits and the text polling helper in tests/Orbit.Infrastructure.Tests/Events/EventsControllerTests.cs. The bounds guard against hangs rather than define a performance budget, so the larger allowance tolerates delayed scheduling while retaining a finite failure bound.

All behavioral assertions and token expiry inputs are unchanged. Only the named test file changes; production code and contracts are untouched.

Fixes thomasluizon/orbit-tickets#1208.

Test evidence

The unchanged class passed before the fix. The reported timeout at original line 135 was not reproduced in 61 loaded runs of the class, including 20 full infrastructure runs and one full-solution run. No pre-fix failure or red-to-green result is claimed. The acceptance requirement to reuse a load that first made line 135 fail remains unverified.

The observed failure that filed this ticket (orchestrator note): a full env -u LANG dotnet test Orbit.slnx --no-build run on the fix/ticket-1204-validation-fix-copy tree, at a one-minute load of about 8 while another worker ran, failed EventsControllerTests.Stream_SendsPublishedChangesOnce with System.TimeoutException : The operation has timed out. at line 135 (await stream.WaitAsync(TimeSpan.FromSeconds(2))), and the same build then passed the class 12 of 12 in five consecutive runs alone. That single red is the evidence for the change; it widens only hang guards and keeps every assertion.

No behavioral test was strengthened: this ticket changes the hang guards in the existing tests and requires every behavioral assertion to stay unchanged. The existing published-change, replay, readiness, shutdown, revocation and expiry checks were exercised in every class run.

The machine has 18 logical CPUs. Each load harness ran busy loops in the stated number of separate processes and at most four concurrent test commands, with LANG removed from the test environment. The separate full-solution run added test processes during its reproduction attempt. Each focused class run executed all 12 cases.

Command selection and added load Before change After change
Focused class, no added CPU workers 1/1 runs passed 1/1 runs passed
Focused class, 144 CPU workers, four parallel runs 20/20 runs passed 20/20 runs passed
Focused class, 576 CPU workers, four parallel runs 20/20 runs passed 20/20 runs passed
Full infrastructure project, 144 CPU workers, four parallel runs 20/20 runs passed, 2,629 tests per run Not repeated
Full solution 1/1 passed under 144 CPU workers and concurrent infrastructure runs; 7,116 tests 1/1 passed with no added CPU workers; 7,116 tests

The full-solution reproduction command ran during the final portion of the loaded infrastructure batch, adding contention to that portion.

Commands:

env -u LANG dotnet build Orbit.slnx
env -u LANG dotnet test tests/Orbit.Infrastructure.Tests/Orbit.Infrastructure.Tests.csproj --no-build --filter FullyQualifiedName~EventsControllerTests
python3 ticket-1208-load.py --phase before --burners 144 --parallel 4 --runs 20
python3 ticket-1208-load.py --phase before-heavy --burners 576 --parallel 4 --runs 20
python3 ticket-1208-load.py --phase before-project --burners 144 --parallel 4 --runs 20 --full-project
env -u LANG dotnet test Orbit.slnx --no-build
python3 ticket-1208-load.py --phase after --burners 144 --parallel 4 --runs 20
python3 ticket-1208-load.py --phase after-heavy --burners 576 --parallel 4 --runs 20

The solution was rebuilt after the change with zero errors. The focused class passed before committing; all pre-commit hooks passed. Full-suite result after the commit: all 7,116 tests passed, none failed or skipped. The working tree is clean.

Temporary load harness used outside the repository

Save as ticket-1208-load.py and run the commands above. Test output is saved separately for each run. The timeout marker is diagnostic only; pass counts come from the test command exit codes and were checked against the test summaries.

import argparse
import concurrent.futures
import multiprocessing
import os
from pathlib import Path
import subprocess
import time


def burn():
    while True:
        pass


def main():
    parser = argparse.ArgumentParser()
    parser.add_argument('--phase', required=True)
    parser.add_argument('--burners', type=int, default=144)
    parser.add_argument('--parallel', type=int, default=4)
    parser.add_argument('--runs', type=int, default=20)
    parser.add_argument('--full-project', action='store_true')
    args = parser.parse_args()
    root = Path('ticket-1208-' + args.phase)
    root.mkdir(exist_ok=True)
    env = dict(os.environ)
    env.pop('LANG', None)
    cmd = ['dotnet', 'test', 'tests/Orbit.Infrastructure.Tests/Orbit.Infrastructure.Tests.csproj', '--no-build', '--filter', 'FullyQualifiedName~EventsControllerTests']
    if args.full_project:
        cmd = cmd[:-2]
    workers = [multiprocessing.get_context('fork').Process(target=burn) for _ in range(args.burners)]
    def run(index):
        started = time.monotonic()
        with (root / f'run-{index:02d}.log').open('w') as log:
            result = subprocess.run(cmd, env=env, stdout=log, stderr=subprocess.STDOUT, timeout=600)
        output = (root / f'run-{index:02d}.log').read_text()
        line135 = 'line 135' in output and 'TimeoutException' in output
        print(f'run={index} exit={result.returncode} line135_timeout={line135} seconds={time.monotonic() - started:.1f}', flush=True)
        return result.returncode, line135
    try:
        for worker in workers:
            worker.start()
        time.sleep(3)
        print(f'phase={args.phase} burners={args.burners} parallel={args.parallel} runs={args.runs} cmd={cmd}', flush=True)
        with concurrent.futures.ThreadPoolExecutor(max_workers=args.parallel) as pool:
            results = list(pool.map(run, range(1, args.runs + 1)))
        print(f'passed={sum(code == 0 for code, _ in results)} failed={sum(code != 0 for code, _ in results)} line135_timeouts={sum(timed_out for _, timed_out in results)}', flush=True)
    finally:
        for worker in workers:
            if worker.pid is not None:
                worker.terminate()
        for worker in workers:
            if worker.pid is not None:
                worker.join()


if __name__ == '__main__':
    main()

@pullfrog pullfrog Bot 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.

✅ No new issues found.

Reviewed changes Reviewed the timeout-only change in EventsControllerTests.cs and the corresponding stream cancellation behavior.

  • Shared hang guard: All three stream completion waits and the text polling helper now use one 30-second timeout, retaining finite failure bounds while allowing delayed scheduling.
  • Preserved coverage: Behavioral assertions and token-expiry inputs are unchanged. All 12 affected test cases passed locally. The original timeout remains unreproduced, as explicitly documented in the PR; the load harness and full suite were not repeated during this review.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

@sonarqubecloud

sonarqubecloud Bot commented Oct 4, 2026

Copy link
Copy Markdown

@thomasluizon
thomasluizon merged commit 448a582 into main Oct 4, 2026
30 checks passed
@thomasluizon
thomasluizon deleted the fix/ticket-1208-events-stream-hang-guard branch October 4, 2026 20:55
thomasluizon added a commit that referenced this pull request Oct 4, 2026
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.

1 participant