Skip to content

test(phase 2): add critical-path test coverage across Domain / Application / Infrastructure layers#342

Open
Vasar007 wants to merge 125 commits into
masterfrom
phase-02/test-coverage
Open

test(phase 2): add critical-path test coverage across Domain / Application / Infrastructure layers#342
Vasar007 wants to merge 125 commits into
masterfrom
phase-02/test-coverage

Conversation

@Vasar007

Copy link
Copy Markdown
Owner

Closes #341.

Summary

Phase 2: Test Coverage
Milestone: v0.9.8 — Test Coverage & Agent-Testable Feedback Loop
Goal: Identify the critical paths across Domain, Application, and Infrastructure layers and back them with a deliberate mix of unit, integration, and contract tests — running in the right CI stages and surfacing a coverage report for visibility, without committing to a 100 % gate.

Brownfield, additive only — no architectural rewrite. The existing TPL-Dataflow + ShellBuilder pipeline is unchanged; it is now testable and verifiable. 13 plans landed across 5 waves: a shared test library bootstrapped first (ProjectV.Tests.Shared), then the critical-path inventory + CI four-stage rewrite + coverage publication, then concentric slices of Domain unit tests, Application unit tests, pipeline (Dataflow + Crawlers + Executors + IO) tests, contract tests (WireMock.Net) against the three rating-API adapters, DAL integration tests via Testcontainers Postgres, JWT integration tests, Telegram webhook + polling integration tests, and finally a single gap-closure plan (02-13) that absorbed all findings from the post-implementation code-review loop.

Changes

Plan 02-01 — Tests Shared Bootstrap (Wave 1)

Sources/Tests/Directory.Build.props injects the shared C# test stack (xunit + AwesomeAssertions + NSubstitute + coverlet + opt-in TimeProvider.Testing / Testcontainers / WireMock.Net / AspNetCore.Mvc.Testing) into every .csproj under Sources/Tests/ while leaving the F# ProjectV.ContentDirectories.Tests.fsproj untouched (D-04). New library ProjectV.Tests.Shared exposes the base test classes (BaseTest, BaseMockTest, BaseDependencyInjectionTests, BaseExceptionTests<T>, TestTimeHelper), the first generator + mock-builder examples, and a FixtureLoader. Existing ProjectV.Appraisers.Tests + ProjectV.Common.Tests retrofitted to AwesomeAssertions; the previously skipped ModelSerializationTests now runs and passes. 4 new CPM <PackageVersion> entries added.

