gramps/LlamaCasty
0
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

tests: one source-assertion helper, so the prose trap can't recur

Browse Source 
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.
This commit is contained in:
gramps
2026-09-28 11:53:52 -07:00
parent b3edc2e42e
commit 967df83925
5 changed files with 101 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MyMistakes.md
+22
View File
@@ -1340,3 +1340,25 @@ while actively documenting it. Repeating a documented mistake is not a knowledge
knowledge was fine — it is a *check* failure. The check that would have caught it: when a
`DoesNotContain` is aimed at a file, ask "does this file's own documentation have a legitimate
reason to contain the string?" If yes, the assertion is aimed at the wrong target.
### Extracting the duplicated helper created two fresh bugs in the same commit (2026-09-28)
Pulling the three hand-rolled `RepoFile` path-walkers into one `SourceAssert` helper — the fix for
the repeated prose-matching trap — shipped with two defects of its own, both caught by the build:
- **A method named `Path` shadowed `System.IO.Path` inside the class**, so the failure message that
wanted to *name the missing file* was itself a compile error (`CS0119`). Renamed to `RepoPath`.
A helper whose name collides with a BCL type is a trap for the next reader, not just for me.
- **The signature invited the bug it was meant to prevent.** `Literal(string fileName, string
memberName)` looked like it took a file name, but the body resolved the path from the repo root,
so the only legal call was a full directory path — and the call site passed
`"YouTubeAuthService.cs"`, which resolves to a path that does not exist. The failure was a
bare `FileNotFoundException` with no hint. Flipping the argument order to
`Literal(memberName, params string[] parts)` makes a wrong path *visible at the call site*
instead of buried in a resolved path.
**The generalisation:** a parameter shape that cannot express the common legal call is a defect
even when the compiler accepts it. `fileName` + `memberName` looks fine and is wrong; the honest
shape puts the variable part last and the required context first. This is the same family as the
prose-matching lesson — **the aim is to make the wrong thing hard to write, not to detect it
afterward** — and I got it wrong in the very commit written to apply that idea.
ytLive.Tests/FfmpegLocatorTests.cs
+3 -21
View File
@@ -32,24 +32,7 @@ public class FfmpegLocatorTests
return dir;
}
/// <summary>Repo root, found by walking up to a file only the repo has.</summary>
private static string RepoRoot()
{
var dir = new DirectoryInfo(AppContext.BaseDirectory);
while (dir != null)
{
if (File.Exists(Path.Combine(dir.FullName, "ytLive.csproj"))) return dir.FullName;
dir = dir.Parent;
}
throw new FileNotFoundException($"could not locate ytLive.csproj above {AppContext.BaseDirectory}");
}
private static string RepoFile(params string[] parts)
{
var candidate = Path.Combine(new[] { RepoRoot() }.Concat(parts).ToArray());
if (File.Exists(candidate)) return candidate;
throw new FileNotFoundException($"could not locate {Path.Combine(parts)} under {RepoRoot()}");
}
/// <summary>
/// The Good Dog integration test for this unit: prove the whole chain end to end —
@@ -66,7 +49,7 @@ public class FfmpegLocatorTests
// repository folder name. The repo directory happens to also be called "ytLive",
// so naively appending "ytLive" would resolve to a path that only works on this
// machine's checkout.
var projectDir = Path.GetDirectoryName(RepoFile("ytLive.csproj"))!;
var projectDir = Path.GetDirectoryName(SourceAssert.RepoPath("ytLive.csproj"))!;
var binRoot = Path.Combine(projectDir, "bin");
Assert.True(Directory.Exists(binRoot),
$"no build output at {binRoot} — build ytLive.csproj before running this test");
@@ -224,8 +207,7 @@ public class FfmpegLocatorTests
[InlineData("ExtractBinaries(")]
public void TheLocatorCarriesNoNetworkCapability(string forbidden)
{
var source = File.ReadAllText(
RepoFile("Services", "Encoder", "FfmpegLocator.cs"));
var source = SourceAssert.Text("Services", "Encoder", "FfmpegLocator.cs");
Assert.DoesNotContain(forbidden, source, StringComparison.OrdinalIgnoreCase);
}
@@ -238,7 +220,7 @@ public class FfmpegLocatorTests
[Fact]
public void TheBuildRefusesToCompileWithoutTheBundle()
{
var csproj = File.ReadAllText(RepoFile("ytLive.csproj"));
var csproj = SourceAssert.Text("ytLive.csproj");
Assert.Contains("GuardBundledFfmpeg", csproj, StringComparison.Ordinal);
Assert.Contains("ffmpeg\\ffmpeg.exe", csproj, StringComparison.Ordinal);
Assert.Contains("CopyToPublishDirectory=\"Always\"", csproj, StringComparison.Ordinal);
ytLive.Tests/InstanceIsolationTests.cs
+1 -13
View File
@@ -202,7 +202,7 @@ public sealed class InstanceIsolationTests
// DataRoot / WebViewDataFolder are present in both configurations. Note a
// const like YTLIVE_INSTANCE is INLINED at compile time and therefore never
// appears in either binary — grepping for the variable name proves nothing.
var source = File.ReadAllText(RepoFile("Helpers", "InstanceProfile.cs"));
var source = SourceAssert.Text("Helpers", "InstanceProfile.cs");
// The whole promise is that a Release build compiles to the #else arm, so assert
// that arm IS production: the one real user profile, no alternate folder, and no
@@ -225,16 +225,4 @@ public sealed class InstanceIsolationTests
Assert.Contains("instances", debugArm);
}
private static string RepoFile(params string[] parts)
{
var dir = new DirectoryInfo(AppContext.BaseDirectory);
while (dir != null)
{
var candidate = Path.Combine(new[] { dir.FullName }.Concat(parts).ToArray());
if (File.Exists(candidate)) return candidate;
dir = dir.Parent;
}
throw new FileNotFoundException(
$"could not locate {Path.Combine(parts)} above {AppContext.BaseDirectory}");
}
}
ytLive.Tests/SourceAssert.cs
+72
View File
@@ -0,0 +1,72 @@
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);
}
ytLive.Tests/YouTubeAuthServiceTests.cs
+3 -19
View File
@@ -140,25 +140,9 @@ public class YouTubeAuthServiceTests
[Fact]
public void ExactlyOneScopeIsRequested_AndItIsTheBroadOne()
{
var source = File.ReadAllText(RepoFile("Services", "YouTubeAuthService.cs"));
var match = Regex.Match(source, @"private const string Scope = ""([^""]*)"";");
Assert.True(match.Success, "could not find the Scope const in YouTubeAuthService.cs");
var scopes = match.Groups[1].Value.Split(' ', StringSplitOptions.RemoveEmptyEntries);
Assert.Equal(new[] { "https://www.googleapis.com/auth/youtube" }, scopes);
Assert.Equal(
new[] { "https://www.googleapis.com/auth/youtube" },
SourceAssert.Scopes("Scope", "Services", "YouTubeAuthService.cs"));
}
private static string RepoFile(params string[] parts)
{
var dir = new DirectoryInfo(AppContext.BaseDirectory);
while (dir != null)
{
var candidate = Path.Combine(new[] { dir.FullName }.Concat(parts).ToArray());
if (File.Exists(candidate)) return candidate;
dir = dir.Parent;
}
throw new FileNotFoundException(
$"could not locate {Path.Combine(parts)} above {AppContext.BaseDirectory}");
}
}