Files
ersatztv/ErsatzTV.Tests/Integration/SearchIndexMutationCoverageTests.cs
T
timothyandClaude Opus 5 9685132ee0
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 11s
PR Gates / Docs update reminder (pull_request) Successful in 14s
review-verdict/h10 Review-verdict: MERGEABLE @ 9685132 (base: main)
Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 12s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 13s
PR Gates / decisions lifecycle (pull_request) Successful in 17s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 43s
Review verdict / Set review-verdict status (pull_request_target) Successful in 9s
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 11m28s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 14m32s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 10m10s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 9m23s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 11s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 10s
fix(823): three cleanups from the coherence pass — a wrong witness, a wrong because, a duplicated paths:
Round-five review confirmed the decision record is coherent with no third
survivor of the empty reading, and returned three LOW findings. All are in
prose I wrote in the last two commits.

- The comment defending `(IsAbstract && !IsSealed)` cited
  AlternateScheduleSelectorTests as an in-repo static-fixture witness. That
  class IS static, but it merely NESTS its [TestFixture]es and declares no test
  of its own, so it would fail the sibling "declares no runnable test" assertion
  rather than demonstrating the point. The rule is right and the witness was
  wrong, which is the worse of the two failures because a wrong example is what
  a reader checks the rule against. No witness is cited now, and why is stated.

- A mis-bound `because` in `rule:`: "assigning a null and calling SaveChanges
  SUCCEEDS ... because only the HTTP request records normalize with `?? []`".
  The `?? []` clause explains how a null could REACH the entity; what makes the
  save succeed is the column being nullable. A right observation with a wrong
  cause attached. Split into the two claims.

- `signals:` carried the literal token `paths:` twice, an artifact of appending
  the #823 path list to the existing one. It degrades the field the discovery
  surface parses.

Also recorded from that review, and NOT changed: `MonthsOfYear ?? AllDaysOfMonth()`
survives the selector fixture and no date can kill it -- 1..31 contains every
valid month, so it is an EQUIVALENT mutant there rather than a coverage gap.
Its non-equivalent twin at the DTO boundary is pinned per-dimension by
RecurrenceLimitsMapperNullTests. Left alone deliberately: chasing an equivalent
mutant with a contrived date would buy nothing and cost the fixture's
readability.

Local gate: ErsatzTV.Tests 2091 passed / 6 skipped, Core.Tests 697/1 -- 0
failures. Format clean, no BOM. decisions_validate OK.

Refs #823
Refs #824

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
2026-08-29 21:14:27 +02:00

130 lines
7.7 KiB
C#