Key files: Sources/Tests/Directory.Build.props, Sources/Tests/ProjectV.Tests.Shared/**, Sources/Directory.Packages.props, Sources/ProjectV.sln.

Plan 02-02 — Coverage Inventory + Scenarios Doc (Wave 2)

Docs/Testing/Coverage/test-coverage.md enumerates the critical paths across Domain / Application / Infrastructure and maps each row to the plan that covers it (TEST-01). Docs/Testing/scenarios.md describes the four-tier scenario taxonomy + a Mermaid architecture diagram.

Key files: Docs/Testing/Coverage/test-coverage.md, Docs/Testing/scenarios.md.

Plan 02-03 — CI Four-Stage Rewrite + Coverage Publication (Wave 2)

.github/workflows/build.yml is split from a single omnibus C# test step into four named Linux stages — Test (Unit) / Test (Integration) / Test (Contract) / Test (F#) — plus a Windows Test (Non-Docker) stage (TEST-05). Coverlet → Cobertura → ReportGenerator pipeline publishes the HTML coverage report as a CI artifact on every run (TEST-06). Sources/Tests/coverlet.runsettings excludes generated code per D-27.

Key files: .github/workflows/build.yml, Sources/Tests/coverlet.runsettings.

Plan 02-04 — Domain Unit Tests (Wave 2)

83 Unit-tagged tests exercise ProjectV.Models (exception conventions, value-object invariants, BasicInfo round-trips) and ProjectV.Appraisers (rating-computation accuracy on the canonical Appraiser<T> + IAppraisal<T> compositions and AppraisersManager). Closes the Domain-layer slice of TEST-02.

Key files: Sources/Tests/ProjectV.Models.Tests/**, Sources/Tests/ProjectV.Appraisers.Tests/**.

Plan 02-05 — Core Application Unit Tests (Wave 2)

New ProjectV.Core.Tests project covers Shell + ShellBuilder composition + CommunicationServiceClient + Polly retry policies. New mock builders for Shell, ServiceClient, and IO/Crawlers managers added to Tests.Shared. Closes a substantial portion of the TEST-03 Application-layer slice.

Key files: Sources/Tests/ProjectV.Core.Tests/**, Sources/Tests/ProjectV.Tests.Shared/Helpers/Mocks/{Core,Net,IO,Crawlers}/**.

Plan 02-06 — Pipeline: Dataflow + Crawlers Tests (Wave 2)

DataflowPipeline integration coverage + 3 crawler unit suites. Split off from the original 02-06 per code-review WARNING-03 (file count). 3 critical-path rows of the TEST-02 / TEST-03 inventory close here.

Key files: Sources/Tests/ProjectV.Core.Tests/DataPipeline/**, Sources/Tests/ProjectV.Tests.Shared/Helpers/Mocks/Crawlers/**.

Plan 02-07 — Pipeline: Executors + IO Tests (Wave 2)

Unit-test slice for Executors + InputProcessing + OutputProcessing. Sibling split of 02-06 — closes the executors + input + output critical-path rows of TEST-03.

Key files: Sources/Tests/ProjectV.Core.Tests/{Executors,InputProcessing,OutputProcessing}/**.

Plan 02-08 — Contract Tests via WireMock.Net (Wave 2)

Three new contract-tier test projects (ProjectV.TmdbService.Tests / OmdbService.Tests / SteamService.Tests) exercise the real TMDbLib / OMDbApiNet / SteamWebApiLib HTTP pipelines against in-process WireMock.Net stubs fed from 7 recorded JSON fixtures. No live API calls; per-adapter contract assertions on URL shape, query params, and response decoding. Closes the contract slice of TEST-04.

Key files: Sources/Tests/ProjectV.TmdbService.Tests/**, Sources/Tests/ProjectV.OmdbService.Tests/**, Sources/Tests/ProjectV.SteamService.Tests/**, Sources/Tests/Fixtures/{Tmdb,Omdb,Steam}/*.json.

Plan 02-09 — DAL Integration Tests (Testcontainers + PostgreSQL) (Wave 2)

First DB integration-test slice: a single-container Testcontainers Postgres fixture, EF Core 9 design-time factory, initial migrations, and a generator + mock-builder layer (DbCollection / TestDbHelper / DAL generators). Rule 1 production-model fixes landed in the same plan to make persistence round-trip cleanly. Closes the DB slice of TEST-04.

Key files: Sources/Libraries/ProjectV.DataAccessLayer/Migrations/**, Sources/Tests/ProjectV.DataAccessLayer.Tests/**, Sources/Tests/ProjectV.Tests.Shared/Helpers/Persistence/**.

Plan 02-10 — JWT Integration Tests (Wave 2)

JWT runtime path on ProjectV.CommunicationWebService covered via WebApplicationFactory + a TestJwtHelper. New WebApiBaseTest base class lands in Tests.Shared. Closes the runtime JWT pending decision from Phase 1 (D-31).

Key files: Sources/Tests/ProjectV.CommunicationWebService.Tests/**, Sources/Tests/ProjectV.Tests.Shared/Helpers/WebApi/{TestWebApplicationFactory,TestJwtHelper,WebApiBaseTest}.cs.

Plan 02-11 — Telegram Webhook Integration Tests (Wave 3)

Webhook half of D-15 (Telegram full integration coverage). TestWebApplicationFactory extended with ITelegramBotClient + ICommunicationServiceClient stubs and a new TestTelegramBotClientBuilder added to Tests.Shared. Absorbs review WARNING-06 Option A on factory composition.

Key files: Sources/Tests/ProjectV.TelegramBotWebService.Tests/Webhook/**, Sources/Tests/ProjectV.Tests.Shared/Helpers/WebApi/TestWebApplicationFactory.cs, Sources/Tests/ProjectV.Tests.Shared/Helpers/Mocks/Telegram/TestTelegramBotClientBuilder.cs.

Plan 02-12 — Telegram Polling Integration Tests (Wave 4)

Polling half of D-15. ProjectV.TelegramBotWebService polling path (PoolingProcessor) integration-tested with the same factory composition as 02-11.

Key files: Sources/Tests/ProjectV.TelegramBotWebService.Tests/Polling/**.

Plan 02-13 — Review Followups (Wave 5 — gap-closure)

Closed four findings carried out of the Phase 2 code-review loop into a single atomic plan:

  • IN-02 — NLog root-cause fix + 12-way [ModuleInitializer] dedup; NLog test initializer hoisted to Tests.Shared.
  • IN-03FakeHttpMessageHandler hoisted to Tests.Shared (was duplicated across contract suites).
  • DA-1RefreshTokenDbInfo ctor guards against Guid.Empty.
  • QA S-1 — multi-row filter integration test for FindByUserIdAsync (with TokenHash assertion per iter-3).

Three subsequent doc-only iterations (iter-1..3) tightened comments in EF + NLog test infrastructure per the audit-team review.

Key files: Sources/Tests/ProjectV.Tests.Shared/Helpers/Logging/**, Sources/Tests/ProjectV.Tests.Shared/Helpers/Net/FakeHttpMessageHandler.cs, Sources/Libraries/ProjectV.DataAccessLayer/Models/Tokens/RefreshTokenDbInfo.cs, Sources/Tests/ProjectV.DataAccessLayer.Tests/Repositories/Tokens/RefreshTokenRepositoryTests.cs.

Requirements Addressed

REQ-ID Description Covered by
TEST-01 Critical-path inventory doc mapping Domain / Application / Infrastructure paths to tests Plan 02-02
TEST-02 Unit tests close Domain-layer logic gaps from the TEST-01 inventory Plans 02-04, 02-06
TEST-03 Application-layer tests cover Shell / ShellBuilder / DataflowPipeline / Executors Plans 02-05, 02-06, 02-07
TEST-04 Integration tests run against a real DB (Testcontainers) and contract tests cover TMDb / OMDb / Steam adapters Plans 02-08, 02-09
TEST-05 CI distinguishes unit / integration / contract stages; F# tests no longer silently skipped Plan 02-03
TEST-06 Coverage report is produced on every CI run and reachable from the build summary (artifact) Plan 02-03

Test Plan

  • Restore + build: dotnet restore Sources && dotnet build Sources/ProjectV.sln -c Release -p:Platform=x64 → 0 warnings / 0 errors.
  • Format check: dotnet format Sources/ProjectV.sln --severity warn --verify-no-changes → exit 0.
  • Unit stage (Linux): dotnet test Sources/ProjectV.sln -c Release -p:Platform=x64 --no-build --filter "Category=Unit" → pass locally; gated in CI as Test (Unit).
  • Integration stage (Linux, Testcontainers required): dotnet test Sources/ProjectV.sln -c Release -p:Platform=x64 --no-build --filter "Category=Integration" → pass; gated in CI as Test (Integration).
  • Contract stage (Linux): dotnet test Sources/ProjectV.sln -c Release -p:Platform=x64 --no-build --filter "Category=Contract" → pass; gated in CI as Test (Contract).
  • F# stage: dotnet test Sources/Tests/ProjectV.ContentDirectories.Tests/ProjectV.ContentDirectories.Tests.fsproj -c Release -p:Platform=x64 --no-build → 9 / 9 pass; gated in CI as Test (F#).
  • Windows non-Docker stage: Run on windows-latest — unit + contract only; integration suite skipped on Windows by design (Testcontainers not in scope for Windows runner).
  • Coverage: ReportGenerator HTML attached to every CI run as the coverage-report artifact. No hard coverage gate; visibility only (TEST-06).
  • Branch state: 52 commits ahead of master, 0 behind. All review-loop findings closed via Plan 02-13.

Key Decisions

Material decisions captured during Phase 2 planning + execution (full ledger in the plan artifacts):

  • D-02 / D-31..D-35ProjectV.Tests.Shared is a single shared library housing base test classes, generators, and mock builders. Every test project under Sources/Tests/ references it; no per-project duplication.
  • D-03 — Test stack injected via Sources/Tests/Directory.Build.props, conditioned on $(MSBuildProjectExtension) == '.csproj' so the F# project keeps its self-contained Unquote stack untouched (D-04).
  • D-04 — F# ProjectV.ContentDirectories.Tests.fsproj retains its own stack and runs in its own CI stage. Reaffirmed from Phase 1.
  • D-07 — License-clean assertion / mocking / mapper stack (AwesomeAssertions, NSubstitute, Riok.Mapperly) propagated to all new tests. Reaffirmed from Phase 1.
  • D-18 — Contract-test fixtures live under Sources/Tests/Fixtures/{Tmdb,Omdb,Steam}/ as checked-in recorded JSON. No live API calls in CI.
  • D-22 — Contract tests use WireMock.Net (in-process) rather than full process-isolated stubs — keeps CI fast and deterministic.
  • D-26 — CI rule: Linux owns tests; Windows owns build-only verification. Phase 2 keeps that split — Windows runs only the Non-Docker stage. Reaffirmed from Phase 1.
  • D-27 — Coverlet exclusions encoded in Sources/Tests/coverlet.runsettings; covers generated EF migrations + auto-generated NSwag clients.
  • WR-02 / 03 / 04 — Address review findings on naming + assertion shape + filter coverage (applied in 5e8c3af / ae883e1 / f0b1602).
  • IN-02 / IN-03 — Cross-suite NLog initializer + FakeHttpMessageHandler hoisted to Tests.Shared rather than left duplicated per project (Plan 02-13).
  • DA-1RefreshTokenDbInfo ctor rejects Guid.Empty to make the invariant explicit at the domain layer (Plan 02-13).

Out of Scope

Tracked but explicitly deferred:

Closing Issues

Closes #341

Vasar007 and others added 30 commits May 18, 2026 23:24
…d test-stack injection

- Add Sources/Tests/Directory.Build.props that imports the parent
  Sources/Directory.Build.props (so child projects keep AppPlatforms /
  TestTargetFrameworks / CSharpLangVersion / ManagePackageVersionsCentrally)
  and injects the shared test stack as PackageReference items into every
  C# test csproj — coverlet.collector + Microsoft.NET.Test.Sdk + xunit +
  xunit.runner.visualstudio + AwesomeAssertions + NSubstitute (core), plus
  the opt-in stack Microsoft.AspNetCore.Mvc.Testing + Microsoft.Extensions.TimeProvider.Testing
  + Testcontainers.PostgreSql + WireMock.Net.
- ItemGroup PackageReference items are conditioned on
  '$(MSBuildProjectExtension)' == '.csproj' so the F# ContentDirectories.Tests
  project keeps its own Unquote-based stack untouched (Decision D-04 +
  Specifics §4).
- Add four new PackageVersion entries to Sources/Directory.Packages.props:
  Microsoft.AspNetCore.Mvc.Testing 10.0.8, Microsoft.Extensions.TimeProvider.Testing
  10.0.0 (10.0.8 was not published; 10.0.0 is the closest matching version on
  nuget.org — minor deviation from PLAN), Testcontainers.PostgreSql 4.11.0,
  WireMock.Net 2.6.0. No Version= attributes on any PackageReference (CPM).
- Drop the now-duplicate per-csproj PackageReference entries (coverlet, test
  sdk, xunit, xunit runner) from ProjectV.Appraisers.Tests and ProjectV.Common.Tests
  — the shared injection makes them duplicates that would error under
  TreatWarningsAsErrors=true (NU1504). The retrofit of test bodies + adding the
  Tests.Shared ProjectReference stays in Task 3 as planned.

Decision references: D-02, D-03, D-04 (F# untouched).
…pers

Establishes the shared test-infrastructure library under
Sources/Tests/ProjectV.Tests.Shared/ that every downstream Phase 2 test
plan will reference (Decisions D-02, D-31..D-35).

- ProjectV.Tests.Shared.csproj — RootNamespace=ProjectV.Tests.Shared,
  Library, IsPublishable/IsPackable=false. No PackageReference for the
  core test stack (Directory.Build.props supplies it). ProjectReferences:
  ProjectV.Appraisers, ProjectV.CommonCSharp, ProjectV.Models plus
  Acolyte.NET + Microsoft.Extensions.{DependencyInjection,Hosting} via CPM.
- Usings/SharedUsings.cs — global usings exposed to every consumer:
  System, Collections.Generic, Linq, Threading.Tasks, AwesomeAssertions,
  NSubstitute (+ ExceptionExtensions / ReceivedExtensions), Xunit,
  ProjectV.Tests.Shared.ForTests. Scoped #pragma warning disable IDE0005
  on the file (intentional unused-in-this-assembly usings — downstream
  test projects rely on them).
- ForTests/ base-class hierarchy (D-32):
  - BaseTest (root; xUnit-shaped — no [TestFixture], ctor for setup)
  - BaseMockTest : BaseTest (NSubstitute Substitute.For helper)
  - BaseDependencyInjectionTests : BaseMockTest (CreateServiceCollection /
    CreateHostAppBuilder + AssertOn_NotRegistered / RegisteredService /
    RegisteredServiceBeOfType using AwesomeAssertions)
  - BaseExceptionTests<TException> where TException : Exception : BaseTest
    (abstract Create / Create(string) / Create(string, Exception) hooks
    plus three [Fact] methods covering the 3-ctor convention; explicit
    Arrange/Act/Assert markers)
  - TestTimeHelper (thin wrapper around FakeTimeProvider)
- Helpers/Generators/Models/BasicInfoGenerator.cs (D-34) — seeded
  Random(Seed: 42) per Specifics §5; Create*/Generate* twin pattern;
  Acolyte guard clause; XML docs on every public member.
  NOTE: the planned `new Random(seed: 42)` lowercase parameter name does
  not compile under .NET 10 (CS1739) — the constructor parameter is
  `Seed` (capital S). Used `Random(Seed: 42)` to preserve the intent
  (deterministic seed 42 per Specifics §5).
- Helpers/Mocks/Appraisers/TestAppraiserBuilder.cs (D-33) — sealed builder
  for IAppraiser substitutes with WithRating / WithRatingFactory fluent
  configuration; CreateWithoutSetup static convenience; XML docs.
  NOTE: IAppraiser.GetRatings(BasicInfo, bool) returns a single
  RatingDataContainer (not a list), so the builder exposes single-rating
  configuration rather than the list-state described in PLAN §Task 2
  action — the builder shape matches the real production API.
- Helpers/Fixtures/FixtureLoader.cs (D-18) — static LoadJsonFixture
  resolving paths under AppContext.BaseDirectory/Fixtures; Acolyte
  ThrowIfNullOrWhiteSpace; clear FileNotFoundException on miss.
- Helpers/{WebApi,Stubs,Persistence,Extensions}/.gitkeep — reserved
  directories per D-31 + D-35 (downstream plans populate them).
- Sources/Tests/Fixtures/{Tmdb,Omdb,Steam}/.gitkeep — fixture directories
  for contract tests (02-08).
- Sources/ProjectV.sln — registered ProjectV.Tests.Shared in the Tests
  solution folder with Debug|x64 and Release|x64 configurations
  (hand-edited; `dotnet sln add` corrupted the solution with Any CPU /
  x86 configurations on every project — restored from HEAD and applied
  the entry manually).

All .cs files are UTF-8 with BOM per Sources/.editorconfig.
`dotnet build Sources/ProjectV.sln -c Release -p:Platform=x64` and
`dotnet format Sources/ProjectV.sln --severity warn --verify-no-changes`
both pass with 0 warnings / 0 errors.

Decision references: D-02, D-05, D-18, D-31, D-32, D-33, D-34, D-35.
…ssertions

Translates every Xunit.Assert call in the two existing C# test projects to
AwesomeAssertions, adds [Trait("Category","Unit")] for the xUnit filter
discovery in the upcoming four-stage CI rewrite (02-03), and references
ProjectV.Tests.Shared so downstream plans can rely on the shared base
classes / generators / mock builders being available.

