diff --git a/sources/EncodingChecker.Tests/HarnessInvariantTests.cs b/sources/EncodingChecker.Tests/HarnessInvariantTests.cs new file mode 100644 index 0000000..4ef7c49 --- /dev/null +++ b/sources/EncodingChecker.Tests/HarnessInvariantTests.cs @@ -0,0 +1,173 @@ +using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; + +namespace EncodingChecker.Tests; + +/// +/// Rules about the test suite itself, checked by the test suite itself. +/// +/// and invoke +/// their callbacks concurrently from worker threads and say so. The suite passed +/// List<T>.Add anyway, in twenty files, and lost entries — measured at three +/// runs in forty over two hundred files. It surfaced once, as a CI failure whose message +/// blamed the product: a file modified after planning was reported as converted rather +/// than stale, because that file had never reached the plan for the staleness check to +/// look at. +/// +/// The rest of the time it did not surface at all, which is the part worth guarding +/// against. A dropped entry does not throw; it quietly removes a file from what a test +/// then asserts about, so the test passes while asserting less than it claims. A +/// concurrent test that silently loses an input does not merely flake — it can report a +/// safety property as verified when it was never exercised. +/// +/// So the rule is enforced here rather than left to each future author to remember. +/// +public sealed class HarnessInvariantTests +{ + /// + /// The calls whose callbacks the engine documents as concurrent. Callbacks handed to + /// anything else — a plain sequential enumeration, say — are not this rule's business, + /// and flagging them would make the rule noise that people learn to ignore. + /// + private static readonly string[] ConcurrentCalls = + [ + "ScanEngine.ScanDirectory(", + "ScanEngine.ConvertFiles(", + ]; + + /// + /// Passes NAME.Add as a method group — an argument, not a call. A call is + /// followed by '('; a callback being handed over is followed by ',' or ')'. + /// + private static readonly Regex AddAsCallback = new( + @"\b(\w+)\.Add\s*(?=[,)\r\n])", RegexOptions.Compiled); + + private static string TestSourceDirectory([CallerFilePath] string thisFile = "") => + Path.GetDirectoryName(thisFile)!; + + [Fact] + public void NoTestHandsAPlainListToAConcurrentCallback() + { + string directory = TestSourceDirectory(); + + Assert.True( + Directory.Exists(directory), + $"the test sources were not found at '{directory}'; this check reads them, " + + "so it has to run from a source checkout"); + + var offences = new List(); + var scanned = 0; + + foreach (string file in Directory.EnumerateFiles(directory, "*.cs")) + { + string text = File.ReadAllText(file); + + foreach ((int start, string arguments) in ConcurrentCallArguments(text)) + { + scanned++; + + foreach (Match match in AddAsCallback.Matches(arguments)) + { + string name = match.Groups[1].Value; + + if (!IsDeclaredAsAPlainList(text, start, name)) + continue; + + offences.Add( + $"{Path.GetFileName(file)}:{LineOf(text, start)}: " + + $"'{name}.Add' is a List's Add handed to a concurrent callback"); + } + } + } + + // A rule that silently matched nothing would look identical to a rule that + // passed, so the search itself is checked before its result is trusted. + Assert.True( + scanned > 20, + $"only {scanned} concurrent call site(s) were found to check; the search is " + + "not looking where it thinks it is"); + + Assert.True( + offences.Count == 0, + "A List is being handed to a callback the scan engine documents as " + + "concurrent, so entries can be dropped without anything failing. Collect " + + "with EntrySink instead." + + Environment.NewLine + + string.Join(Environment.NewLine, offences)); + } + + [Fact] + public void TheSanctionedSinkIsActuallyUsed() + { + // A ban with nothing to point at is a ban people work around. If the replacement + // ever stops being used, the rule above has quietly become unenforceable rather + // than satisfied. + int users = Directory + .EnumerateFiles(TestSourceDirectory(), "*.cs") + .Count(f => File.ReadAllText(f).Contains( + "new EntrySink()", StringComparison.Ordinal)); + + Assert.True( + users > 10, + $"only {users} test file(s) collect through EntrySink; the concurrent " + + "callbacks are being collected some other way"); + } + + /// + /// The argument list of every call whose callback runs concurrently, with the index + /// the call starts at. + /// + private static IEnumerable<(int Start, string Arguments)> ConcurrentCallArguments( + string text) + { + foreach (string call in ConcurrentCalls) + { + int from = 0; + + while (true) + { + int start = text.IndexOf(call, from, StringComparison.Ordinal); + + if (start < 0) + break; + + int open = start + call.Length - 1; + int depth = 0; + int i = open; + + for (; i < text.Length; i++) + { + if (text[i] == '(') + depth++; + else if (text[i] == ')' && --depth == 0) + break; + } + + yield return (start, text[(open + 1)..Math.Min(i, text.Length)]); + from = start + call.Length; + } + } + } + + /// + /// Whether the nearest declaration of before + /// makes it a plain List. + /// + /// + /// Nearest, not anywhere in the file: one method's List named entries + /// must not condemn another method's EntrySink of the same name, which is the + /// false positive the first version of this rule produced. + /// + private static bool IsDeclaredAsAPlainList(string text, int before, string name) + { + MatchCollection declarations = Regex.Matches( + text[..before], + $@"\b(?:var|List<[^>]*>)\s+{Regex.Escape(name)}\s*=\s*(new\s+)?(?\w+)"); + + return declarations.Count > 0 + && declarations[^1].Groups["type"].Value == "List"; + } + + private static int LineOf(string text, int index) => + text[..index].Count(c => c == '\n') + 1; +}