test(mesh): role-scoping scan handles braceless if (hasAdmin) — closes the phantom-block false pass

This commit is contained in:
Joseph Doherty
2026-08-06 10:33:15 -04:00
parent d7c9ead64e
commit 027f0403c4
@@ -16,7 +16,13 @@ namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Configuration;
/// Program.cs gates several unrelated concerns behind repeated <c>if (hasAdmin)</c> checks
/// (auth/AdminUI mounting, health probes, gRPC endpoint mapping, ...), so this pin cannot
/// assume there is only one such block in the file — it must find the SPECIFIC block that
/// contains each registration and prove that containment is unambiguous.
/// contains each registration and prove that containment is unambiguous. One of the six
/// markers (the ConfigDb gate) is a braceless single-statement <c>if</c>; the scanner handles
/// that shape explicitly rather than scanning forward for a brace that belongs to unrelated
/// code (see <see cref="FindHasAdminBlockRanges"/>). The scanner is also NOT string/comment
/// aware — a literal <c>{</c>/<c>}</c>/<c>;</c> inside a string or comment inside a scanned
/// span would misscan; it survives today only because no such span contains one, and the
/// implausible-length tripwire below is the cheap backstop for exactly this class of mistake.
/// </remarks>
[Trait("Category", "Unit")]
public sealed class MeshRegistrationRoleScopingTests
@@ -29,12 +35,18 @@ public sealed class MeshRegistrationRoleScopingTests
private const string HasAdminMarker = "if (hasAdmin)";
// The real hasAdmin blocks in Program.cs top out around 40 lines (the mesh-registration block
// itself). 150 leaves ample margin for legitimate growth while still catching a mis-scan: the
// phantom-block defect this tripwire was added for fabricated a ~290-line range by adopting an
// unrelated if-block's brace.
private const int MaxPlausibleBlockLines = 150;
private static string ReadProgramSource()
{
var dir = new DirectoryInfo(AppContext.BaseDirectory);
while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx")))
dir = dir.Parent;
dir.ShouldNotBeNull();
dir.ShouldNotBeNull("could not locate the repository root (no ZB.MOM.WW.OtOpcUa.slnx above the test binary)");
return File.ReadAllText(Path.Combine(
dir!.FullName, "src", "Server", "ZB.MOM.WW.OtOpcUa.Host", "Program.cs"));
@@ -54,9 +66,11 @@ public sealed class MeshRegistrationRoleScopingTests
}
/// <summary>
/// Brace-matches every <c>if (hasAdmin)</c> block in the file (not a regex over the block
/// Scans every <c>if (hasAdmin)</c> occurrence in the file (not a regex over the block
/// body — the body is free-form C# and a regex would be fragile to reformatting) and
/// returns each block's (open-brace, close-brace) character range.
/// returns each one's body as a (start, end) character range. C# permits an <c>if</c> with
/// no braces at all when the body is a single statement (Program.cs's ConfigDb gate is
/// exactly this shape), so each occurrence is classified by what actually follows it:
/// </summary>
private static List<(int Start, int End)> FindHasAdminBlockRanges(string source)
{
@@ -68,37 +82,91 @@ public sealed class MeshRegistrationRoleScopingTests
if (markerIndex < 0)
break;
var openBraceIndex = source.IndexOf('{', markerIndex + HasAdminMarker.Length);
openBraceIndex.ShouldBeGreaterThan(-1,
$"'{HasAdminMarker}' at offset {markerIndex} is not followed by an opening brace.");
var bodyStart = markerIndex + HasAdminMarker.Length;
while (bodyStart < source.Length && char.IsWhiteSpace(source[bodyStart]))
bodyStart++;
bodyStart.ShouldBeLessThan(source.Length,
$"'{HasAdminMarker}' at offset {markerIndex} has no body — reached end of file.");
var depth = 0;
var closeBraceIndex = -1;
for (var i = openBraceIndex; i < source.Length; i++)
{
if (source[i] == '{')
depth++;
else if (source[i] == '}')
{
depth--;
if (depth == 0)
{
closeBraceIndex = i;
break;
}
}
}
(int Start, int End) range;
if (source[bodyStart] == '{')
range = ScanBracedBlock(source, bodyStart, markerIndex);
else
// NOT a fallback IndexOf('{', ...) scan: the next brace in the file belongs to
// whatever code comes after this statement, not to this if — adopting it fabricates
// a phantom block spanning unrelated code (the exact defect this scanner exists to
// catch when it happens to a REAL registration).
range = ScanBracelessStatement(source, bodyStart, markerIndex);
closeBraceIndex.ShouldBeGreaterThan(-1,
$"brace-matching for the '{HasAdminMarker}' block starting at offset {markerIndex} never closed.");
var lineCount = 1 + CountOccurrences(source.Substring(range.Start, range.End - range.Start + 1), "\n");
lineCount.ShouldBeLessThanOrEqualTo(MaxPlausibleBlockLines,
$"the scanned '{HasAdminMarker}' body starting at offset {range.Start} spans {lineCount} " +
"lines, which is implausibly long for one of this file's admin-role gates — almost " +
"certainly a mis-scan (e.g. a braceless if adopting an unrelated block's brace) rather " +
"than a real block. Do not trust this range; fix the scanner instead.");
ranges.Add((openBraceIndex, closeBraceIndex));
searchIndex = closeBraceIndex + 1;
ranges.Add(range);
searchIndex = range.End + 1;
}
return ranges;
}
private static (int Start, int End) ScanBracedBlock(string source, int openBraceIndex, int markerIndex)
{
var depth = 0;
var closeBraceIndex = -1;
for (var i = openBraceIndex; i < source.Length; i++)
{
if (source[i] == '{')
depth++;
else if (source[i] == '}')
{
depth--;
if (depth == 0)
{
closeBraceIndex = i;
break;
}
}
}
closeBraceIndex.ShouldBeGreaterThan(-1,
$"brace-matching for the '{HasAdminMarker}' block starting at offset {markerIndex} never closed.");
return (openBraceIndex, closeBraceIndex);
}
/// <summary>
/// Bounds a braceless single-statement <c>if</c> body to its terminating <c>;</c>, tracking
/// paren/brace depth so a semicolon nested inside the statement itself (e.g. a lambda or
/// object initializer in the call's arguments) does not end the scan early.
/// </summary>
private static (int Start, int End) ScanBracelessStatement(string source, int statementStart, int markerIndex)
{
var depth = 0;
var semicolonIndex = -1;
for (var i = statementStart; i < source.Length; i++)
{
var c = source[i];
if (c is '(' or '{')
depth++;
else if (c is ')' or '}')
depth--;
else if (c == ';' && depth == 0)
{
semicolonIndex = i;
break;
}
}
semicolonIndex.ShouldBeGreaterThan(-1,
$"the braceless '{HasAdminMarker}' single-statement body starting at offset {markerIndex} " +
"never terminated with ';'.");
return (statementStart, semicolonIndex);
}
/// <summary>
/// Locates the single <c>if (hasAdmin)</c> block that contains <paramref name="registration"/>.
/// Fails loudly — rather than silently picking the first match — if the registration is
@@ -166,4 +234,29 @@ public sealed class MeshRegistrationRoleScopingTests
block.ShouldContain(DialerRegistration, Case.Sensitive,
$"IMeshCommandDialer must be registered inside 'if (hasAdmin)'. {why}");
}
/// <summary>
/// Pins the scanner's expected result shape against today's Program.cs — 6 markers,
/// 5 braced blocks + the 1 braceless ConfigDb gate, so 6 scanned ranges. Without this, a
/// scanner bug that silently drops or merges a marker (as the pre-fix version did to the
/// braceless marker — it fabricated a phantom range spanning an unrelated if-block and swallowed
/// a real nested 'if (hasAdmin)' block inside it, yielding 5 ranges instead of 6) would not
/// surface unless it happened to also break one of the two registration tests above.
/// </summary>
[Fact]
public void HasAdminMarkerScan_MatchesTheKnownShapeOfProgramCs()
{
var source = ReadProgramSource();
CountOccurrences(source, HasAdminMarker).ShouldBe(6,
"Program.cs's 'if (hasAdmin)' marker count changed. If a marker was added or removed, " +
"update this pin — and check whether the new/removed occurrence is braced or braceless — " +
"rather than letting the scanner silently mis-shape its result.");
var ranges = FindHasAdminBlockRanges(source);
ranges.Count.ShouldBe(6,
"expected one scanned range per 'if (hasAdmin)' marker, including the braceless ConfigDb " +
"gate. A mismatch means the scanner adopted an unrelated brace or otherwise merged/dropped " +
"a marker instead of handling each one independently.");
}
}