- AppraiserTests.cs: every Assert.NotNull/NotEmpty/Equal/Throws call
  rewritten to .Should().Be(...) / .Should().NotBeEmpty() /
  .Should().Throw<T>().WithParameterName(...). Explicit AAA comment
  markers (// Arrange. / // Act. / // Assert.) per D-07. Scoped
  #pragma warning disable CS8625 retained around the null-literal
  argument tests. Intentional typo CallGetRatingsWithConteinerWithOneItem
  preserved verbatim (Specifics §3). Sealed class shape and explicit
  empty ctor unchanged.
- TestDataCreator.cs: RandomInstance seeded with 42 for run-to-run
  determinism (Specifics §5). The literal `new Random(seed: 42)` from
  PLAN does NOT compile under .NET 10 (CS1739 — parameter is `Seed`
  with capital S), so the call site uses `new Random(Seed: 42)` —
  preserves the seed value and intent. File retained (callers in
  AppraiserTests still use it; ProjectV.Tests.Shared.Helpers.Generators
  .Models.BasicInfoGenerator coexists for downstream plans).
- ModelSerializationTests.cs: rewritten to use Newtonsoft.Json
  (JsonConvert.SerializeObject / DeserializeObject) which honours
  BasicInfo's [JsonConstructor] annotation without a parameterless
  ctor — removes the original [Fact(Skip=...)] (Pitfall 7). Also
  rewritten to AwesomeAssertions with explicit AAA markers and
  [Trait("Category","Unit")] on the class. Both compact + pretty-print
  round trips covered.
- Appraisers.Tests + Common.Tests csprojs:
  - Drop now-supplied-by-Directory.Build.props PackageReference entries
    happened in Task 1 (CPM/restore was already broken otherwise).
  - Add ProjectReference to ProjectV.Tests.Shared so downstream test
    work can opt into the shared infrastructure incrementally.
  - Common.Tests adds explicit Newtonsoft.Json PackageReference (CPM —
    no Version=) because the model round-trip now uses Newtonsoft.
- F# ProjectV.ContentDirectories.Tests UNTOUCHED (D-04 + Specifics §4):
  `git diff HEAD Sources/Tests/ProjectV.ContentDirectories.Tests/` shows
  zero changes; the 9 existing F# tests still pass.

`dotnet build Sources/ProjectV.sln -c Release -p:Platform=x64` exits 0
(0 warnings, 0 errors). `dotnet test` on Appraisers.Tests passes
14/14, Common.Tests passes 1/1, F# ContentDirectories.Tests passes 9/9.
`dotnet format Sources/ProjectV.sln --severity warn --verify-no-changes`
exits 0.

Decision references: D-04, D-07, D-21.
…/Coverage/test-coverage.md

- New file Docs/Testing/Coverage/test-coverage.md — Phase 2 TEST-01 deliverable
- 31 critical-path rows across Domain (8) / Application (10) / Infrastructure (13) sections
- Sources rows verbatim from .planning/phases/02-test-coverage/02-RESEARCH.md (Categorical Critical-Path Inventory)
- Quotes TEST-01 wording from .planning/REQUIREMENTS.md verbatim
- Legend documents Path / Component / Planned Test Project / Test Type / Status columns
  and the planned / partially covered / covered / tested around vocabulary
- Trait conventions section names [Trait("Category","Unit")], [Trait("Category","Integration")],
  [Trait("Category","Contract")], and the secondary RequiresDocker trait per D-21
- Three anti-patterns from ARCHITECTURE.md (Shell-references-plugins, SimpleExecutor stub) flagged as "tested around"
- Maintenance section explains the downstream contract: flip Status -> covered + add Test Files column
- Cross-references to projectv-scenario-tests-overview.md, ARCHITECTURE.md, INTEGRATIONS.md, REQUIREMENTS.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…agram

- New file Docs/Testing/Scenarios/projectv-scenario-tests-overview.md (D-37)
- Purpose / Audience / Architecture / Scenario Family Documents / Conventions
- Mermaid flowchart TD translates 02-RESEARCH.md System Architecture Diagram
  into a renderable diagram with 5 branches (Unit, Integration via WebApplicationFactory,
  Contract via WireMock, F# via Unquote) and explicitly marks the dashed-edge
  test-double boundary (WireMock for HTTP, Testcontainers for Postgres)
- Conventions section names: sealed class shape, per-family base class,
  explicit AAA comment markers, [Trait("Category","Integration")] + [Trait("RequiresDocker","true")],
  XML class doc in business terms, [Collection(DbCollection.Name)] for shared container
- Scenario Family Documents section enumerates expected downstream filenames
  (projectv-jwt-scenarios.md / projectv-telegram-scenarios.md / projectv-tmdb-pipeline-scenarios.md)
  and quotes D-37 that family docs land alongside their suites
- Cross-references to test-coverage.md (TEST-01 inventory) and to
  .planning/codebase/ARCHITECTURE.md plus 02-RESEARCH.md patterns

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ions

- Adds the Coverlet runsettings consumed by the Phase 2 CI four-stage rewrite
  (Plan 02-03) via the `--settings` argument on each Linux test step.
- Format: cobertura (matches `dotnet test --collect:"XPlat Code Coverage"`
  output expected by ReportGenerator's `**/TestResults/**/coverage.cobertura.xml`
  glob).
- Excludes assemblies: `[ProjectV.DesktopApp]*` (Windows-only WPF, untested in
  Phase 2 by D-04 + D-26 — Linux coverage scope) and `[ProjectV.*.Tests]*`
  (test assemblies are not production code).
- ExcludeByFile: `**/Migrations/**/*.cs` (EF schema migrations) and
  `**/*.Designer.cs` (generated WinForms/resource designers).
- Satisfies D-27.
…r + coverage publication

- Replaces the omnibus `Test (C# projects)` + `Test (F# ContentDirectories)`
  steps with four named Linux stages: `Test (Unit)`, `Test (Integration)`,
  `Test (Contract)`, `Test (F#)` (D-20, D-21, D-23). Each C# stage filters by
  the xUnit `[Trait("Category", ...)]` trait established in Plan 02-01;
  the F# stage keeps the existing explicit `fsproj` invocation but is now a
  first-class named step (closes the silent-skip wording in TEST-05).
- Adds coverage publication on the Linux job only (D-24, D-26): each Linux C#
  stage runs `--collect:"XPlat Code Coverage" --settings Sources/Tests/coverlet.runsettings`;
  `danielpalme/ReportGenerator-GitHub-Action@5` merges the per-stage Cobertura
  outputs into an HtmlInline + MarkdownSummaryGithub bundle;
  `actions/upload-artifact@v4` publishes the `coverage-report` directory;
  `coverage-report/SummaryGithub.md` is appended to `$GITHUB_STEP_SUMMARY`
  for an in-PR coverage panel.
- Adds a Windows `Test (Non-Docker)` stage (D-22) — single-quoted YAML filter
  `'RequiresDocker!=true'` so PowerShell does NOT history-expand `!=`
  (Pitfall 4) and YAML keeps the string verbatim. Docker-dependent
  Testcontainers tests stay Linux-only via the `[Trait("RequiresDocker","true")]`
  tag.
- Branch-protection contract preserved (Pitfall 5): job name
  `Build (${{ matrix.os }})` unchanged, so the required-checks list
  (`Build (ubuntu-latest)`, `Build (windows-latest)`, `CodeQL`) needs no
  admin update. Triggers (push + pull_request on master / phase-** / feature/**),
  matrix definition, Checkout, Setup .NET, Restore (both solutions), Build
  (both solutions), and Format check all unchanged.

Satisfies TEST-05 (four named stages, F# first-class) and TEST-06 (coverage
artifact + per-run step summary).
- TestAppraisersManagerBuilder: real AppraisersManager populated with
  NSubstitute IAppraiser children (sealed-class fallback per D-33 +
  PLAN.md Task 1 action).
- UserIdGenerator + JobIdGenerator: Create/Generate twin pattern
  (D-34); raw GUID fed via UserId.Parse / JobId.Parse.
- XML docs on every public member; Acolyte guard on Create*.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…object, BasicInfo invariant suites

- New Sources/Tests/ProjectV.Models.Tests/ project, RootNamespace
  ProjectV.Models.Tests, ProjectReferences to ProjectV.Models +
  ProjectV.Tests.Shared, Newtonsoft.Json PackageReference (CPM, no
  Version=). Registered in Sources/ProjectV.sln under the Tests folder
  with Debug|x64 + Release|x64 only — no AnyCPU.
- Exceptions/CannotGetTmdbConfigExceptionTests: inherits
  BaseExceptionTests<TException> from 02-01 (D-32), 3 inherited Facts.
- Exceptions/CommonExceptionsTestSuite: reflection-driven 3-ctor
  convention enforcement across every sealed Exception subtype in
  ProjectV.Models.Exceptions — auto-discovers new types.
- ValueObjects/UserIdTests + JobIdTests: 13 facts each covering Create,
  Wrap, Parse, TryParse, None, IsSpecified, round-trip; uses
  UserIdGenerator/JobIdGenerator from 02-04 Task 1.
- Data/BasicInfoInvariantsTests: primitive defaults, Kind discriminator,
  equality semantics (memberwise + 1e-6 tolerance on VoteAverage),
  Newtonsoft.Json compact + pretty round-trip.
- [Trait("Category", "Unit")] on every class — 43 tests pass under the
  Unit CI filter.
- Docs/Testing/Coverage/test-coverage.md: Domain rows flipped to
  covered with new Test Files column; BasicInfo invariants row split
  from MovieInfo/GameInfo (still planned).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…aisersManager suites

- AppraisersExtensions/MovieCommonAppraiserTests: 8 facts verifying
  Appraiser<TmdbMovieInfo> + TmdbCommonAppraisal end-to-end —
  Tag/TypeId/RatingName, popularity-as-rating, null guard,
  type-mismatch guard, ctor guard.
- AppraisersExtensions/MovieNormalizedAppraiserTests: 7 facts
  verifying Appraiser<BasicInfo> + BasicAppraisalNormalized with
  PrepareCalculation/RawDataContainer/MinMaxDenominator wiring —
  min/max endpoints map to 0/2, middle item bounded, missing-prepare
  throws InvalidOperationException, null guards.
- AppraisersExtensions/AppraisersManagerTests: 10 facts verifying
  Add/Remove/CreateFlow over AppraisersManager via the new
  TestAppraisersManagerBuilder + TestAppraiserBuilder doubles —
  null guards, idempotent Add, two-instance-same-TypeId flow,
  Remove existing/missing, flow distinctness.
- AppraisersExtensions/AppraisersManagerTestsInit: [ModuleInitializer]
  pins NLog.LogManager.Configuration to an empty LoggingConfiguration
  so the pre-existing Sources/Libraries/ProjectV.Logging/NLog.config
  bug (NLog 6 dropped `concurrentWrites="true"` but the file still
  declares it under throwConfigExceptions=true) does not trip the
  AppraisersManager static logger field at type-init time.
- Plan named the rating-computation files
  MovieCommon/MovieNormalizedAppraiserTests; ProjectV has no such
  types — the production shape is Appraiser<T> + IAppraisal<T>.
  Tests exercise the strategy directly (it IS the unit under test
  for rating-computation accuracy). Documented inline on each file.
- Docs/Testing/Coverage/test-coverage.md: Appraiser rows flipped to
  covered; concrete-appraiser row split into MovieCommon (covered) +
  MovieNormalized (covered) + Game-suite (still planned).
- All 25 new facts carry [Trait("Category", "Unit")]; existing 14
  AppraiserTests facts unchanged — 39 Unit tests in Appraisers.Tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…uilders to Tests.Shared

- TestInputManagerBuilder / TestCrawlersManagerBuilder / TestOutputManagerBuilder:
  D-33 fallback builders returning real sealed concrete managers populated
  with NSubstitute child doubles via the public Add API
- TestShellBuilder under Helpers/Mocks/Core/: composes the four managers
  (defaulting to empty CreateWithoutSetup() instances) and exposes a fluent
  WithXxxManager / WithBoundedCapacity chain
- TestCommunicationServiceClientBuilder under Helpers/Mocks/Core/: substitutes
  the ICommunicationServiceClient interface seam with optional Login/StartJob
  success or error responses (Result<TOk, ErrorResponse>)
- ProjectV.Tests.Shared.csproj: add ProjectReferences to ProjectV.Core,
  ProjectV.Crawlers, ProjectV.InputProcessing, ProjectV.OutputProcessing so
  the new builders compile

Note: the plan's TestAppraisalManagerBuilder is intentionally NOT added — the
production type is AppraisersManager and the matching builder
(TestAppraisersManagerBuilder) already shipped with 02-04. Decision recorded
in 02-05-SUMMARY.md.
…r unit suites

- ProjectV.Core.Tests.csproj: new test project mirroring Sources/Libraries/ProjectV.Core/
- ShellTests: 8 facts covering ctor null-guards (5 args), property surface,
  Dispose idempotency, CreateBuilderDirector static factory.
  Run() branch tests intentionally omitted — see SUMMARY § Deviations
  for the Gridsum.DataflowEx empty-pipeline blocker (deferred to integration).
- ShellBuilderFromXDocumentTests: 6 facts covering ctor null/missing-root guards,
  minimal-config happy path, GetResult-before-build guard, Reset, BuildMessageHandler
  missing-element error path.
- ShellBuilderDirectorTests: 6 facts covering ctor null-guard, happy path,
  ChangeShellBuilder null-guard, MakeShell invokes all 7 IShellBuilder methods
  (Reset + 5 build steps + GetResult) in declared order, and dispatches to a
  replaced builder.
- CoreTestsModuleInitializer: [ModuleInitializer] installs an empty NLog
  LoggingConfiguration so Shell / ShellBuilderFromXDocument / CommunicationServiceClient
  static loggers don't trip the pre-existing NLog.config concurrentWrites bug
  (same pattern as Appraisers.Tests AppraisersExtensions/TestModuleInitializer).
- Sources/ProjectV.sln: register ProjectV.Core.Tests under Tests folder with
  Debug|x64 + Release|x64 only (no AnyCPU).
- Docs/Testing/Coverage/test-coverage.md: flip Shell.Run row to 'tested around'
  with rationale, ShellBuilderFromXDocument + ShellBuilderDirector rows to
  'covered' with Test Files paths. Added Test Files column to Application Layer.
- Tests.Shared mock builders: UTF-8 BOM normalized via dotnet format.
… Net/

- CommunicationServiceClientTests: 3 facts covering LoginAsync.
  Constructs the production concrete CommunicationServiceClient with a
  substituted IHttpClientFactory whose HttpClient is backed by an inline
  FakeHttpMessageHandler (DelegatingHandler). Asserts:
    1. 200 + token JSON body  → Result.Ok<TokenResponse> with the access token.
    2. 401 + ErrorResponse body → Result.Error<ErrorResponse> (NOT a thrown
       exception — production code uses Result<TOk, ErrorResponse>; recorded
       in SUMMARY as deviation from plan's 'throws AuthFailureException' wording).
    3. null login request → ArgumentNullException('login').
  StartJobAsync deferred to integration tests (deviation rationale in SUMMARY).
- HttpClientPollyPolicyTests: 3 facts covering the Polly retry policy wired
  by AddHttpClientWithOptions → AddHttpOptions → AddTransientHttpErrorPolicy:
    1. 503 → 503 → 503 → 200 with RetryCountOnFailed=3 → 4 handler invocations.
    2. Always-503 with RetryCountOnFailed=2 → 3 invocations (initial + 2 retries).
    3. First-call-200 → 1 invocation (no retry on success).
  Uses an inline DelegatingHandler subclass set as the primary HTTP message
  handler via ConfigurePrimaryHttpMessageHandler — Substitute.For<HttpMessageHandler>
  is explicitly avoided per 02-RESEARCH.md Pitfall 6 (NSubstitute cannot mock
  protected methods).
- ProjectV.Core.Tests.csproj: add Newtonsoft.Json PackageReference (CPM, no
  Version) for serializing the test response bodies — production code on this
  path uses Newtonsoft.Json (System.Text.Json migration deferred per 02-CONTEXT).
- Docs/Testing/Coverage/test-coverage.md: flip CommunicationServiceClient row
  and AddHttpClientWithOptions Polly retry row to 'covered' with Test Files.
…Shared

- TestTmdbCrawlerBuilder / TestOmdbCrawlerBuilder / TestSteamCrawlerBuilder:
  D-33 builders for ICrawler substitutes. Each yields an IAsyncEnumerable
  to match the real ICrawler.GetResponse(string, bool) signature (not
  Task-returning as the PATTERNS.md illustrative template implied).
  Default Tag matches the production crawler's nameof; supports
  WithThrowOnGetResponse(ex) for exercising CrawlersManager log+rethrow.
- TestDataflowPipelineBuilder: D-33 fallback for the sealed
  DataflowPipeline (real instance with optional InputtersFlow /
  OutputtersFlow stages; empty flows by default).
- ProjectV.Tests.Shared.csproj picks up ProjectV.DataPipeline as a
  ProjectReference so the new DataPipeline builder compiles.
…th rows

New test projects:
- ProjectV.DataPipeline.Tests (Integration) — DataflowPipelineTests +
  InputtersFlowTests. The DataflowPipeline integration test wires the full
  InputtersFlow → CrawlersFlow → AppraisersFlow → OutputtersFlow stages
  with NSubstitute ICrawler/IAppraiser leaves; the InputtersFlow tests
  cover the dedup + length-filter contract via reflection on the private
  FilterInputData predicate (predicated-link without LinkLeftToNull()
  deadlocks Gridsum.DataflowEx completion — see SUMMARY Deviations) plus
  a happy-path end-to-end smoke.
- ProjectV.Crawlers.Tests (Unit) — CrawlersManagerTests covering the
  log+rethrow contract on the private TryGetResponse (reflection probe;
  the static NLog logger half is intentionally not substituted), plus
  ctor + Add null-guard + Remove happy path.

ModuleInitializer in each new project installs an empty
NLog.LoggingConfiguration to bypass the NLog 6 / concurrentWrites
auto-load bug (same pattern as 02-04 / 02-05).

Sources/ProjectV.sln hand-edited (per 02-01 lesson — `dotnet sln add`
corrupts to AnyCPU) to register both projects under the Tests folder
with Debug|x64 + Release|x64 only.

Docs/Testing/Coverage/test-coverage.md updated: 3 rows in the
Application Layer section flipped from `planned` to `covered`
(DataflowPipeline.Execute / InputtersFlow / CrawlersManager.TryGetResponse)
with rationale notes for the deviation cases.
…test slice

Adds three new test projects covering the executors / input / output
critical-path rows from the Phase 2 inventory (TEST-03 subset):

- ProjectV.Executors.Tests — SimpleExecutorTests asserts the current
  parameterless ExecuteAsync() anti-pattern (NotImplementedException
  stub per ARCHITECTURE.md). Inventory row flipped to "covered (tested
  around)". Also covers ctor null-guard, executions-number guard, and
  happy-path property exposure.
- ProjectV.InputProcessing.Tests — InputManagerTests asserts
  CreateFlow(string) returns non-null InputtersFlow (with/without
  registered IInputter substitutes), ctor null/whitespace guard, Add
  null-guard, and Remove round-trip. Empty-storage-name path is left
  to higher-level integration coverage because InputManager.CreateFlow
  reaches into the process-wide GlobalMessageHandler static.
- ProjectV.OutputProcessing.Tests — OutputManagerTests asserts the
  analogous CreateFlow(string) → non-null OutputtersFlow contract,
  including the empty-storage-name fallback (OutputManager does not
  reach into GlobalMessageHandler), ctor null/whitespace guard, Add
  null-guard, and Remove round-trip.

Each project carries [Trait("Category", "Unit")] per D-21, picks up
the shared test stack from Sources/Tests/Directory.Build.props (D-03),
and uses NSubstitute for IInputter / IOutputter collaborators (D-05).
A ModuleInitializer mirrors the 02-05 / 02-06 NLog auto-load
short-circuit pattern for assemblies that touch production types with
static `_logger` fields.

Sources/ProjectV.sln — three new project entries with Debug|x64 +
Release|x64 only (no AnyCPU), nested under the Tests solution folder.
Docs/Testing/Coverage/test-coverage.md — three Application Layer rows
flipped from `planned` to `covered`.

Refs: 02-07 Plan / Phase 2 Test Coverage / TEST-03

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t tests

- Sources/Tests/Fixtures/Tmdb/search-movie-success.json (id=12345)
- Sources/Tests/Fixtures/Tmdb/search-movie-empty.json (zero-results envelope)
- Sources/Tests/Fixtures/Tmdb/configuration-success.json (images + change_keys)
- Sources/Tests/Fixtures/Omdb/movie-by-title-success.json (Response:"True")
- Sources/Tests/Fixtures/Omdb/movie-by-title-not-found.json (Response:"False")
- Sources/Tests/Fixtures/Steam/app-list-success.json (applist + 3 entries)
- Sources/Tests/Fixtures/Steam/app-detail-success.json (appid 730 envelope)

Synthetic data only — no live-API responses, no keys, no PII (D-17).
Fixture filenames reflect production surface (TmdbClient exposes
TrySearchMovieAsync / GetConfigAsync, not GetMovieAsync) — Rule 1 deviation
from plan filenames movie-by-id-*.json (no such SUT method exists);
documented in SUMMARY.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three new test projects under Sources/Tests/, each tagged
[Trait("Category", "Contract")] so the 02-03 CI Contract stage picks
them up:

- ProjectV.TmdbService.Tests (3 tests):
  TrySearchMovieAsyncReturnsExpectedContainer,
  TrySearchMovieAsyncEmptyResultReturnsEmptyContainer,
  GetConfigAsyncReturnsExpectedConfig.
  Redirection seam: new TmdbClient(apiKey, useSsl: false,
  baseUrl: WireMockHostPort) via InternalsVisibleTo.

- ProjectV.OmdbService.Tests (2 tests):
  TryGetItemByTitleAsyncReturnsExpectedMovie,
  TryGetItemByTitleAsyncNotFoundReturnsNull.
  Redirection seam: HttpClient.DefaultProxy = new WebProxy(WireMock.Url)
  because OMDbApiNet 1.3.0 hardcodes BaseUrl as a const field.

- ProjectV.SteamService.Tests (2 tests):
  GetAppListAsyncReturnsExpectedApps,
  TryGetSteamAppAsyncReturnsExpectedApp.
  Redirection seam: reflection-replace _steamApiClient with an SDK
  instance built from a SteamApiConfig whose SteamPoweredBaseUrl /
  SteamStoreBaseUrl point at WireMock.

Each test uses real-bytes-on-the-wire against in-process WireMockServer
instances serving the 7 recorded JSON fixtures from Sources/Tests/Fixtures/
(committed in the previous commit). No live API calls, no API keys, no
mocked HttpMessageHandler. Each test asserts LogEntries.Should().HaveCount(1)
to verify exactly-once HTTP semantics on the success path.

Three production libraries get a Properties/AssemblyInfo.cs adding
InternalsVisibleTo("ProjectV.<X>Service.Tests") — minimal, opt-in
seam-widening so the internal sealed wrapper types are constructable from
the test assembly. Same pattern already used by ProjectV.Appraisers /
ProjectV.Appraisers.Tests.

Sources/ProjectV.sln registers all three new projects under the Tests
solution folder (Debug|x64 + Release|x64 only, no AnyCPU).

Docs/Testing/Coverage/test-coverage.md: the three Infrastructure rows
for TmdbClient / OmdbClient / SteamApiClient flip to `covered` with
their test files; the table grows the `Test Files` column for the
remaining `planned` rows in the same section.

Build: dotnet build Sources/ProjectV.sln -c Release -p:Platform=x64 → 0 warnings.
Tests: dotnet test ... --filter "Category=Contract" → 7/7 passed (3+2+2).
Format: dotnet format Sources/ProjectV.sln --severity warn --verify-no-changes → clean.
No regressions: Unit=129, Integration=7, F#=9, Contract=7 (was 129/7/9 → +7 Contract).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… BLOCKING fallback)

Adds the EF Core 10.0.8 design-time tooling required to attempt
`dotnet ef migrations add InitialCreate` for ProjectV.DataAccessLayer:

- CPM: Microsoft.EntityFrameworkCore.Design 10.0.8 entry.
- ProjectV.DataAccessLayer.csproj: PackageReference (PrivateAssets="All").
- ProjectV.ProcessingWebService.csproj: PackageReference (PrivateAssets="All")
  so the EF CLI accepts it as the startup project.
- ProjectVDbContextDesignTimeFactory: implements
  IDesignTimeDbContextFactory<ProjectVDbContext> so the CLI can construct the
  context (the production type exposes two single-arg ctors and the CLI
  cannot disambiguate them on its own).
- Migrations/.gitkeep with a remark documenting how the generation attempt
  was made and why it stops at design-time model discovery.

[BLOCKING] Migration generation does NOT succeed in 02-09. EF Core rejects
every ctor of UserDbInfo because the immutable `RefreshTokenDbInfo refreshToken`
parameter cannot bind to a mapped scalar or navigation. Fixing this requires
architectural changes (parameterless ctor / explicit HasOne / mapper-only nav)
which are out of scope for 02-09. The 02-09 DbCollectionFixture takes the
RESEARCH.md fallback path and bootstraps the test container with raw SQL —
see Task 2 commit + 02-09-SUMMARY.md "[BLOCKING] Migration generation deferred".
…sk 2)

Adds the Testcontainers PostgreSQL test infrastructure for the DAL slice
plus three D-34 entity generators in ProjectV.Tests.Shared.

ProjectV.Tests.Shared additions:
- Helpers/Generators/DataAccessLayer/JobInfoGenerator.cs (Create/Generate
  twin, seeded Random(seed: 42), composes with JobIdGenerator).
- Helpers/Generators/DataAccessLayer/UserInfoGenerator.cs (Create/Generate
  twin, composes with UserIdGenerator).
- Helpers/Generators/DataAccessLayer/RefreshTokenInfoGenerator.cs
  (Create/Generate twin, composes with UserIdGenerator).
- ForTests/TestDbHelper.cs — TRUNCATE TABLE … RESTART IDENTITY CASCADE on
  public.{jobs,users,tokens}; ChangeTracker.Clear() to avoid stale-entity
  DbUpdateConcurrencyException between cases (D-11).
- ProjectReference on ProjectV.DataAccessLayer (TestDbHelper consumes
  ProjectVDbContext).

ProjectV.DataAccessLayer.Tests (new):
- ProjectV.DataAccessLayer.Tests.csproj — references DataAccessLayer +
  Configuration + Models + Tests.Shared; Testcontainers, EF Core, Npgsql
  flow transitively or via Sources/Tests/Directory.Build.props (D-03).
- ForTests/DbCollection.cs — [CollectionDefinition("ProjectV.DAL.Db")] +
  ICollectionFixture<DbCollectionFixture>.
- ForTests/DbCollectionFixture.cs — PostgreSqlBuilder("postgres:16.4")
  (pinned image, Pitfall 1), WithDatabase/User/Password, WithWaitStrategy
  Wait.ForUnixContainer().UntilInternalTcpPortIsAvailable(5432). Sets
  CanUseDatabase=true on every DatabaseOptions (Pitfall 2).
- Schema bootstrap path: raw SQL CREATE TABLE statements derived from
  [Table]/[Column] attrs on JobDbInfo/UserDbInfo/RefreshTokenDbInfo. See
  the class XML doc + Migrations/.gitkeep + 02-09-SUMMARY.md "[BLOCKING]
  Migration generation deferred" for why MigrateAsync/EnsureCreatedAsync
  are NOT used in Phase 2.

Sources/ProjectV.sln registers ProjectV.DataAccessLayer.Tests with x64-only
configs (Debug|x64 + Release|x64); no AnyCPU.

`dotnet build Sources/ProjectV.sln -c Debug -p:Platform=x64` is green.
…ask 3)

Adds four DAL integration test classes against the live Testcontainers
PostgreSQL fixture from Task 2 plus the Rule 1 production fixes that
unblock them.

Test classes (10 [Fact] methods, all green against postgres:16.4):

- Services/Jobs/DatabaseJobInfoServiceTests.cs (3 tests):
  AddAsync round-trip, FindByIdAsync round-trip, UpdateAsync mutation.
- Services/Users/DatabaseUserInfoServiceTests.cs (3 tests):
  AddAsync, FindByIdAsync, FindByUserNameAsync lookup.
- Services/Tokens/DatabaseRefreshTokenInfoServiceTests.cs (2 tests):
  AddAsync, FindByIdAsync expiry round-trip.
- ProjectVDbContextSchemaTests.cs (2 tests):
  information_schema query for public.{jobs,users,tokens};
  CanUseDb() + Npgsql connection sanity.

Every class declares
  [Trait("Category", "Integration")]  [Trait("RequiresDocker", "true")]
  [Collection(DbCollection.Name)]
so the Linux CI Integration stage picks them up and Windows Non-Docker
correctly filters them out (RequiresDocker!=true).

Rule 1 production-code fixes (necessary for the tests to run at all —
see 02-09-SUMMARY.md Deviations for the full reasoning):

- UserDbInfo: `RefreshToken` marked [NotMapped] + new internal 6-arg ctor
  for EF Core ctor binding. The previous 7-arg ctor + `builder.Property(e
  => e.RefreshToken)` configuration raised ModelValidator on EVERY DbContext
  access. Mapper path (7-arg ctor → 6-arg ctor → 7-arg sets RefreshToken)
  preserves existing semantics for DataAccessLayerMapper.MapToUserDbInfo.
- DatabaseUserInfoService.FindByUserNameAsync: rewritten to compare the
  raw `user.UserName` string scalar instead of the unmapped
  `user.WrappedUserName` computed property — EF Core cannot translate the
  latter ("Translation of member 'WrappedUserName' on entity type
  'UserDbInfo' failed. This commonly occurs when the specified member is
  unmapped.").
- DatabaseRefreshTokenInfoService.FindByUserIdAsync: same fix —
  compare the raw `token.UserId` Guid scalar instead of the never-assigned
  `token.WrappedUserId` record-struct property.

Test infrastructure:

- TestDbHelper now takes a connection string and uses Npgsql directly so
  the per-test TRUNCATE does not depend on a working ProjectVDbContext.
- DbCollectionFixture's ApplySchemaAsync uses Npgsql directly (raw SQL
  CREATE TABLE) for the same reason — the bootstrap is independent of
  the EF model.

Docs/Testing/Coverage/test-coverage.md: 4 Infrastructure rows flipped
planned → covered with status notes pointing at the SUMMARY deviations.

`dotnet build Sources/ProjectV.sln -c Debug -p:Platform=x64` is green;
`dotnet format Sources/ProjectV.sln --severity warn --verify-no-changes`
exits 0; `dotnet test ... --filter "Category=Integration"` reports
10/10 passing against the live container; no regressions in Unit / Contract.
Vasar007 and others added 5 commits July 7, 2026 01:29
The dispatch and duplicate-registration tests only observed mock
property reads and could not fail if the manager stopped routing
entities. They now post an entity into the constructed flow, await the
emitted rating, and assert the child appraiser was invoked exactly
once; the registration test is renamed to state what it verifies.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Expected ratings in the appraiser suite were computed by re-running the
production appraisal, so a formula regression could never fail them.
A new test pins the rating to a hand-computed literal (VoteAverage),
and the creator helper now documents the wiring-only nature of the
derived expectations.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…security audit

Transitive packages Microsoft.OpenApi 2.4.1 and Scriban.Signed 7.0.6
trip NuGetAudit (NU1902/NU1903) as hard errors under
TreatWarningsAsErrors, failing restore solution-wide. Enable central
transitive pinning and pin the lowest patched versions:

- Microsoft.OpenApi 2.7.5 (fixes GHSA-v5pm-xwqc-g5wc)
- Scriban.Signed 7.2.1 (fixes GHSA-24c8-4792-22hx, GHSA-6q7j-xr26-3h2c,
  GHSA-q6rr-fm2g-g5x8)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace per-dependency constructor null-argument [Fact]s in ShellTests,
DataflowPipelineTests, and SimpleExecutorTests with the canonical
two-test shape: a positive Constructor_OnAllArgumentsProvided test built
on a Create{Sut}WithAllArguments factory helper, and a single negative
test iterating a list of actions that pass exactly one dependency as
null and assert the ArgumentNullException parameter name. Non-null-guard
constructor cases (ArgumentOutOfRangeException, property population)
stay as separate tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add BaseGuidWrapperTests<TId> to ProjectV.Tests.Shared encoding the
shared behaviour of GUID-backed identifier value-objects (None/default
equality, Create, Wrap, Parse, TryParse valid/invalid/null, and
generator round-trips) behind abstract hooks. Collapse the previously
near-verbatim JobIdTests and UserIdTests twins into thin subclasses
that only bridge each type's static surface and its test-data
generator; every previous assertion is preserved on the base class.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Vasar007

Vasar007 commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

Test-suite quality pass — audit + fixes

Ran a full quality audit of the C# test suite (81 files) against the project's test conventions and the canonical example test project, then verified and fixed every valid finding. 24 commits on this branch (5818581..2717594), one commit per fix.

What changed (test code)

  • Dead test infrastructure removed — unused OMDb/Steam crawler builder clones, an unused dataflow-pipeline test builder, an unused time helper, and a stale unused-variable discard (8f5bb41, 4308eb0, b664f42, 8803e7e).
  • Test data generators aligned to convention — direct generator/creator calls pulled out of [Fact] bodies into private helpers; generators exposed as singletons; seeded Random replaced with the thread-safe Random.Shared and misleading "deterministic" docs corrected (fae7b99, bdab019, 7099406).
  • Stronger assertions — appraiser-manager dispatch tests now drive AppraisersFlow end-to-end and assert the emitted rating; the webhook happy-path scenario now asserts a bot reply is actually sent instead of only checking HTTP 200 (445b541, 9f7c02c).
  • Doc hygiene — XML docs that narrated authoring history rewritten to describe current behaviour; scenario-test docs updated to match the concrete stubs (no longer referencing the old substitute approach); an invalid namespace cref fixed (0b93dde, 825709b, 7ad61ba).
  • Constructor null-guard tests adopted the canonical two-test shape (all-arguments factory + one parameterised null-guard test) for Shell, DataflowPipeline, and SimpleExecutor (ef1ee0c).
  • Primitive-wrapper tests deduplicated — a shared BaseGuidWrapperTests<T> base collapses the near-identical JobId/UserId test twins with no loss of coverage (2717594).
  • Plus smaller cleanups: BuildSut() helper, arranged-value reuse in assertions, extracted config/arrangement helpers, and a misspelled-method-name fix.

Dependency security fix

a2d64b1 pins patched Microsoft.OpenApi 2.7.5 and Scriban.Signed 7.2.1 (transitive) to clear a solution-wide NU1903 warning-as-error from newly-published high-severity advisories. This audit failure was pre-existing at 5818581 (advisories published after the last green run), so restore/build fail without it regardless of the test changes.

Verification at HEAD 2717594

  • dotnet build Sources/ProjectV.sln -c Release -p:Platform=x64 — 0 errors (26 pre-existing MSB3277 warnings).
  • Full C# suite — 170 passed, 0 failed, 0 skipped across 15 assemblies.
  • dotnet format Sources/ProjectV.sln --severity warn --verify-no-changes — exit 0.
  • CI green: Build (ubuntu-latest), Build (windows-latest), Analyze (csharp), CodeQL — all pass.

Production-code follow-ups (not changed here — test-scoped run)

The audit also surfaced production observations. The three net-new ones (an unfinished SimpleExecutor.ExecuteAsync() stub, Shell's lack of interface seams, and static logger fields that make log-contract assertions impossible) are tracked in #353. The EF Core model-validation migration blocker remains tracked in #346; the remaining items were already on the project backlog.

Vasar007 and others added 15 commits July 17, 2026 01:14
The webhook and polling scenario base classes were near-verbatim twins:
duplicate ResolvedBotStubs holder, IBotService/ICommunicationServiceClient
DI swaps, NLog capture hook, and in-memory configuration builder differing
only in the WorkingMode value. Hoist the shared machinery into an abstract
TelegramScenarioBaseTest (Scenarios/Helpers) parameterized by working mode,
shrink both per-family bases to thin wrappers, and move the text-message
Update builder onto the shared base so webhook and polling scenarios
arrange updates identically.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
… helpers

Several appraiser and pipeline tests constructed BasicInfo and
RatingDataContainer directly in test bodies although Tests.Shared ships
BasicInfoGenerator with the exact explicit-args creation method. Add one
private CreateBasicInfo helper per class delegating to the generator
singleton and a private CreateRating helper wrapping the
RatingDataContainer constructor (no generator exists for it yet), and
route every inline creation site through them.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
CreateBasicInfoListRandomly multiplied unbounded Random values by the loop
index, so voteCount overflowed Int32 into negative values for most
elements and voteAverage reached ~100 against the documented 0-10 domain;
the random title suffix length was also coupled to the list size. Route
each element through BasicInfoGenerator.GenerateBasicInfo (sequential
thing ids, generator-bounded random fields, GUID-unique titles) and drop
the hand-rolled random-string helper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
Eighteen With* builder methods had zero consumers anywhere in the test
tree: the entire slot mechanism on TestShellBuilder, the child/storage
overrides on the three manager builders, WithAppraiser and
WithOutputResults on TestAppraisersManagerBuilder, WithRatingFactory on
TestAppraiserBuilder, and WithResponses on TestTmdbCrawlerBuilder. Delete
the dead methods together with their now-unneeded backing fields and
update the affected XML docs; the consumed surface (CreateWithoutSetup,
Build, WithAppraisers, WithResponse, WithTag, WithTypeId, WithRating,
WithThrowOnGetResponse) is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
…helper

CreateMovie declared non-nullable optional voteAverage/voteCount
parameters that no caller ever overrode; test-class data helpers must be
optional-free. Keep the single popularity parameter callers actually vary
and inline the remaining fields as fixed valid values.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
The middle-item test only asserted the [0, 2] envelope although the fixed
batch makes the value hand-computable ((50-10)/(90-10) + (7-5)/(9-5) =
1.0), so an in-envelope formula regression passed unnoticed. Assert the
exact value with the arithmetic in the assertion reason and rename the
test accordingly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
…eator

The (Guid, params BasicInfo[]) overload of CreateExpectedValueForBasicInfo
had no callers - the sole external call site passes an IReadOnlyList and
binds to the IEnumerable overload - and was reachable only through its own
class remarks cref. Delete the overload and retarget the cref at the
surviving one.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
The coverage-scope paragraph carried a doubled article ("in an a future")
and residual forward-looking phrasing left over from an earlier doc
rewrite. Reword the sentence to plain prose.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
…suite

ShellTests and ShellBuilderFromXDocumentTests carried byte-identical
minimal-XDocument helpers under two different names in the same assembly.
Rename CreateMinimalConfiguration to CreateMinimalShellConfigXml so the
sibling classes use one name for one shape.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
The comment claimed `await Task.CompletedTask` forces the method to be
truly asynchronous - an already-completed awaitable continues
synchronously and forces nothing. Reword it to state the actual purpose:
satisfying the compiler's async-iterator requirement while iteration
remains synchronous.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
Three test files spelled out System.Collections.Generic.IReadOnlyList,
System.Collections.Concurrent.ConcurrentBag, and System.Data.ConnectionState
inline although importing the namespace is the file and suite norm. Add
the missing using directives and shorten the references.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
The by-title contract test arranged a title constant but asserted against
a repeated string literal. Compare against the variable so the arranged
value and the expectation cannot drift apart.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
Remove the no-signal Remove(...) tail assertion from the Add-idempotency
test (Remove returns true whether one or two Adds happened and its
behaviour is covered by the dedicated Remove tests), and move the
CreateFlow call in CreateWithoutSetupReturnsEmptyManager from the Assert
section into a proper Act section.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G7ErBrm9WdLKFMzykyxKXW
Pass every argument to the Create{Sut}WithAllArguments factories by name
in both the positive and negative constructor tests, so each null-guard
entry reads as a grep-friendly single-parameter diff.

The factories intentionally keep required, non-nullable parameters: an
optional-parameter variant with null-coalescing fallbacks would replace
an explicitly passed null with a valid dependency, so the negative test
could never reach the constructor guard (verified empirically - the
assertion reports "no exception was thrown"). Required parameters keep
each null observable by the guard and match the reference test shape
used elsewhere.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The two scenario base classes were saved without the UTF-8 BOM required
by .editorconfig, which fails the formatting verification step in CI.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Vasar007

Copy link
Copy Markdown
Owner Author

Test-suite quality pass — second audit round

Re-audited the full C# test suite at the new HEAD against the project's test conventions and the canonical example test project, including the code changed by the first round. 15 commits (2717594..f44a252), one commit per fix.

Real defect found and fixed

TestDataCreator.CreateBasicInfoListRandomly built voteCount: i * Random.Shared.Next(), which overflows Int32 for every i >= 2 and routinely produced negative vote counts; voteAverage reached ~100 against a documented 0–10 domain. Test data now comes from the shared generator with valid, in-domain values (fc90924).

Cleanups from the first round's own changes

  • Twin scenario bases merged — the webhook/polling Telegram scenario base classes had become ~150-line near-verbatim twins; the shared parts are now one base class (763fd61, BOM follow-up f44a252).
  • Inline test data routed through generatorsnew BasicInfo(...) / new RatingDataContainer(...) built inline in test bodies now go through generator-backed private helpers (c8c3649).
  • Dead builder API removed — 18 fluent With* methods with zero consumers, an unused params overload, and unused optional parameters (016a864, 42567b0, 70e5069).
  • Assertions tightened — exact normalized-rating assertion replaces a permissive range; assertions compare against arranged values instead of repeated literals; a no-signal trailing assert removed and an Act moved out of an Assert block (3cc598c, 5a14c32, f35d31d).
  • Docs/consistency — corrected a false "truly asynchronous" comment, fixed doc grammar, aligned a duplicated helper's name, replaced fully-qualified type names with usings (315d210, 385f280, 7611abb, 751ce88).

Constructor null-guard tests — a note worth recording

The audit flagged that Create{Sut}WithAllArguments uses required parameters rather than nullable optionals with fallbacks. Implementing the nullable-optional form was tried and rejected on evidence: C# cannot distinguish an omitted optional from an explicitly passed null, so a null! argument gets replaced by the fallback, the guard is never reached, and the test passes for the wrong reason (Expected a <System.ArgumentNullException> to be thrown, but no exception was thrown). The canonical example project's own helpers all use required parameters with named arguments in the negative cases — which is what this suite already did. Only the named-argument style was missing, and that was applied (9130a10). The null-guard tests were then proven failing-capable by deliberately corrupting an expected parameter name and confirming all three suites fail.

Verification at HEAD f44a252

  • dotnet build Sources/ProjectV.sln -c Release -p:Platform=x64 — 0 errors (26 pre-existing MSB3277 warnings).
  • Full C# suite — 170 passed, 0 failed, 0 skipped across 15 assemblies.
  • dotnet format Sources/ProjectV.sln --severity warn --verify-no-changes — exit 0.
  • CI green: Build (ubuntu-latest), Build (windows-latest), Analyze (csharp), CodeQL.

Production-code follow-ups

Two further testability seams were found (the Steam/OMDb client wrappers expose no injection seam, so contract tests use private-field reflection and mutate the global proxy; CommunicationServiceClient builds its HttpClient eagerly in the constructor). Both are low severity and were added to #353 rather than opened as a separate issue — no production code changed in this test-scoped round.

Vasar007 and others added 7 commits July 17, 2026 12:51
BaseDependencyInjectionTests in ProjectV.Tests.Shared had zero consumers
across the whole test tree — no test class inherits it and no member is
called. Delete it and repoint the package-reference comment in the csproj
at the actual consumers of the DI/Hosting packages (WebApiBaseTest and
TestWebApplicationFactory).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rator

GetRatingsThrowsForBaseBasicInfoBecauseAppraiserExpectsTmdb built its
BasicInfo input inline in the test body. Add the same private
CreateBasicInfo helper the sibling appraiser test classes carry
(delegating to BasicInfoGenerator.Instance) and route the test through
it, so no test body creates model data inline.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
UpdateAsyncWithExistingJobPersistsChanges built its mutated JobInfo copy
inline in the test body. Add a private CreateMutatedJobInfo helper that
delegates to JobInfoGenerator.CreateJobInfo, preserving the exact
mutation the assertions depend on (state and result incremented, id,
name and config kept from the original row).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ce exclusion

The webhook base class remark claimed the polling / webhook hosted
services would resolve IBotService before the ConfigureTestServices
override wins — that is wrong (test overrides mutate the service
collection before the provider is built, and hosted services resolve
only at host start, as the polling sibling base documents correctly).
Reword to the real reason: excluding those hosted services keeps their
stub calls out of BotServiceStub.CalledMethodNames and limits each
scenario family's host to the machinery its scenarios exercise.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The class remark claimed the test stops the host explicitly inside the
act-phase polling loop — it never does; the host is stopped by
WebApiBaseTest disposal after the test, and the act phase only waits
with a bounded timeout for the expected SendMessageAsync call count.
The arrange comment also referenced a nonexistent WithUpdateSequence
method; the stub is pre-loaded through its constructor.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TestShellBuilder and ShellTests docs still claimed the composed managers
are populated with NSubstitute child doubles. Since the fluent With*
slots were removed, every sibling manager builder returns an empty
production manager via CreateWithoutSetup(), and no substitutes are
attached through this path. Reword both docs to match the code.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The arrange comment claimed the inputter echoes the storage-name input
back as the entity name, but the lambda ignores its input and always
yields the fixed entity name. Reword the comment to match the code.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Vasar007

Copy link
Copy Markdown
Owner Author

Test-suite quality pass — third audit round

Re-audited the full C# test suite at the new HEAD against the project's test conventions and the canonical example test project, with particular attention to the code the second round changed. 7 commits (f44a252..b22c6c1), one commit per fix.

The suite is now materially clean: 0 high-severity findings, 0 new production findings, and every hard convention prohibition passes. What remained was one dead class and documentation that the previous rounds' own refactors had left stale.

What changed

  • Dead base class removedBaseDependencyInjectionTests (89 lines) had zero consumers; both earlier dead-code sweeps missed it. Deleted, and the stale project-file comment repointed at the packages' real consumers (8ce31c0).
  • Inline test data routed through generators — two residual sites building new BasicInfo(...) / new JobInfo(...) directly in test bodies now go through generator-backed private helpers. The mutated-copy test keeps the exact field mutation its assertions depend on (cdc741c, 9ecbfca).
  • Documentation corrected to match the code — four docs described behaviour that no longer exists or never did:
    • the webhook scenario base's dependency-swap rationale stated the opposite of what the code does, and contradicted its polling sibling (3bebe3b);
    • the polling scenario referenced a WithUpdateSequence method that no longer exists and claimed the test stops the host explicitly, which it does not (1a808ab);
    • TestShellBuilder/ShellTests still claimed managers were "populated with NSubstitute child doubles" — after the unused fluent methods were removed they return real, empty managers (09c82a6);
    • a dataflow test comment claimed the inputter "echoes the storage-name input back", while the lambda ignores its input and returns a constant (b22c6c1).

Verification at HEAD b22c6c1

  • dotnet build Sources/ProjectV.sln -c Release -p:Platform=x64 — 0 errors (26 pre-existing MSB3277 warnings).
  • Full C# suite — 170 passed, 0 failed, 0 skipped across 15 assemblies.
  • dotnet format Sources/ProjectV.sln --severity warn --verify-no-changes — exit 0.
  • CI green: Build (ubuntu-latest), Build (windows-latest), Analyze (csharp), CodeQL.

Production code

No new production findings this round. The previously reported items remain tracked in #346 and #353; no production code was changed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Tests Related to the tests type: Code Maintenance New feature/requirement which is targeting on improve architecture, realization and code style

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

test(phase 2): add critical-path test coverage across Domain / Application / Infrastructure layers

1 participant