test(engine): extract ShouldAutoRebuild as pure fn + cover 6 cases
Some checks failed
CI / build-and-test (push) Failing after 27s
Some checks failed
CI / build-and-test (push) Failing after 27s
The self-heal trigger from c30a616 was time-based logic embedded in the RunAsync poll loop — easy to regress on a future refactor without anyone noticing. Pull it out into a public static ShouldAutoRebuild(sinceStart, sinceLastSeen, sinceLastRebuild) that returns the rebuild reason or null. RunAsync just calls it and acts on the result.
Six new test cases cover the matrix:
- never seen + before warmup -> hold
- never seen + after warmup -> rebuild
- never seen + recent rebuild -> backoff
- had sources + long-gone -> rebuild
- had sources + recently gone -> grace window
- had sources + recent rebuild -> backoff
112/112 Engine tests passing (was 106; +6 new).
This commit is contained in:
parent
aaa2a76814
commit
ea940ffac4
2 changed files with 115 additions and 25 deletions
|
|
@ -127,36 +127,19 @@ public sealed class NdiDiscoveryService
|
|||
RebuildFinder("operator request");
|
||||
lastRebuildAt = now;
|
||||
}
|
||||
// Auto-healing rebuilds. Only one path can fire per tick.
|
||||
// Auto-healing rebuilds — see ShouldAutoRebuild.
|
||||
else if (_previous.Count == 0)
|
||||
{
|
||||
var sinceStart = now - startedAt;
|
||||
var sinceRebuild = now - lastRebuildAt;
|
||||
if (lastSeenAt is null)
|
||||
var decision = ShouldAutoRebuild(
|
||||
sinceStart: now - startedAt,
|
||||
sinceLastSeen: lastSeenAt is { } seen ? now - seen : (TimeSpan?)null,
|
||||
sinceLastRebuild: now - lastRebuildAt);
|
||||
if (decision is { } reason)
|
||||
{
|
||||
// Never seen a source. After 5s of empty results, try a rebuild
|
||||
// every 5s. This handles the cold-start case where the initial
|
||||
// finder didn't bind to the right network interface.
|
||||
if (sinceStart > TimeSpan.FromSeconds(5) && sinceRebuild > TimeSpan.FromSeconds(5))
|
||||
{
|
||||
RebuildFinder("auto-heal: never saw a source");
|
||||
RebuildFinder(reason);
|
||||
lastRebuildAt = now;
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
// We've seen sources before but currently see nothing. After
|
||||
// 15s of empty results, try a rebuild every 10s. This handles
|
||||
// the "Teams stopped broadcasting then started again but our
|
||||
// finder didn't pick up the new advertisements" case.
|
||||
var sinceLastSeen = now - lastSeenAt.Value;
|
||||
if (sinceLastSeen > TimeSpan.FromSeconds(15) && sinceRebuild > TimeSpan.FromSeconds(10))
|
||||
{
|
||||
RebuildFinder("auto-heal: source set went empty 15s ago");
|
||||
lastRebuildAt = now;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
try { PollOnce(); }
|
||||
catch (Exception ex) { _logger.LogWarning(ex, "Discovery poll failed; will retry on next tick."); }
|
||||
|
|
@ -170,6 +153,39 @@ public sealed class NdiDiscoveryService
|
|||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Pure-function decision for whether the discovery loop should rebuild the
|
||||
/// NDI finder on the current tick. Returns a non-null reason string when
|
||||
/// the rebuild should fire (which is also logged); null means "leave the
|
||||
/// finder alone." Caller is responsible for tracking the timestamps and
|
||||
/// updating <c>lastRebuildAt</c> after the rebuild.
|
||||
///
|
||||
/// Public + static for unit-testability — the time-based rules are easy to
|
||||
/// regress and hard to spot in integration testing.
|
||||
///
|
||||
/// Rules:
|
||||
/// <list type="number">
|
||||
/// <item><b>Never seen a source</b> (<paramref name="sinceLastSeen"/> is null):
|
||||
/// rebuild when sinceStart > 5s AND sinceLastRebuild > 5s.</item>
|
||||
/// <item><b>Used to see sources, now empty</b>: rebuild when sinceLastSeen
|
||||
/// > 15s AND sinceLastRebuild > 10s.</item>
|
||||
/// </list>
|
||||
/// Both rules back off the rebuild cadence to avoid churn during legitimate
|
||||
/// empty periods (no meeting active, all participants left, etc.).
|
||||
/// </summary>
|
||||
public static string? ShouldAutoRebuild(TimeSpan sinceStart, TimeSpan? sinceLastSeen, TimeSpan sinceLastRebuild)
|
||||
{
|
||||
if (sinceLastSeen is null)
|
||||
{
|
||||
if (sinceStart > TimeSpan.FromSeconds(5) && sinceLastRebuild > TimeSpan.FromSeconds(5))
|
||||
return "auto-heal: never saw a source";
|
||||
return null;
|
||||
}
|
||||
if (sinceLastSeen.Value > TimeSpan.FromSeconds(15) && sinceLastRebuild > TimeSpan.FromSeconds(10))
|
||||
return "auto-heal: source set went empty 15s ago";
|
||||
return null;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Dispose the current finder and create a fresh one against the cached
|
||||
/// discovery groups. Clears the seen-set so all currently-visible sources
|
||||
|
|
|
|||
|
|
@ -66,4 +66,78 @@ public class NdiDiscoveryServiceTests
|
|||
while (reader.TryRead(out var ev)) list.Add(ev);
|
||||
return list;
|
||||
}
|
||||
|
||||
// ============================================================
|
||||
// ShouldAutoRebuild — pure function gating the auto-heal path
|
||||
// ============================================================
|
||||
//
|
||||
// Two rules under test:
|
||||
// (a) Never seen a source AND sinceStart>5s AND sinceLastRebuild>5s -> rebuild
|
||||
// (b) Used to see sources, now empty AND sinceLastSeen>15s AND sinceLastRebuild>10s -> rebuild
|
||||
//
|
||||
// Both rules back off to avoid churn during legitimate empty periods.
|
||||
|
||||
[Fact]
|
||||
public void ShouldAutoRebuild_NeverSeenSource_BeforeWarmup_ReturnsNull()
|
||||
{
|
||||
// 3s after startup is well inside the "give cold start a chance" window.
|
||||
NdiDiscoveryService.ShouldAutoRebuild(
|
||||
sinceStart: TimeSpan.FromSeconds(3),
|
||||
sinceLastSeen: null,
|
||||
sinceLastRebuild: TimeSpan.FromSeconds(99))
|
||||
.Should().BeNull();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ShouldAutoRebuild_NeverSeenSource_AfterWarmup_TriggersRebuild()
|
||||
{
|
||||
NdiDiscoveryService.ShouldAutoRebuild(
|
||||
sinceStart: TimeSpan.FromSeconds(6),
|
||||
sinceLastSeen: null,
|
||||
sinceLastRebuild: TimeSpan.FromSeconds(6))
|
||||
.Should().Contain("never saw a source");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ShouldAutoRebuild_NeverSeenSource_RecentRebuild_HoldsOff()
|
||||
{
|
||||
// sinceStart qualifies, but the last rebuild was 2s ago — back off.
|
||||
NdiDiscoveryService.ShouldAutoRebuild(
|
||||
sinceStart: TimeSpan.FromSeconds(20),
|
||||
sinceLastSeen: null,
|
||||
sinceLastRebuild: TimeSpan.FromSeconds(2))
|
||||
.Should().BeNull();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ShouldAutoRebuild_HadSources_NowEmpty_LongAgo_TriggersRebuild()
|
||||
{
|
||||
NdiDiscoveryService.ShouldAutoRebuild(
|
||||
sinceStart: TimeSpan.FromMinutes(5),
|
||||
sinceLastSeen: TimeSpan.FromSeconds(20),
|
||||
sinceLastRebuild: TimeSpan.FromSeconds(30))
|
||||
.Should().Contain("source set went empty");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ShouldAutoRebuild_HadSources_NowEmpty_Recently_HoldsOff()
|
||||
{
|
||||
// 10s since last source seen — still inside the 15s grace window.
|
||||
NdiDiscoveryService.ShouldAutoRebuild(
|
||||
sinceStart: TimeSpan.FromMinutes(5),
|
||||
sinceLastSeen: TimeSpan.FromSeconds(10),
|
||||
sinceLastRebuild: TimeSpan.FromSeconds(30))
|
||||
.Should().BeNull();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ShouldAutoRebuild_HadSources_NowEmpty_RecentRebuild_HoldsOff()
|
||||
{
|
||||
// Grace window expired, but we just rebuilt 8s ago — back off.
|
||||
NdiDiscoveryService.ShouldAutoRebuild(
|
||||
sinceStart: TimeSpan.FromMinutes(5),
|
||||
sinceLastSeen: TimeSpan.FromSeconds(30),
|
||||
sinceLastRebuild: TimeSpan.FromSeconds(8))
|
||||
.Should().BeNull();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue