gramps
967df83925
The trap I hit twice today — a file-wide DoesNotContain matching the doc comment that explains the ban — was possible because three tests each hand-rolled their own File.ReadAllText + path-walker, two of them mine. Fixed at the root rather than per-test: SourceAssert now owns path walking and literal extraction, with Literal(memberName, params parts) and Scopes() that assert on the parsed VALUE. Tests that genuinely assert structure (the #if DEBUG arm in InstanceIsolationTests, the absence of a whole API in the locator) still read the file themselves, because those claims cannot be reduced to a value — the helper is for literal facts only. Shipping the fix introduced two defects, both caught by the build: a method named Path shadowed System.IO.Path and broke the error message that names the missing file, and Literal(fileName, memberName) invited a call that silently resolved to a nonexistent path. Argument order is now required-context-first so a wrong path is visible at the call site. Suite 380/380, 0 warnings. LayerReorderPersistenceTests.RealMouseDrag flaked once mid-session with no ytLive instance running and passed in isolation and on re-run — the environment flake already recorded in HANDOFF.md, not a regression from this change.
73 lines
3.9 KiB
C#
73 lines
3.9 KiB
C#
using System;
|
|
using System.IO;
|
|
using System.Linq;
|
|
using System.Text.RegularExpressions;
|
|
using Xunit;
|
|
namespace ytLive.Tests;
|
|
|
|
/// <summary>
|
|
/// Shared helpers for the <b>source-level</b> tests — the ones that assert on the text of a
|
|
/// source file because the behaviour they guard is structural (a field confined to
|
|
/// <c>#if DEBUG</c>, a type carrying no network capability, a build refusing to compile
|
|
/// without an asset). Those are legitimate assertions: <c>InstanceIsolationTests</c> has
|
|
/// carried one since 2026-08 and it caught a real hoisting bug.
|
|
///
|
|
/// <para><b>Use <see cref="Literal"/>, not a file-wide string match.</b> Three tests each grew
|
|
/// their own <c>File.ReadAllText</c> + <c>RepoFile</c> pair, and two of them then failed on the
|
|
/// file's own doc comment — the locator's <c>DoesNotContain("Download")</c> matched the sentence
|
|
/// saying "nothing is downloaded", and the auth test's <c>DoesNotContain("force-ssl")</c> matched
|
|
/// the comment explaining why force-ssl was removed. Both were fixed by parsing the literal out
|
|
/// of the source and asserting on the <i>value</i>. That failure is not a one-off: a file is
|
|
/// mostly prose, and the documentation is <em>supposed</em> to name the thing being banned. A
|
|
/// grep-based ban asserts "this string never appears", which is a much stronger and much less
|
|
/// useful claim than "this value is correct".</para>
|
|
///
|
|
/// <para>Only the path-walking and the literal extraction live here. Tests that genuinely need to
|
|
/// assert on structure — the <c>#if DEBUG</c> arm, the absence of a whole API — should still read
|
|
/// the file themselves, because those claims are about code and cannot be reduced to a value.</para>
|
|
/// </summary>
|
|
internal static class SourceAssert
|
|
{
|
|
/// <summary>Walks up from the test output directory to the repo root and returns the
|
|
/// absolute path of the given repo-relative file.</summary>
|
|
public static string RepoPath(params string[] parts)
|
|
{
|
|
var dir = new DirectoryInfo(AppContext.BaseDirectory);
|
|
while (dir != null)
|
|
{
|
|
var candidate = System.IO.Path.Combine(new[] { dir.FullName }.Concat(parts).ToArray());
|
|
if (File.Exists(candidate)) return candidate;
|
|
dir = dir.Parent;
|
|
}
|
|
throw new FileNotFoundException(
|
|
$"could not locate {System.IO.Path.Combine(parts)} above {AppContext.BaseDirectory}");
|
|
}
|
|
|
|
public static string Text(params string[] parts) => File.ReadAllText(RepoPath(parts));
|
|
|
|
/// <summary>Extracts the value of a C# string constant or <c>const</c> field declared in the
|
|
/// given source file, e.g. <c>Literal("Scope", "Services", "YouTubeAuthService.cs")</c> for
|
|
/// <c>private const string Scope = "…";</c>. Whitespace and the trailing semicolon are not
|
|
/// part of the value. The path parts are repo-relative and may span directories — the
|
|
/// member name comes first so a wrong path can never be mistaken for a member name.
|
|
/// </summary>
|
|
public static string Literal(string memberName, params string[] parts)
|
|
{
|
|
var source = Text(parts);
|
|
var match = Regex.Match(
|
|
source,
|
|
$@"(?:const|static readonly|readonly)\s+string\s+{Regex.Escape(memberName)}\s*=\s*""([^""]*)""\s*;");
|
|
Assert.True(match.Success,
|
|
$"could not find a string constant named '{memberName}' in {System.IO.Path.Combine(parts)} — " +
|
|
"if it was renamed or its declaration reshaped, update this test rather than " +
|
|
"loosening the assertion.");
|
|
return match.Groups[1].Value;
|
|
}
|
|
|
|
/// <summary>The scopes requested in a space-separated OAuth <c>scope</c> const, split into
|
|
/// individual scope URIs so a test can assert on the SET rather than on one string.</summary>
|
|
public static string[] Scopes(string memberName, params string[] parts) =>
|
|
Literal(memberName, parts)
|
|
.Split(' ', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
|
|
}
|