test(phase 2): add critical-path test coverage across Domain / Application / Infrastructure layers#342
Open
Vasar007 wants to merge 62 commits into
Open
test(phase 2): add critical-path test coverage across Domain / Application / Infrastructure layers#342Vasar007 wants to merge 62 commits into
Vasar007 wants to merge 62 commits into
Conversation
…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.
- WR-02: OmdbContractTests moves HttpClient.DefaultProxy mutation from ctor into InitializeAsync so the save/restore pair is strictly paired across the xUnit IAsyncLifetime boundary. Save in ctor (immutable state), mutate in InitializeAsync, restore in DisposeAsync. - WR-03: Add the missing FindByUserIdAsyncAfterAddReturnsTokenWithExpectedUserId integration test in DatabaseRefreshTokenInfoServiceTests. Exercises the Plan 02-09 Rule-1 raw-Guid comparison fix that previously had zero integration coverage. With this commit the DAL Docker suite is 11 tests (up from 10). - WR-04: Add explicit FRAGILE comment on the SteamContractTests private-field reflection seam (`_steamApiClient`) explaining the documented Rule-3 deviation from 02-08-SUMMARY.md and the non-fragile fix path (ctor overload on ProjectVSteamApiClient that accepts a SteamApiConfig). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three findings from the iter-2 audit-team re-review (5 parallel reviewers @ sonnet on the iter-1 fix diff). Audit verdicts in Notes/Reviews/phase-02-test-coverage-cycle-2.md. - W-1 (QA): reorder OmdbContractTests.InitializeAsync so the global HttpClient.DefaultProxy mutation is the LAST operation. xUnit v2 does not call DisposeAsync when InitializeAsync throws, so loading the fixture file (which can throw) BEFORE mutating the global state guarantees that any failure happens before the global is touched. Avoids a proxy-leak scenario in fixture-missing deployments. - WA-01 (Business-Analyst): strengthen the WR-03 FindByUserIdAsync test to assert TokenSalt and ExpiryDate round-trip in addition to Id and UserId — brings it to parity with the FindByIdAsync test pattern so a predicate inversion that returned a wrong-but-same-UserId row would surface. Renamed to FindByUserIdAsyncAfterAddReturnsTokenWithExpectedFields. - WA-02 / W-2 (BA + QA consensus): add FindByUserIdAsyncForUnknownUserReturnsNull covering the null-return branch of FindByUserIdAsync — previously the only branch left without integration coverage. DAL Docker suite is now 12 tests (was 11). Rejected findings (verified against source — false positives or out of scope): TA-1 alternative FQN form does not compile in RefreshTokenDbInfo (sibling namespace shadows); W-3 trivial expression doesn't warrant a property-level unit test; DA-1 ctor Guid.Empty guard is pre-existing on master, out of scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Iter-3 audit-team re-review (4-of-5 reviewer consensus: business-analyst, QA-engineer, senior-developer, devil's-advocate) flagged a doc/code mismatch: the XML summary on FindByUserIdAsyncAfterAddReturnsTokenWithExpectedFields explicitly mentions "TokenHash / TokenSalt / ExpiryDate" but the assertion block omitted TokenHash. TokenHash is a security-relevant credential field; a mapper regression silently zeroing it on round-trip would pass the previous assertion set. Adding actualValue.TokenHash.Should().Be(...) brings the assertions in line with the documented intent. Full iter-3 cycle report at Notes/Reviews/phase-02-test-coverage-cycle-3.md (local; gitignored). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…NLog.config root cause (IN-02) - Remove `concurrentWrites="true"` from `Sources/Libraries/ProjectV.Logging/NLog.config`. NLog 6 dropped that attribute, and combined with `throwConfigExceptions="true"` it caused the auto-load to throw `NLog.NLogConfigurationException` whenever any production type with a static `NLog.Logger` field was touched inside a test process. - Hoist the empty-config `[ModuleInitializer]` body into `ProjectV.Tests.Shared.ForTests.TestModuleInitializer` so every test assembly that references Tests.Shared (all of them) inherits the suppression for free via the global-using of that namespace. - Delete the 12 per-assembly `*ModuleInitializer.cs` / `*TestsInit.cs` files that used to carry an identical body (Appraisers, CommunicationWebService, Core, Crawlers, DataPipeline, Executors, InputProcessing, OmdbService, OutputProcessing, SteamService, TelegramBotWebService, TmdbService). - Update three test-class XML-doc comments that previously `<see cref>`'d the deleted per-assembly initializer types (OutputManagerTests, InputManagerTests, CrawlersManagerTests) to point at the hoisted `ProjectV.Tests.Shared.ForTests.TestModuleInitializer` in prose form. Build + Unit/Contract/Integration tests still 170 green; format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Create `Sources/Tests/ProjectV.Tests.Shared/Helpers/Http/FakeHttpMessageHandler.cs` as the single shared `DelegatingHandler` test seam (public sealed; ctor takes a `Func<HttpRequestMessage, HttpResponseMessage>` responder; exposes a read-only `CallCount`; `SendAsync` invokes the responder, attaches the inbound request to the produced response via `RequestMessage`, and returns `Task.FromResult`). - Replace the two identical private nested `FakeHttpMessageHandler` declarations in `CommunicationServiceClientTests.cs` and `HttpClientPollyPolicyTests.cs` with `using ProjectV.Tests.Shared.Helpers.Http;` directives. No call-site changes — the public surface (ctor signature, `CallCount` property) matches the original. Preserves the existing comment block about "do not mock HttpMessageHandler with NSubstitute (RESEARCH.md Pitfall 6)" by carrying it into the hoisted file's XML doc. - Drop redundant `using System.Threading;` from the two callers (now provided by Tests.Shared `SharedUsings.cs` globals). Core.Tests still 25/25 passing; build + format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add `userId.ThrowIfEmpty(nameof(userId))` to the `RefreshTokenDbInfo` constructor so the model rejects empty user IDs at construction. The production mapper `DataAccessLayerMapper.MapToRefreshTokenInfo` already calls `UserId.Wrap` upstream which guards against `Guid.Empty`, and `UserIdGenerator.GenerateUserId()` uses `Guid.NewGuid()` and never produces `Guid.Empty` — so this is a self-consistency guard on the model itself rather than a new functional change. Carried out of the Phase 2 review-loop iter-2 audit-team Devil's-Advocate suggestion (DA-1), which the review-loop deferred as a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ync (QA S-1) Add `FindByUserIdAsyncWithMultipleUsersReturnsOnlyMatchingRow` to `DatabaseRefreshTokenInfoServiceTests`: insert tokens for two distinct users, call `FindByUserIdAsync(tokenA.UserId)`, assert the returned record's `Id == tokenA.Id` and `Id != tokenB.Id`. The pre-existing single-row happy-path test would still pass even if the EF-translated WHERE clause were a no-op (only one candidate row could surface); the multi-row variant exercises the actual filter — a regression that broke the predicate would return the wrong row here. Carried out of the Phase 2 review-loop iter-3 QA suggestion (QA S-1). DAL Docker test suite now 13/13 (was 12, +1 new). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ments (iter-1) Iter-1 gsd-code-review on Plan 02-13 diff (WR-01 + IN-02 applied; IN-01 theoretical race rejected — same shape as prior Round-1 yielded refutation). - WR-01: RefreshTokenDbInfo ctor — document that EF Core 10 uses this constructor for entity materialization via parameter-name matching, so the ThrowIfEmpty / ThrowIfNullOrWhiteSpace guards fire on both writes (domain construction) and reads (DB row materialization). The defense-in-depth is intentional (matches token-issuance invariants); data-repair reads of corrupt rows must fix the SQL first. - IN-02: TestModuleInitializer XML doc — replace "handful of early log lines" with a more precise description of the race window between process start and the first Tests.Shared symbol use, including a follow-up path if stray log writes ever become unacceptable (per-assembly initializers or a startup hook). The global-using directive is compile-time only — it does not force Tests.Shared to load, which is the root of the window. No code-behavior changes; both edits are documentation only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Iter-2 audit-team re-review surfaced 10 APPLY-classified comment fixes (see Notes/Reviews/phase-02-test-coverage-13-cycle-2.md for the full verdict table). Multi-reviewer consensus on technical-accuracy edits; single-reviewer-but-substantive edits where DA's primary-source tracing held up; 4 single-reviewer pedantic findings rejected. RefreshTokenDbInfo.cs: - EF binding matches CLR property names, not column names (BA + TA) - "EF Core" (stable since 2.1), not "EF Core 10" (TA + Senior-Dev) - "Both/and" lowercased (Senior-Dev) - Scoped invariant claim — this ctor IS the sole enforcement, no DB CHECK constraint, no independent RefreshTokenInfo domain-model guard (DA: traced via UsersController.Login -> TokenService -> domain model) - Added: Ts/ExpiryDate carry no ctor-level guard; temporal invariants not checked here (QA) - Added: DeleteAsync also routes through FindByIdAsync so corrupt rows cannot be deleted via the service either (QA) TestModuleInitializer.cs: - "no longer throws" scoped to the concurrentWrites cause; flagged that throwConfigExceptions + extensions still gates other failure modes (DA) - Added explicit "do not re-add concurrentWrites" guard paragraph (DA) - Dropped ambiguous "startup hook" mechanism name; kept only the unambiguous per-assembly [ModuleInitializer] follow-up path (TA) - "initializes" (American spelling, BA + Senior-Dev) No code changes; both edits are documentation only. Build green; 171 tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…er-3) Iter-3 final audit-team caught two factual errors in the iter-2 ctor comment (4-of-5 reviewer consensus on the first, Devil's-Advocate primary-source trace on the second): 1. "reads, updates, and deletes all route through `FindByIdAsync`" was wrong about `UpdateAsync` — the service's `UpdateAsync` calls `_mapper.MapToRefreshTokenDbInfo(tokenInfo)` directly on the input domain object and then `dbSet.Update(...)`; no FindByIdAsync call. Rewritten to "reads and deletes route through FindByIdAsync; updates are protected by the domain-layer input UpdateAsync receives." 2. "no independent guard in the `RefreshTokenInfo` domain model" was wrong — `RefreshTokenInfo.cs:30` calls `tokenSalt.ThrowIfNull(nameof(tokenSalt))`, and Id/UserId are value-object types whose `.Wrap()` factories already enforce `ThrowIfEmpty`. Rewritten to "the parallel RefreshTokenInfo domain model carries its own value-object and null guards on the write path" so the ctor's role is the EF-materialization-path enforcement (not the sole enforcement everywhere). Rejected finding (preserved in cycle-3 notes): "12 downstream test assemblies" count — reviewers misread the past tense; 12 matches the iter-1 file deletions. Comment-only change; no code behavior shift; build green; 171 tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Vasar007
added
type: Code Maintenance
Vasar007
commented
May 23, 2026
Owner
Author
Vasar007
left a comment
Vasar007
left a comment
There was a problem hiding this comment.
Review round 1 is done. Let's start with fixing this comments first.
| if: matrix.os == 'ubuntu-latest' | ||
| run: dotnet test Sources/Tests/ProjectV.ContentDirectories.Tests/ProjectV.ContentDirectories.Tests.fsproj --configuration Release --no-build -p:Platform=x64 | ||
|
|
||
| # Coverage publication (D-24, D-26): merge per-stage Cobertura outputs into |
Owner
Author
There was a problem hiding this comment.
[ToFix] Fix and remove D-NN and all other identifiers which we don't store and repositories.
Owner
Author
There was a problem hiding this comment.
[Repetative] Fix all over the new code. Verify D-* T* I-* and other identifiers won't be in this PR.
Owner
Author
There was a problem hiding this comment.
[AI] [T#3276812772]
Fixed in d542967: removed all D-NN-style decision IDs and other internal identifiers from .github/workflows/build.yml comments — lines 52–53, 67, 72, and 94 are now plain-English descriptions of what each CI stage does. See commit d542967 for the full diff. Note this same scrubbing pattern is applied across the whole PR diff for consistency.
Owner
Author
There was a problem hiding this comment.
[AI] [T#3276816973]
Fixed in d542967: (a) replaced TEST-01 header and verbatim-quote block with a plain-English "Purpose" section; (b) removed all .planning/ cross-reference links from the Cross-references section at the bottom (.planning/codebase/ARCHITECTURE.md, .planning/codebase/INTEGRATIONS.md, .planning/REQUIREMENTS.md) — now references only publicly-committed Docs paths; (c) also aligned the Legend table column widths and removed D-NN/02-NN-SUMMARY.md references from Status cell prose in the Domain, Application, and Infrastructure tables. See commit d542967 for the full diff. Note this same scrubbing pattern is applied across the whole PR diff for consistency.
|
|
||
| ## Domain Layer | ||
|
|
||
| | Path | Component | Planned Test Project | Test Type | Status | Test Files | |
Owner
Author
There was a problem hiding this comment.
[ToFix] [Repetative] Align the tables in the markdown files. I mean all md files added in this PR.
Owner
Author
There was a problem hiding this comment.
TEST-NN also should be replaced with some generic identifies, words or statements.
Owner
Author
There was a problem hiding this comment.
And you also mentioned .planning folder, remove all mentions.
Owner
Author
There was a problem hiding this comment.
[AI] [T#3276846643]
Fixed in d542967: removed Plan 02-09 Task 1 Rule 1 fix from the inline comment in DatabaseRefreshTokenInfoService.FindByUserIdAsync (line 64). The comment now reads as a plain-English explanation of the EF Core translation constraint — without referencing any plan, task, or rule ID. See commit d542967 for the full diff. Note this same scrubbing pattern is applied across the whole PR diff for consistency.
|
|
||
| ## Maintenance | ||
|
|
||
| Downstream Phase 2 plans update this document in step with the test files they |
Owner
Author
There was a problem hiding this comment.
[ToFix] [Repetative] Again, you should not mention Phases, Tasks and other non-commited artifacts we don't have committed in this repository. GSD artifacts are stored only locally.
Owner
Author
There was a problem hiding this comment.
[AI] [T#3276831075]
Fixed in d542967: removed all "Phase 2 plans", "Downstream Phase 2", "02-NN-SUMMARY.md" references and .planning/ links from the Maintenance section and Cross-references section of Docs/Testing/Coverage/test-coverage.md. The Maintenance section now describes the update process in plain English without referencing phases or tasks; the Cross-references section now only links to publicly-committed files under Docs/ rather than .planning/ paths. See commit d542967 for the full diff. Note this same scrubbing pattern is applied across the whole PR diff for consistency.
| @@ -0,0 +1,3 @@ | |||
| using System.Runtime.CompilerServices; | |||
|
|
|||
| [assembly: InternalsVisibleTo("ProjectV.OmdbService.Tests")] | |||
Owner
Author
There was a problem hiding this comment.
[ToFix] [Repetative] Remove all AssemblyInfo.cs files. Use attributes in csproj:
We use the value of AssemblyName to declare the value of the attribute. You can use the specifc name, not the variable.
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net10.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute">
<_Parameter1>$(AssemblyName).Tests</_Parameter1>
</AssemblyAttribute>
</ItemGroup>
</Project>
Owner
Author
There was a problem hiding this comment.
[AI] [T#3276891245]
Fixed in 0d93afe: removed 3 AssemblyInfo.cs files (and their now-empty Properties/ directories) across OmdbService, SteamService, and TmdbService. The InternalsVisibleTo declarations now live as <AssemblyAttribute> items in each project's .csproj. Build and unit tests green.
| @@ -0,0 +1,28 @@ | |||
| Reserved for EF Core migrations. | |||
Owner
Author
There was a problem hiding this comment.
[ToDiscuss] Why do we need this file?
Owner
Author
There was a problem hiding this comment.
And we have this file in other projects too (some of them are empty). Why?
Owner
Author
There was a problem hiding this comment.
[AI] [T#3276910502]
Audited the 8 .gitkeep files added in this PR. Policy applied: keep only when the directory must persist in git AND has no other tracked content AND has a concrete near-term consumer.
Resolution (fixed in f9449d5):
- Removed (7):
Sources/Tests/Fixtures/{Omdb,Steam,Tmdb}/.gitkeep(each dir already holds real JSON fixtures),Sources/Tests/ProjectV.Tests.Shared/Helpers/{Stubs,WebApi}/.gitkeep(each dir holds real files after the recent test infra changes), andSources/Tests/ProjectV.Tests.Shared/Helpers/{Extensions,Persistence}/.gitkeep(aspirational empty placeholders with no current consumer — re-add when actual content lands). - Kept (1):
Sources/Libraries/ProjectV.DataAccessLayer/Migrations/.gitkeep— the directory is the target for the generated EF Core migration classes once issue refactor(dal): unblock EF Core migration generation — ProjectVDbContext model validation #346 unblocks the work; until then, the.gitkeepis what keeps the directory in git so the migration tooling has a stable target.
| <target name="logfile" | ||
| xsi:type="File" | ||
| fileName="${specialfolder:folder=CommonApplicationData}/ProjectV/logs/${shortdate:universalTime=false}.log" | ||
| concurrentWrites="true" |
Owner
Author
There was a problem hiding this comment.
[ToDiscuss] Explain why did you remove this.
Owner
Author
There was a problem hiding this comment.
[AI][Draft] [T#3292733451]
The removal was intentional and the rationale is spelled out in commit 4dae754: NLog 6 dropped the concurrentWrites attribute entirely. With throwConfigExceptions="true" still present in NLog.config, NLog 6 was treating the now-unknown attribute as a configuration error and throwing NLogConfigurationException whenever any production type with a static NLog.Logger field was first touched inside a test process. Removing the attribute stops that throw.
A note on the production concern the reviewer may be raising: concurrentWrites="true" in NLog 5 and earlier put a per-write file mutex on the target so multiple processes could append to the same dated log file safely. The file target here resolves to ${CommonApplicationData}/ProjectV/logs/${shortdate}.log, which is a shared path across all WebServices running on the same host. Since NLog 6 no longer supports the attribute, any multi-process write safety for that shared path would need to be addressed differently (e.g. per-service log file names via a ${processname} variable) — but that is a pre-existing deployment concern, not introduced by this PR.
The TestModuleInitializer XML-doc in Sources/Tests/ProjectV.Tests.Shared/ForTests/TestModuleInitializer.cs also explicitly documents "Do NOT re-add concurrentWrites=\"true\"" for this reason.
Draft pending author confirmation.
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="BaseMockTest" /> class. | ||
| /// </summary> | ||
| protected BaseMockTest() |
Owner
Author
There was a problem hiding this comment.
[ToFix] Add IFixture Fixture property initialized from a new static factory, and use it instead of CreateMock method.
Owner
Author
There was a problem hiding this comment.
[AI] [T#3292738367]
Fixed in b26115f: BaseMockTest now exposes an IFixture Fixture property initialised from a new static CreateFixture factory wired with AutoNSubstituteCustomization, plus AutoFixture + AutoFixture.AutoNSubstitute are added to Central Package Management and the shared test build props so every test project inherits them transitively. Subclasses now resolve substitutes via Fixture.Create<T>() / Fixture.Freeze<T>() instead of bare Substitute.For<T>(). The sweep across all new test classes lands in be7bbde.
| /// and <see cref="TestAppraiserBuilder" /> for substitute children. | ||
| /// </summary> | ||
| [Trait("Category", "Unit")] | ||
| public sealed class AppraisersManagerTests |
Owner
Author
There was a problem hiding this comment.
[ToFix] [Repetition] Here and in many other tests you violated the skill how to create tests:
- no base class (BaseTest, BseMockTest or any other)
- used Substitute instead of BaseMockTest.Fixture
- didn't use one private methods per test mock builder (e.g.
TestAppraisersManagerBuilderin this case) - didn't use test data generators for the values, you generated them in place.
Fix all of these for ALL tests (align the ones I had before this PR too if needed).
Owner
Author
There was a problem hiding this comment.
[AI] [T#3292740516]
Fixed in be7bbde: every new test class on this branch now inherits BaseMockTest (directly, or transitively via WebApiBaseTest / BaseExceptionTests, which now chain to BaseMockTest as well). The six classes that previously called Substitute.For<T>() inline (AppraisersManagerTests, CommunicationServiceClientTests, ShellBuilderDirectorTests, DataflowPipelineTests, InputManagerTests, OutputManagerTests) were rewritten to resolve their substitutes through Fixture.Create<T>() (chosen over Freeze so tests that need fresh per-method substitutes still work). Per-class private BuildSut helpers were added where the SUT construction benefited from a per-class factory. Build green, Unit suite 129/129 and Contract suite 7/7 pass (Integration skipped — Docker).
| // Act. | ||
| var act = () => | ||
| { | ||
| #pragma warning disable CS8625 // Cannot convert null literal to non-nullable reference type. |
Owner
Author
There was a problem hiding this comment.
[ToFix] DO not use pragma disable for null validation, use operator ! instead. It's okay to use it in such cases.
Owner
Author
There was a problem hiding this comment.
[AI] [T#3292741148]
Fixed in b26115f: all 24 #pragma warning disable CS8625 blocks used to feed null into non-null parameters in null-validation tests across 14 new test files were replaced with the null-forgiving (!) operator (null!). Every pragma block in the new test code is gone — the suppression directive is no longer needed because null! expresses the intent inline.
| using Acolyte.Assertions; | ||
| using ProjectV.Appraisers; | ||
|
|
||
| namespace ProjectV.Tests.Shared.Helpers.Mocks.Appraisers |
Owner
Author
There was a problem hiding this comment.
[ToFix] How does this exist in Mocks folder but it ain't a mock builder? You can move it to Stubs folder instead, or suggest a better alternative.
Mocks folder for the code which accepts Fixture and create mocks using it.
Owner
Author
There was a problem hiding this comment.
[AI] [T#3292744028]
Fixed in b26115f: TestAppraisersManagerBuilder, plus five sibling builders that are structurally identical (zero Substitute.For<> calls, build real concrete objects through their public APIs), were moved out of Helpers/Mocks/ into Helpers/Stubs/ with mirrored sub-folder structure and namespace updates. Specifically the relocations are TestAppraisersManagerBuilder → Stubs/Appraisers/, TestShellBuilder → Stubs/Core/, TestInputManagerBuilder + TestCrawlersManagerBuilder + TestOutputManagerBuilder → Stubs/Managers/, and TestDataflowPipelineBuilder → Stubs/DataPipeline/. The five builders that DO use Substitute.For (TestAppraiserBuilder, TestCommunicationServiceClientBuilder, TestOmdbCrawlerBuilder, TestSteamCrawlerBuilder, TestTmdbCrawlerBuilder, TestTelegramBotClientBuilder) stay in Mocks/ — they are real mock builders and belong there. All caller using-statements were updated accordingly.
…iles Replaces internal-pipeline identifiers (decision IDs, plan IDs, .planning/ paths, phase nomenclature, requirement IDs) with public identifiers or plain-English descriptions across .github/workflows/build.yml, Docs/Testing/Coverage/test-coverage.md, and Sources/Libraries/.../DatabaseRefreshTokenInfoService.cs. Also removes .planning/ cross-reference links from test-coverage.md and normalises the Legend table column widths. Addresses 4 threads on PR #342. Co-Authored-By: Claude <noreply@anthropic.com>
The ARCHITECTURE.md / INTEGRATIONS.md refs left over from the prior scrub still pointed at gitignored files. Removed; the remaining Docs/-rooted cross-references are sufficient on their own. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…emblyAttribute> Replaces the AssemblyInfo.cs files added during the test-coverage build-out with equivalent <AssemblyAttribute> MSBuild items in each .csproj. The InternalsVisibleTo target assembly is unchanged. Addresses 1 thread on PR #342. Co-Authored-By: Claude <noreply@anthropic.com>
… factory The design-time factory used by `dotnet ef migrations` previously fell back to a hardcoded local-dev connection string when the DatabaseOptions__ConnectionString env var was unset. Now it throws InvalidOperationException with a clear setup message naming the env var and showing an example value, so the problem is immediately obvious rather than silently using wrong credentials. Also removes a stale internal-artifact reference from the CanUseDatabase comment, and adds a short EF Core migrations setup note to the top-level README. Addresses 1 thread on PR #342. Co-Authored-By: Claude <noreply@anthropic.com>
… null-validation pattern Adds an IFixture Fixture property to BaseMockTest (initialized from a new static CreateFixture factory wired with AutoNSubstituteCustomization) so test classes can resolve substitutes through the base instead of bare Substitute.For<T>. Brings AutoFixture + AutoFixture.AutoNSubstitute into Central Package Management and the shared test build props so every test project inherits them. Moves the six non-Fixture builder classes out of Helpers/Mocks/ into Helpers/Stubs/ to match their actual role (they build real concrete objects, not NSubstitute proxies). Rewrites the #pragma warning disable nullability suppressions used in null-validation tests to the null-forgiving (!) operator, dropping the pragma blocks entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…new test classes Migrates every new test class added on this branch to inherit from BaseMockTest (directly, or transitively via WebApiBaseTest / BaseExceptionTests, which now chain to BaseMockTest as well), replaces bare Substitute.For<TDep>() calls with Fixture.Create<TDep>() (per-test fresh class instance keeps NSubstitute proxies isolated), introduces a private BuildSut helper per test class where the SUT construction benefits from a per-class factory, and explicitly imports ProjectV.Tests.Shared.ForTests in consuming files because global usings do not cross assembly boundaries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes seven .gitkeep files where the parent directory either now has
real content (Sources/Tests/Fixtures/{Omdb,Steam,Tmdb}, the relocated
stub builders under Helpers/Stubs, and Helpers/WebApi) or was an
aspirational empty placeholder with no current consumer
(Helpers/Extensions, Helpers/Persistence).
The single .gitkeep retained is the one in
Sources/Libraries/ProjectV.DataAccessLayer/Migrations — the EF migration
generation work tracked on GitHub issue #346 is the natural consumer.
Addresses 1 thread on PR #342.
Co-Authored-By: Claude <noreply@anthropic.com>
… in committed code Removes leftover decision-ID, plan-filename, requirement-ID, review-finding-ID, and gitignored-pipeline path references from XML doc comments, inline comments, and markdown content across 65 files. Also replaces one surviving #pragma warning disable CS8625 with the null-forgiving operator, matching the test infrastructure refactor's pattern. No runtime behaviour change. Addresses 6 findings from the phase-wide cross-cutting code review. Co-Authored-By: Claude <noreply@anthropic.com>
…ln header The cross-cutting sweep missed this one solution-file comment because the Desktop solution was outside the 108-file phase-touched scope. Same scrub rule applies: no internal-pipeline identifiers in committed files. Co-Authored-By: Claude <noreply@anthropic.com>
Owner
Author
|
[AI] PR review-processing pass complete — summary All 13 reviewer comments on this PR have been processed in 6 batches following a fix-then-review-loop discipline (one fix-and-commit subagent per batch, then a 2-cycle review-loop with Per-batch outcomes (8 commits pushed)
Cross-cutting sweep (after the 6 batches)A phase-wide cross-cutting code review caught surviving internal-pipeline identifiers in files that were outside any single batch's scope (decision-ID tokens in XML doc comments, leftover plan-file references in inline
What still needs your inputTwo threads were tagged
Verification at HEAD (
|
The test-infrastructure refactor in be7bbde removed callsites that needed these usings but left the directives themselves in place. CI on PR #342 flagged both via IDE0005 (TreatWarningsAsErrors promotes it to an error under dotnet format --verify-no-changes). Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.propsinjects the shared C# test stack (xunit + AwesomeAssertions + NSubstitute + coverlet + opt-in TimeProvider.Testing / Testcontainers / WireMock.Net / AspNetCore.Mvc.Testing) into every.csprojunderSources/Tests/while leaving the F#ProjectV.ContentDirectories.Tests.fsprojuntouched (D-04). New libraryProjectV.Tests.Sharedexposes the base test classes (BaseTest,BaseMockTest,BaseDependencyInjectionTests,BaseExceptionTests<T>,TestTimeHelper), the first generator + mock-builder examples, and aFixtureLoader. ExistingProjectV.Appraisers.Tests+ProjectV.Common.Testsretrofitted to AwesomeAssertions; the previously skippedModelSerializationTestsnow 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.mdenumerates the critical paths across Domain / Application / Infrastructure and maps each row to the plan that covers it (TEST-01).Docs/Testing/scenarios.mddescribes 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.ymlis split from a single omnibus C# test step into four named Linux stages —Test (Unit)/Test (Integration)/Test (Contract)/Test (F#)— plus a WindowsTest (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.runsettingsexcludes 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,BasicInforound-trips) andProjectV.Appraisers(rating-computation accuracy on the canonicalAppraiser<T>+IAppraisal<T>compositions andAppraisersManager). 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.Testsproject coversShell+ShellBuildercomposition +CommunicationServiceClient+ Polly retry policies. New mock builders forShell,ServiceClient, andIO/Crawlersmanagers added toTests.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)
DataflowPipelineintegration 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 realTMDbLib/OMDbApiNet/SteamWebApiLibHTTP 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.CommunicationWebServicecovered viaWebApplicationFactory+ aTestJwtHelper. NewWebApiBaseTestbase class lands inTests.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).
TestWebApplicationFactoryextended withITelegramBotClient+ICommunicationServiceClientstubs and a newTestTelegramBotClientBuilderadded toTests.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.TelegramBotWebServicepolling 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:
[ModuleInitializer]dedup; NLog test initializer hoisted toTests.Shared.FakeHttpMessageHandlerhoisted toTests.Shared(was duplicated across contract suites).RefreshTokenDbInfoctor guards againstGuid.Empty.FindByUserIdAsync(withTokenHashassertion 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
Shell/ShellBuilder/DataflowPipeline/ExecutorsTest Plan
dotnet restore Sources && dotnet build Sources/ProjectV.sln -c Release -p:Platform=x64→ 0 warnings / 0 errors.dotnet format Sources/ProjectV.sln --severity warn --verify-no-changes→ exit 0.dotnet test Sources/ProjectV.sln -c Release -p:Platform=x64 --no-build --filter "Category=Unit"→ pass locally; gated in CI asTest (Unit).dotnet test Sources/ProjectV.sln -c Release -p:Platform=x64 --no-build --filter "Category=Integration"→ pass; gated in CI asTest (Integration).dotnet test Sources/ProjectV.sln -c Release -p:Platform=x64 --no-build --filter "Category=Contract"→ pass; gated in CI asTest (Contract).dotnet test Sources/Tests/ProjectV.ContentDirectories.Tests/ProjectV.ContentDirectories.Tests.fsproj -c Release -p:Platform=x64 --no-build→ 9 / 9 pass; gated in CI asTest (F#).windows-latest— unit + contract only; integration suite skipped on Windows by design (Testcontainers not in scope for Windows runner).coverage-reportartifact. No hard coverage gate; visibility only (TEST-06).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):
ProjectV.Tests.Sharedis a single shared library housing base test classes, generators, and mock builders. Every test project underSources/Tests/references it; no per-project duplication.Sources/Tests/Directory.Build.props, conditioned on$(MSBuildProjectExtension) == '.csproj'so the F# project keeps its self-contained Unquote stack untouched (D-04).ProjectV.ContentDirectories.Tests.fsprojretains its own stack and runs in its own CI stage. Reaffirmed from Phase 1.Sources/Tests/Fixtures/{Tmdb,Omdb,Steam}/as checked-in recorded JSON. No live API calls in CI.Sources/Tests/coverlet.runsettings; covers generated EF migrations + auto-generated NSwag clients.5e8c3af/ae883e1/f0b1602).FakeHttpMessageHandlerhoisted toTests.Sharedrather than left duplicated per project (Plan 02-13).RefreshTokenDbInfoctor rejectsGuid.Emptyto make the invariant explicit at the domain layer (Plan 02-13).Out of Scope
Tracked but explicitly deferred:
Shell/ShellBuilder/ Dataflow pipeline — additive phase, no redesign.Closing Issues
Closes #341