using System.Reflection;
using ErsatzTV.Core.Interfaces.Search;
using ErsatzTV.Infrastructure.Search;
using NUnit.Framework;
using NUnit.Framework.Interfaces;
using Shouldly;
namespace ErsatzTV.Tests.Integration;
/// <summary>
/// ersatztv#824. The defect that produced #824 was not that <c>ElasticSearchIndex</c> was hard to
/// test — it was that NOTHING NOTICED it had no cover. #701 fixed two independent copies of the same
/// <c>UpdateSong</c> logic and pinned one; the suite stayed green, and the gap survived on a
/// hand-written list of what had been covered (namely, one entry).
/// <para>
/// So the covered set is compared against a population DERIVED FROM THE ASSEMBLY rather than
/// restated: a third <see cref="ISearchIndex" /> implementation reddens this test until it is
/// given a mutation fixture of its own. That is
/// <c>testing.guard-derives-population-from-source</c> applied to a test population instead of a
/// file population.
/// </para>
/// <para>
/// <b>Scope, stated rather than implied.</b> The derivation is over the assembly that declares
/// both indexers (<c>ErsatzTV.Infrastructure</c>). An implementation added in a DIFFERENT
/// assembly is outside what this sees — it is not covered and this test cannot say so. Both
/// implementations have lived here since the interface existed, so the narrower scope buys a
/// guard that cannot be defeated by an unrelated assembly load order; widening it to every
/// loaded assembly would be the false-confidence version of the same check.
/// </para>
/// </summary>
[TestFixture]
public class SearchIndexMutationCoverageTests
{
/// <summary>
/// The indexers whose <c>UpdateSong</c> is pinned against the ersatztv#701 mutation. Referenced by
/// TYPE, so renaming an indexer or deleting a fixture is a compile error rather than a silent
/// divergence.
/// </summary>
private static readonly Dictionary<Type, Type> CoveredBy = new()
{
[typeof(LuceneSearchIndex)] = typeof(SongIndexerMetadataMutationTests),
[typeof(ElasticSearchIndex)] = typeof(ElasticSongIndexerMetadataMutationTests)
};
[Test]
public void Every_ISearchIndex_Implementation_Has_A_Metadata_Mutation_Fixture()
{
List<Type> implementations = typeof(LuceneSearchIndex).Assembly
.GetTypes()
.Where(t => t is { IsAbstract: false, IsInterface: false })
.Where(t => typeof(ISearchIndex).IsAssignableFrom(t))
.OrderBy(t => t.FullName, StringComparer.Ordinal)
.ToList();
// The set comparison below would ALREADY fail on an empty derivation, because the expected side
// is non-empty -- so this floor is not load-bearing for correctness and saying it is would be a
// false claim about a check. It is a DIAGNOSTIC: it separates "the reflection stopped finding
// types" (a moved type, a renamed interface) from "someone added an indexer", which the set
// comparison alone reports identically.
implementations.Count.ShouldBeGreaterThanOrEqualTo(
2,
"the ISearchIndex population derivation found almost nothing -- this is the derivation "
+ "breaking, not an indexer being added");
implementations.ShouldBe(
CoveredBy.Keys.OrderBy(t => t.FullName, StringComparer.Ordinal),
ignoreOrder: false,
"every ISearchIndex implementation needs an UpdateSong metadata-mutation fixture — see "
+ "ersatztv#824, where a second copy of the same logic went uncovered and an Elastic-only "
+ "reintroduction of `metadata.Artists ??= []` left the whole suite green");
// Comparing the KEYS alone would leave the mapping half-checked: a third indexer could be pointed
// at an EXISTING fixture, or at a fixture class holding no runnable test, and the set comparison
// above would still pass. Both are closed here. What NO static check can establish is that the
// named fixture actually DRIVES its indexer -- that is stated as a residual on this guard rather
// than implied away, and it is why the record claims a third implementation cannot be added
// WITHOUT NOTICE, not that it cannot be mis-covered.
CoveredBy.Values.Distinct().Count().ShouldBe(
CoveredBy.Count,
"two indexers are mapped to the SAME fixture, so one of them is not actually covered");
foreach ((Type indexer, Type fixture) in CoveredBy)
{
// Test-ness is decided by NUnit's INTERFACES, not by a hand-listed set of attribute types.
// A list has to be kept in step with NUnit and silently falsely-reddens whatever it omits --
// an earlier revision listed TestAttribute alone (so a [TestCase]-only fixture failed), then
// three types (so a [Theory] one did). ITestBuilder/ISimpleTestBuilder is what NUnit itself
// dispatches on, so it cannot fall behind the vocabulary.
//
// DeclaredOnly is load-bearing: without it a fixture that merely SUBCLASSES another inherits
// its [Test] and satisfies this while driving the wrong indexer -- and Values.Distinct()
// cannot catch that, since the two Types differ. Static is included because NUnit runs a
// public static test method in a non-static fixture, and omitting the flag would falsely
// redden one.
const BindingFlags TestMethods =
BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly;
static bool NeverRuns(object[] attributes) =>
attributes.Any(a => a is ExplicitAttribute or IgnoreAttribute);
bool declaresARunnableTest = fixture
.GetMethods(TestMethods)
.Select(m => m.GetCustomAttributes(inherit: true))
.Any(attrs =>
attrs.Any(a => a is ITestBuilder or ISimpleTestBuilder) && !NeverRuns(attrs));
declaresARunnableTest.ShouldBeTrue(
$"{fixture.Name} is named as the mutation fixture for {indexer.Name} but DECLARES no "
+ "runnable test method of its own, so it proves nothing");
// The same "wired is not running" failure at fixture level, in its three forms: an abstract
// class NUnit will not instantiate, and [Explicit]/[Ignore]. The indexer population at the top
// of this method already filters IsAbstract; the fixture side needs the mirror of that.
// `IsAbstract` ALONE is wrong here and would have been a false red: a C# `static class`
// compiles to `abstract sealed`, and NUnit runs a test declared in one. What must be rejected
// is an abstract BASE (abstract and NOT sealed), which NUnit cannot instantiate. Deliberately
// no in-repo witness is cited: `AlternateScheduleSelectorTests` is a static class, but it
// merely NESTS its `[TestFixture]`es and declares no test of its own, so it would fail the
// sibling assertion above instead -- naming it here would have been a wrong example attached
// to a right rule.
(fixture.IsAbstract && !fixture.IsSealed).ShouldBeFalse(
$"{fixture.Name} is named as the mutation fixture for {indexer.Name} but is an abstract "
+ "base class, so NUnit never runs it");
NeverRuns(fixture.GetCustomAttributes(inherit: true)).ShouldBeFalse(
$"{fixture.Name} is named as the mutation fixture for {indexer.Name} but is [Explicit] or "
+ "[Ignore]d, so it never runs and proves nothing");
}
}
}