This repository was archived by the owner on Jun 5, 2026. It is now read-only.
chore(mutation): add per-BC Stryker tooling and exclude i18n registration from scope#158
Merged
Merged
Conversation
…tion from scope Adds a local dotnet-stryker tool manifest and a solution-less audit config so mutation testing can run per bounded context (the committed config's single solution can't build the web workspaces in CI-less environments). Excludes Domain/Errors/*Translations.cs from the mutate scope: these are pure FR/EN string-registration tables with no business logic, mirroring the existing SharedKernel/Localization exclusion.
…->100% Covers previously unexercised domain logic through sociable use-case scenarios where reachable (IngestRunOutcome: empty/invalid/out-of-order levers; ComputeWeeklySnapshots: skip metrics entry with no identifiable user) and through targeted value-object tests for boundary invariants that commands carry pre-constructed (IsoWeek bounds/rollover, StopsMetrics non-negative guards, LeverImpactEstimate finiteness, WowCharacterRef region rules, UseCaseKey kebab-case, strongly-typed id guards and equality components). Remaining survivors are equivalent mutants (ArgumentNullException.ThrowIfNull guards and guards made redundant by a following validation).
…%->100% Hotfix.Create branches are already covered through the use case; the remaining gap was the strongly-typed HotfixId (empty-guard, equality components) which a command carries pre-constructed, so it is tested directly as a boundary VO.
…undary VO tests Adds sociable scenarios on the use cases (AddPhase invalid ordinal / ordering / event, AttachUseCase invalid kind / event) and direct tests for logic no use case reaches: Encounter.Create explicit provenance and the UpdateUseCase aggregate method (not-found, invalid kind, unknown phase, manual-override protection, in-place replace). Boundary value objects (BlizzardEncounterId, EncounterName, EncounterId, UseCaseKey, PatchVersion, Provenance) get targeted tests since commands carry them pre-constructed.
Adds a direct BehavioralPatternCandidate test: the Raise factory validation matrix (pattern kind, encounter/difficulty, evidence spells, lift, support, supporting runs), MinedUseCaseKey encoding, and the Accept/Merged/Reject state machine with events. These run through the aggregate because no use case can drive invalid mining evidence (the algorithm only emits valid candidates). Application stays at 89% by design: the remaining undetected mutants are the documented ResolveEncounterIdAsync stub (attach-dispatch path is dead until the EncounterKnowledge read port is wired) and defensive guards against states the mining flow cannot produce (supportAll==0 with top samples being a subset of all; Raise failing on already-validated inputs).
Adds targeted value-object tests for the boundary invariants commands carry pre-built (CohortKey dimensions + equality, MetricKey kebab-case, MetricPercentile monotonicity with equal-adjacent boundaries and DeltaScore including the degenerate zero-spread case, CohortId guard). Adds a sociable RebuildCohorts scenario (target with an undefined tier is skipped) covering the Cohort.Create failure branch. Removes two provably-dead guards in RebuildCohorts (percentiles.Count==0 is unreachable since a non-empty sample always yields at least one percentile and Cohort.Create already rejects empty percentiles via EmptySample; sorted.Length==0 is unreachable since a GroupBy group is never empty) and the redundant sample-size ternary whose else branch could never run.
Covers the previously-untested core of the ingestion pipeline with sociable tests driving the application services directly: the Raid and Mythic+ ingestion strategies (wrong-input, duplicate fingerprint, invalid player, persisted session/run detail, per-pull event batching, malformed-pull skipping, Mythic 0 keystone exemption, completed vs abandoned outcome), SegmentHasher determinism/discrimination/canonicalisation, the resumable-upload completion handler (success, typed-error persistence, gzip decompression) and the ListUserUploads / GetRaidSessionPlayerNames queries. Remaining survivors are equivalent mutants (StreamReader BOM/buffer/leaveOpen flags, SHA payload prefix constants) and a few weak spots in the pre-existing UploadCombatLogUseCase dedup path.
Adds targeted tests for previously-uncovered value objects: ShareToken (length + url-safe char ranges + trim + equality), SegmentHash (length + lowercase hex-only char ranges + equality), UploadJournalEntry (Create + FromSnapshot invariants + snapshot roundtrip) and PlayerLoadout (component equality, defensive input copy). Remaining survivors are diffuse and largely equivalent (SeasonSlug normalization regex internals, ArgumentNullException guards on aggregate factories, DungeonRun Complete/Abandon branches already exercised via snapshot roundtrip).
Adds targeted value-object tests (PatchVersion 3/4-part parse + negative components + revision-aware equality, SpellAttributes group-count guard + interruptibility bit decode + HasFlag bounds + equality, SpellName, SpellTag kebab-case, BlizzardSpellId, SpellId). Remaining survivors are in the Spell and WagoSpellRecord aggregates and the SpellSchool mask VO (follow-up).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds extensive unit tests across domains, tweaks cohort percentile/sample-size logic, and introduces Stryker/.NET tooling manifests. ChangesDomain Value Object and Use Case Test Coverage
Mutation Testing Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dotnet-tools.json`:
- Line 6: The dotnet-tools.json entry for dotnet-stryker specifies an
unavailable version; update the "version" field for dotnet-stryker from "4.14.2"
to a valid published version (e.g., "4.14.1") so the tool can be installed;
locate the dotnet-stryker entry and change the "version" value accordingly.
In
`@src/Coaching/Coaching.Application.UnitTests/DomainTests/StopsMetricsTests.cs`:
- Around line 7-10: The code creates and fills an int array with new int[12]
plus a loop; replace that with a C# collection expression to initialize the
12-element array concisely and then set the single negative element: construct v
using a collection expression (e.g., an array literal or repeated value
expression) instead of new int[12] and the for-loop, then assign v[index] = -1;
update the variable v and preserve the existing use of index and its bounds.
In
`@src/Comparison/Comparison.Application/UseCases/Cohorts/Commands/RebuildCohorts/RebuildCohortsUseCase.cs`:
- Around line 48-50: The use case currently computes sampleSize and percentiles
(lines calling samples.GroupBy(...).Max(...) and ComputePercentiles(...)) which
is business logic that must be moved to the domain; extract the sample-size and
percentile policy into a domain service or aggregate method (e.g.,
ICohortPolicyService.GetSampleSizeAndPercentiles or
CohortAggregate.CalculatePercentiles) and implement the logic there (also move
the logic present around lines 83-99). Replace the inline calls in
RebuildCohortsUseCase with a single call to the new domain service via
constructor-injected interface, returning the sampleSize/percentiles DTO for the
use case to orchestrate only.
In
`@src/LogIngestion/LogIngestion.Application.UnitTests/Domain/LogIngestionBoundaryVoTests.cs`:
- Line 172: Replace the current implicit-typed initialization of the items
variable in LogIngestionBoundaryVoTests with a C# 12 collection expression:
change the declaration of the items variable (currently using "new
List<EquippedItem> { new(1, 600) }") to an explicit List<EquippedItem>
declaration using the C#12 collection expression syntax for a single
EquippedItem instance; update the variable name "items" and ensure the type is
List<EquippedItem> and the element uses the existing EquippedItem constructor
call pattern.
In
`@src/LogIngestion/LogIngestion.Application.UnitTests/Ingestion/LogIngestionStrategyTests.cs`:
- Around line 262-268: Replace the loose IsFailure assertion in the test
Should_FailWithInvalidKeyLevel_When_KeyLevelBelowMinimum (and the similar test
at 271-277) with an assertion that verifies the specific failure key returned by
the strategy; e.g., after calling MplusStrategy().ExecuteAsync(Mplus(keyLevel:
1), ...) assert that result.Error.Key (or the code property your Result type
exposes) equals the expected validation key constant (for example
ValidationErrors.InvalidKeyLevel or the project's equivalent), so the test
checks the exact validation path rather than any failure.
- Around line 65-68: Replace the old array initializers with C# 12 collection
expressions for the test fixtures: change "new IngestionPull[] { ... }" to
"IngestionPull[] pulls = [ ... ]" (and similarly for the other occurrences at
the indicated blocks), ensuring inner collections like the IngestionCombatant
list use collection expressions as well (e.g., replace "new(...) , []" style
inner arrays with "[ new IngestionCombatant(...), ... ]"); update the
declarations for variables named pulls (and other fixtures at the other
locations) so the compiler infers the array from the square-bracket collection
expression while keeping the same constructors and element values for
IngestionPull and IngestionCombatant.
In
`@src/LogIngestion/LogIngestion.Application.UnitTests/Queries/GetRaidSessionPlayerNamesQueryTests.cs`:
- Around line 16-24: The test
Should_ReturnDistinctPlayerNames_When_SessionExists currently only asserts
presence of "Thrall"; add an explicit uniqueness check on the returned
collection to enforce distinctness: after calling _query.HandleAsync and
extracting names via ThenResultIsSuccess, assert that names.PlayerNames has no
duplicates (e.g., compare Count to distinct count or use a uniqueness assertion)
so the test validates distinct player names returned by
GetRaidSessionPlayerNamesRequest handling.
In
`@src/LogIngestion/LogIngestion.Application.UnitTests/UseCases/CompleteResumableUploadHandlerTests.cs`:
- Line 42: Replace the explicit constructor call for the metadata dictionary
with a collection initializer expression: in CompleteResumableUploadHandlerTests
change the variable metadata (currently declared as var metadata = new
Dictionary<string, string>();) to use a collection-expression initializer (e.g.,
var metadata = new Dictionary<string, string> { }; or include entries like {
["key"] = "value" } as needed).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 06b8a50a-47d2-4475-9006-5e20c3167735
📒 Files selected for processing (26)
dotnet-tools.jsonsrc/Coaching/Coaching.Application.UnitTests/DomainTests/CoachingIdTests.cssrc/Coaching/Coaching.Application.UnitTests/DomainTests/IsoWeekTests.cssrc/Coaching/Coaching.Application.UnitTests/DomainTests/LeverImpactEstimateTests.cssrc/Coaching/Coaching.Application.UnitTests/DomainTests/StopsMetricsTests.cssrc/Coaching/Coaching.Application.UnitTests/DomainTests/UseCaseKeyTests.cssrc/Coaching/Coaching.Application.UnitTests/DomainTests/WowCharacterRefTests.cssrc/Coaching/Coaching.Application.UnitTests/Profiles/Commands/IngestRunOutcomeUseCaseTests.cssrc/Coaching/Coaching.Application.UnitTests/Progression/Commands/ComputeWeeklySnapshotsUseCaseTests.cssrc/Comparison/Comparison.Application.UnitTests/Cohorts/Commands/RebuildCohortsUseCaseTests.cssrc/Comparison/Comparison.Application.UnitTests/Cohorts/DomainTests/ComparisonValueObjectTests.cssrc/Comparison/Comparison.Application/UseCases/Cohorts/Commands/RebuildCohorts/RebuildCohortsUseCase.cssrc/EncounterKnowledge/EncounterKnowledge.Application.UnitTests/Encounters/Commands/AddPhaseUseCaseTests.cssrc/EncounterKnowledge/EncounterKnowledge.Application.UnitTests/Encounters/Commands/AttachUseCaseUseCaseTests.cssrc/EncounterKnowledge/EncounterKnowledge.Application.UnitTests/Encounters/DomainTests/EncounterAggregateTests.cssrc/EncounterKnowledge/EncounterKnowledge.Application.UnitTests/Encounters/DomainTests/EncounterValueObjectTests.cssrc/Hotfixes/Hotfixes.Application.UnitTests/Hotfixes/HotfixIdTests.cssrc/KnowledgeMining/KnowledgeMining.Application.UnitTests/Mining/BehavioralPatternCandidateTests.cssrc/LogIngestion/LogIngestion.Application.UnitTests/Domain/LogIngestionBoundaryVoTests.cssrc/LogIngestion/LogIngestion.Application.UnitTests/Ingestion/LogIngestionStrategyTests.cssrc/LogIngestion/LogIngestion.Application.UnitTests/Ingestion/SegmentHasherTests.cssrc/LogIngestion/LogIngestion.Application.UnitTests/Queries/GetRaidSessionPlayerNamesQueryTests.cssrc/LogIngestion/LogIngestion.Application.UnitTests/UseCases/CompleteResumableUploadHandlerTests.cssrc/SpellKnowledge/SpellKnowledge.Application.UnitTests/Spells/Domain/SpellKnowledgeValueObjectTests.csstryker-config.audit.jsonstryker-config.json
| "isRoot": true, | ||
| "tools": { | ||
| "dotnet-stryker": { | ||
| "version": "4.14.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest version of dotnet-stryker NuGet package and is version 4.14.2 available?
💡 Result:
Latest version of the dotnet-stryker NuGet package is 4.14.1 [1]. Yes—version 4.14.2 is available: I could not find dotnet-stryker 4.14.2 on NuGet; the NuGet Gallery page lists 4.14.1 as the latest version, and no 4.14.2 entry was surfaced in the results I checked [1].
Citations:
🌐 Web query:
dotnet-stryker 4.14.2 NuGet available?
💡 Result:
No. As of 2026-05-23, the NuGet package for dotnet-stryker shows 4.14.1 as the latest available version; I could not find a NuGet package entry for dotnet-stryker 4.14.2 on nuget.org. What I found: - NuGet Gallery lists dotnet-stryker version 4.14.1, with install commands for the.NET tool. [1] - The Stryker repository release/tag I found for dotnet-stryker is 4.14.1 (not 4.14.2). [2] A GitHub PR exists titled “Bump dotnet-stryker from 4.14.1 to 4.14.2”, but that is not proof that 4.14.2 has been published to NuGet. [3]
Citations:
- 1: https://www.nuget.org/packages/dotnet-stryker
- 2: https://github.com/stryker-mutator/stryker-net/releases/tag/dotnet-stryker%404.14.1
- 3: Bump dotnet-stryker from 4.14.1 to 4.14.2 b3b00/csly#609
Update dotnet-stryker in dotnet-tools.json—NuGet doesn’t publish 4.14.2
NuGet Gallery lists dotnet-stryker latest as 4.14.1 and there’s no published package entry for 4.14.2, so dotnet-tools.json specifying 4.14.2 will likely fail tool installation. Update version to 4.14.1 (or another version that’s actually on nuget.org). [1]
[1] https://www.nuget.org/packages/dotnet-stryker
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dotnet-tools.json` at line 6, The dotnet-tools.json entry for dotnet-stryker
specifies an unavailable version; update the "version" field for dotnet-stryker
from "4.14.2" to a valid published version (e.g., "4.14.1") so the tool can be
installed; locate the dotnet-stryker entry and change the "version" value
accordingly.
Comment on lines
+7
to
+10
| var v = new int[12]; | ||
| for (var i = 0; i < v.Length; i++) | ||
| v[i] = 1; | ||
| v[index] = -1; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify language-version support and locate non-collection-expression initializations in this test file.
fd -e csproj | xargs rg -n "<LangVersion>|<TargetFramework>"
rg -n "new int\\[12\\]" src/Coaching/Coaching.Application.UnitTests/DomainTests/StopsMetricsTests.csRepository: Leroy-Florian/WOW
Length of output: 296
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the unit test csproj and check its C# version settings
TEST_CSPROJ="$(fd -e csproj "Coaching.Application.UnitTests" -a src | head -n 1 || true)"
echo "TEST_CSPROJ=$TEST_CSPROJ"
if [ -n "${TEST_CSPROJ:-}" ]; then
echo "== LangVersion in test csproj =="
rg -n "<LangVersion>|<TargetFramework>|<LangVersion>" "$TEST_CSPROJ" || true
fi
echo "== Any LangVersion anywhere in repo (csproj only) =="
rg -n --glob "*.csproj" "<LangVersion>" src || true
echo "== Look for existing collection expressions usage =="
# Heuristic: find `var x = [` or `int[] x = [` etc.
rg -n --glob "*.cs" --glob "!**/*.Designer.cs" "=\s*\[" src | head -n 50Repository: Leroy-Florian/WOW
Length of output: 7674
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Repo-root global.json (if any) =="
find . -maxdepth 3 -name "global.json" -print -exec cat {} \;
echo "== Directory.Build.props (if any) containing LangVersion or C# settings =="
find . -maxdepth 4 -name "Directory.Build.props" -print
for f in $(find . -maxdepth 4 -name "Directory.Build.props"); do
echo "--- $f ---"
rg -n "LangVersion|CSharp|<LangVersion>|<TargetFramework>|LangVersion" "$f" || true
done
echo "== Any other build props targets with LangVersion =="
rg -n --glob "**/*.props" --glob "**/*.targets" "<LangVersion>|LangVersion" . || trueRepository: Leroy-Florian/WOW
Length of output: 503
Replace the new int[12] + fill loop with a C# collection expression
Directory.Build.props sets LangVersion to 14.0, and this codebase uses collection expressions ([])—so update this array initialization to match the C# guideline.
♻️ Proposed fix
- var v = new int[12];
- for (var i = 0; i < v.Length; i++)
- v[i] = 1;
+ int[] v = [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1];
v[index] = -1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var v = new int[12]; | |
| for (var i = 0; i < v.Length; i++) | |
| v[i] = 1; | |
| v[index] = -1; | |
| int[] v = [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]; | |
| v[index] = -1; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Coaching/Coaching.Application.UnitTests/DomainTests/StopsMetricsTests.cs`
around lines 7 - 10, The code creates and fills an int array with new int[12]
plus a loop; replace that with a C# collection expression to initialize the
12-element array concisely and then set the single negative element: construct v
using a collection expression (e.g., an array literal or repeated value
expression) instead of new int[12] and the for-loop, then assign v[index] = -1;
update the variable v and preserve the existing use of index and its bounds.
Comment on lines
+48
to
50
| var sampleSize = samples.GroupBy(s => s.MetricKey).Max(g => g.Count()); | ||
|
|
||
| var percentiles = ComputePercentiles(samples, sampleSize); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Move percentile and sample-size policy out of the use case.
These lines embed domain computation rules inside the application layer. Keep this use case orchestration-only and delegate percentile/sample-size calculation to domain (aggregate/domain service) to satisfy layer boundaries.
As per coding guidelines, "Application layer must have NO business logic — delegate to domain."
Also applies to: 83-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/Comparison/Comparison.Application/UseCases/Cohorts/Commands/RebuildCohorts/RebuildCohortsUseCase.cs`
around lines 48 - 50, The use case currently computes sampleSize and percentiles
(lines calling samples.GroupBy(...).Max(...) and ComputePercentiles(...)) which
is business logic that must be moved to the domain; extract the sample-size and
percentile policy into a domain service or aggregate method (e.g.,
ICohortPolicyService.GetSampleSizeAndPercentiles or
CohortAggregate.CalculatePercentiles) and implement the logic there (also move
the logic present around lines 83-99). Replace the inline calls in
RebuildCohortsUseCase with a single call to the new domain service via
constructor-injected interface, returning the sampleSize/percentiles DTO for the
use case to orchestrate only.
| [Fact] | ||
| public void Should_CopyInputCollections_When_PlayerLoadoutConstructed() | ||
| { | ||
| var items = new List<EquippedItem> { new(1, 600) }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact lines around the reported location
FILE="src/LogIngestion/LogIngestion.Application.UnitTests/Domain/LogIngestionBoundaryVoTests.cs"
echo "== File existence =="
ls -la "$FILE"
echo
echo "== Lines 160-180 =="
nl -ba "$FILE" | sed -n '160,180p'
# Determine language version for this project (or repo defaults)
echo
echo "== Relevant csproj(s) =="
fd -e csproj "src/LogIngestion" | head -n 20
echo
echo "== Language settings for the unit test project =="
# Try common locations: project file next to the source (best effort)
# We'll search for csproj containing this folder path
PROJ=$(fd -e csproj "src/LogIngestion" | xargs -I{} sh -c 'grep -q "LogIngestionBoundaryVoTests\.cs" "{}" && echo "{}"' || true | head -n 1)
if [ -z "${PROJ:-}" ]; then
# fallback: pick the first csproj under LogIngestion.Application.UnitTests
PROJ=$(fd -e csproj "src/LogIngestion/LogIngestion.Application.UnitTests" | head -n 1)
fi
echo "Using PROJ=$PROJ"
if [ -n "${PROJ:-}" ] && [ -f "$PROJ" ]; then
echo
echo "== Contents (LangVersion/nullable/other) =="
rg -n "LangVersion|TargetFramework" "$PROJ" || true
echo
echo "== Full property group (small) =="
nl -ba "$PROJ" | sed -n '1,200p'
fi
# Check whether this repo already uses C# 12 collection expressions elsewhere in src/**/*.cs
echo
echo "== Search for collection expressions usage pattern '[ ... ]' with target type =="
rg -n "List<[^>]+> [a-zA-Z_][a-zA-Z0-9_]* = \\[" src --glob='*.cs' || trueRepository: Leroy-Florian/WOW
Length of output: 304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/LogIngestion/LogIngestion.Application.UnitTests/Domain/LogIngestionBoundaryVoTests.cs"
echo "== Lines 165-175 =="
awk 'NR>=165 && NR<=175 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo
echo "== Find csproj(s) under src/LogIngestion =="
find src/LogIngestion -name '*.csproj' | head -n 20
echo
echo "== Determine csproj containing this test file =="
# Find a csproj whose project folder contains the file (best-effort via path match in SDK-style)
# We'll just list csprojs in that subtree and grep for the file name as a fallback.
for p in $(find src/LogIngestion -name '*.csproj' 2>/dev/null | head -n 50); do
if rg -q "LogIngestionBoundaryVoTests\.cs" "$p" 2>/dev/null; then
echo "$p"
fi
done
echo
echo "== Fallback: pick most likely unit test csproj and extract LangVersion/TargetFramework =="
PROJ=""
for p in $(find src/LogIngestion -name '*.csproj' 2>/dev/null | head -n 50); do
if echo "$p" | rg -q "Application\.UnitTests"; then PROJ="$p"; break; fi
done
if [ -z "$PROJ" ]; then
PROJ="$(find src/LogIngestion -name '*.csproj' 2>/dev/null | head -n 1)"
fi
echo "Using PROJ=$PROJ"
if [ -f "$PROJ" ]; then
echo
echo "== LangVersion/TargetFramework in $PROJ =="
rg -n "LangVersion|TargetFramework" "$PROJ" || true
echo
echo "== First 120 lines of $PROJ =="
head -n 120 "$PROJ"
fi
echo
echo "== Scan for existing C# collection-expression usage in repo (target-typed or typed) =="
# Rough heuristics: assignments to [ ... ] and/or variable declarations.
rg -n "=\s*\[" src --glob='*.cs' | head -n 50 || true
rg -n "var\s+\w+\s*=\s*\[" src --glob='*.cs' | head -n 50 || true
echo
echo "== Check for List<...> initializer syntax usage pattern in this unit test project =="
rg -n "new\s+List<[^>]+>\s*{ " src/LogIngestion/LogIngestion.Application.UnitTests --glob='*.cs' || trueRepository: Leroy-Florian/WOW
Length of output: 10105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for repo-wide LangVersion settings =="
rg -n "LangVersion" . --glob='*.props' --glob='*.targets' --glob='*.csproj' || true
rg -n "LangVersion" . --glob='*' -S --hidden --glob='!.git/*' || true
echo
echo "== Locate Directory.Build.props/targets =="
find . -maxdepth 4 -name "Directory.Build.props" -o -name "Directory.Build.targets" -o -name "global.json" | sed 's|^\./||'
echo
echo "== Inspect root/global build properties if present =="
for f in $(find . -maxdepth 6 -name "Directory.Build.props" -o -name "Directory.Build.targets" -o -name "global.json" 2>/dev/null); do
echo "---- $f ----"
head -n 120 "$f"
echo
done
echo "== Show TargetFrameworks in the unit test csproj =="
rg -n "TargetFramework" src/LogIngestion/LogIngestion.Application.UnitTests/LogIngestion.Application.UnitTests.csproj || true
echo
echo "== Search for DateTime.Now usage in domain (not required, but quick context check) =="
rg -n "DateTime\.(Now|UtcNow)" src/LogIngestion/LogIngestion.Application.UnitTests --glob='*.cs' || trueRepository: Leroy-Florian/WOW
Length of output: 3750
Use a C# 12 collection expression for items initialization.
List<EquippedItem> items = [new(1, 600)];Suggested change
- var items = new List<EquippedItem> { new(1, 600) };
+ List<EquippedItem> items = [new(1, 600)];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/LogIngestion/LogIngestion.Application.UnitTests/Domain/LogIngestionBoundaryVoTests.cs`
at line 172, Replace the current implicit-typed initialization of the items
variable in LogIngestionBoundaryVoTests with a C# 12 collection expression:
change the declaration of the items variable (currently using "new
List<EquippedItem> { new(1, 600) }") to an explicit List<EquippedItem>
declaration using the C#12 collection expression syntax for a single
EquippedItem instance; update the variable name "items" and ensure the type is
List<EquippedItem> and the element uses the existing EquippedItem constructor
call pattern.
Comment on lines
+65
to
+68
| var pulls = new IngestionPull[] | ||
| { | ||
| new(3009, "Boss", _start, _start.AddMinutes(5), true, null, [new IngestionCombatant("", "stormrage", WowSpec.RestorationShaman)], []), | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/LogIngestion/LogIngestion.Application.UnitTests/Ingestion/LogIngestionStrategyTests.cs"
# Basic existence check + line count
ls -l "$FILE"
wc -l "$FILE"
# Show the relevant regions around the claimed line ranges
sed -n '45,95p' "$FILE" | nl -ba | sed -n '1,200p'
echo "----"
sed -n '95,160p' "$FILE" | nl -ba | sed -n '1,200p'
echo "----"
sed -n '240,310p' "$FILE" | nl -ba | sed -n '1,200p'
echo "----"
# Find all occurrences of "new IngestionPull[]"
rg -n "new\s+IngestionPull\s*\[\]" "$FILE" || true
# Find any collection expression initializations already used in the file
rg -n "\[\s*(new\s+IngestionPull|new\s+\w+)" "$FILE" || true
# Confirm whether C# collection expressions syntax appears for arrays/collections in the file
rg -n "\bIngestionPull\[\]" "$FILE" || trueRepository: Leroy-Florian/WOW
Length of output: 359
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/LogIngestion/LogIngestion.Application.UnitTests/Ingestion/LogIngestionStrategyTests.cs"
# Show relevant regions with line numbers (cat -n works without nl)
echo "---- lines 55-95 ----"
sed -n '55,95p' "$FILE" | cat -n
echo "---- lines 95-150 ----"
sed -n '95,150p' "$FILE" | cat -n
echo "---- lines 230-310 ----"
sed -n '230,310p' "$FILE" | cat -n
# Search for array initializations of IngestionPull
echo "---- occurrences: new IngestionPull[] ----"
rg -n "new\s+IngestionPull\s*\[\]" "$FILE" || true
# Search for any C# 12 collection expression usage (for arrays/collections) near those fixtures
echo "---- occurrences: IngestionPull = [ (collection expression) ----"
rg -n "IngestionPull\s*\[\]\s*\w*\s*=" "$FILE" || true
echo "---- occurrences: '[' after '=' on IngestionPull variable (loose) ----"
rg -n "IngestionPull.*=\s*\[" "$FILE" || true
# Search for broad failure assertion region
echo "---- occurrences: IsFailure near 262-277 ----"
sed -n '245,285p' "$FILE" | cat -n | rg -n "IsFailure" || trueRepository: Leroy-Florian/WOW
Length of output: 6806
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/LogIngestion/LogIngestion.Application.UnitTests/Ingestion/LogIngestionStrategyTests.cs"
awk 'NR>=55 && NR<=90 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo "----"
awk 'NR>=95 && NR<=150 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo "----"
awk 'NR>=245 && NR<=290 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo "----"
rg -n "new\s+IngestionPull\s*\[\]" "$FILE" || true
echo "----"
rg -n "result\.IsFailure\s*\.Should\(\)\s*\.BeTrue\(\)" "$FILE" || trueRepository: Leroy-Florian/WOW
Length of output: 5356
Use C# 12 collection expressions for IngestionPull[] fixtures.
Convert new IngestionPull[] { ... } to IngestionPull[] pulls = [ ... ]; for these pull fixtures.
Suggested change
- var pulls = new IngestionPull[]
- {
- new(3009, "Boss", _start, _start.AddMinutes(5), true, null, [new IngestionCombatant("", "stormrage", WowSpec.RestorationShaman)], []),
- };
+ IngestionPull[] pulls =
+ [
+ new(3009, "Boss", _start, _start.AddMinutes(5), true, null, [new IngestionCombatant("", "stormrage", WowSpec.RestorationShaman)], []),
+ ];Also applies to: 106-109, 119-122, 133-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/LogIngestion/LogIngestion.Application.UnitTests/Ingestion/LogIngestionStrategyTests.cs`
around lines 65 - 68, Replace the old array initializers with C# 12 collection
expressions for the test fixtures: change "new IngestionPull[] { ... }" to
"IngestionPull[] pulls = [ ... ]" (and similarly for the other occurrences at
the indicated blocks), ensuring inner collections like the IngestionCombatant
list use collection expressions as well (e.g., replace "new(...) , []" style
inner arrays with "[ new IngestionCombatant(...), ... ]"); update the
declarations for variables named pulls (and other fixtures at the other
locations) so the compiler infers the array from the square-bracket collection
expression while keeping the same constructors and element values for
IngestionPull and IngestionCombatant.
Comment on lines
+262
to
+268
| public async Task Should_FailWithInvalidKeyLevel_When_KeyLevelBelowMinimum() | ||
| { | ||
| var result = await MplusStrategy().ExecuteAsync( | ||
| Mplus(keyLevel: 1), new IngestionContext(), _ct); | ||
|
|
||
| result.IsFailure.Should().BeTrue(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Assert the exact failure key, not just IsFailure.
Both tests can pass even if the wrong validation path fails. Assert the specific error key to lock behavior.
Also applies to: 271-277
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/LogIngestion/LogIngestion.Application.UnitTests/Ingestion/LogIngestionStrategyTests.cs`
around lines 262 - 268, Replace the loose IsFailure assertion in the test
Should_FailWithInvalidKeyLevel_When_KeyLevelBelowMinimum (and the similar test
at 271-277) with an assertion that verifies the specific failure key returned by
the strategy; e.g., after calling MplusStrategy().ExecuteAsync(Mplus(keyLevel:
1), ...) assert that result.Error.Key (or the code property your Result type
exposes) equals the expected validation key constant (for example
ValidationErrors.InvalidKeyLevel or the project's equivalent), so the test
checks the exact validation path rather than any failure.
Comment on lines
+16
to
+24
| public async Task Should_ReturnDistinctPlayerNames_When_SessionExists() | ||
| { | ||
| var snapshot = _repository.SeedValidSession(); | ||
|
|
||
| var result = await _query.HandleAsync(new GetRaidSessionPlayerNamesRequest(snapshot.Id)); | ||
|
|
||
| var names = ThenResultIsSuccess(result); | ||
| names.PlayerNames.Should().Contain("Thrall"); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Validate uniqueness explicitly in the “distinct names” test.
Add a uniqueness assertion so the test enforces distinctness, not only presence.
Suggested change
var names = ThenResultIsSuccess(result);
+ names.PlayerNames.Should().OnlyHaveUniqueItems();
names.PlayerNames.Should().Contain("Thrall");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public async Task Should_ReturnDistinctPlayerNames_When_SessionExists() | |
| { | |
| var snapshot = _repository.SeedValidSession(); | |
| var result = await _query.HandleAsync(new GetRaidSessionPlayerNamesRequest(snapshot.Id)); | |
| var names = ThenResultIsSuccess(result); | |
| names.PlayerNames.Should().Contain("Thrall"); | |
| } | |
| public async Task Should_ReturnDistinctPlayerNames_When_SessionExists() | |
| { | |
| var snapshot = _repository.SeedValidSession(); | |
| var result = await _query.HandleAsync(new GetRaidSessionPlayerNamesRequest(snapshot.Id)); | |
| var names = ThenResultIsSuccess(result); | |
| names.PlayerNames.Should().OnlyHaveUniqueItems(); | |
| names.PlayerNames.Should().Contain("Thrall"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/LogIngestion/LogIngestion.Application.UnitTests/Queries/GetRaidSessionPlayerNamesQueryTests.cs`
around lines 16 - 24, The test
Should_ReturnDistinctPlayerNames_When_SessionExists currently only asserts
presence of "Thrall"; add an explicit uniqueness check on the returned
collection to enforce distinctness: after calling _query.HandleAsync and
extracting names via ThenResultIsSuccess, assert that names.PlayerNames has no
duplicates (e.g., compare Count to distinct count or use a uniqueness assertion)
so the test validates distinct player names returned by
GetRaidSessionPlayerNamesRequest handling.
|
|
||
| private static ResumableUploadContext Context(Guid owner, bool gzip = false) | ||
| { | ||
| var metadata = new Dictionary<string, string>(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant lines around the referenced location
FILE="src/LogIngestion/LogIngestion.Application.UnitTests/UseCases/CompleteResumableUploadHandlerTests.cs"
echo "== File exists? =="
ls -l "$FILE" || true
echo "== Context around line 42 =="
nl -ba "$FILE" | sed -n '30,60p'
# Check for C# language version / LangVersion in the test project (csproj)
echo "== Find relevant csproj =="
fd -t f "LogIngestion.Application.UnitTests*.csproj" -d 4 src || true
# Print LangVersion (if present) from any matching csproj
for csproj in $(fd -t f -g "*LogIngestion.Application.UnitTests*.csproj" src -d 6); do
echo "== LangVersion in $csproj =="
rg -n "LangVersion|TargetFramework|LangVersion" "$csproj" || true
# Also show first PropertyGroup if small
head -n 80 "$csproj" | sed -n '1,80p'
doneRepository: Leroy-Florian/WOW
Length of output: 320
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/LogIngestion/LogIngestion.Application.UnitTests/UseCases/CompleteResumableUploadHandlerTests.cs"
echo "== Context around line 42 =="
# Print lines 30-60 with line numbers using awk
awk 'NR>=30 && NR<=60 { printf "%d\t%s\n", NR, $0 }' "$FILE"
echo "== Find csproj for this unit test project =="
# List matching csproj files (limit to a few)
mapfile -t csprojs < <(fd -t f -g "*LogIngestion.Application.UnitTests*.csproj" src || true)
printf "Found %d csproj(s)\n" "${`#csprojs`[@]}"
for csproj in "${csprojs[@]}"; do
echo "---- $csproj ----"
echo "LangVersion/TargetFramework lines:"
rg -n "LangVersion|TargetFramework" "$csproj" || true
echo "First 80 lines:"
head -n 80 "$csproj" | sed -n '1,80p'
doneRepository: Leroy-Florian/WOW
Length of output: 1572
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find candidate unit test csproj files =="
# Print matches (no mapfile/process substitution to avoid sandbox quirks)
for csproj in $(fd -t f -g "*LogIngestion.Application.UnitTests*.csproj" src || true); do
echo "---- $csproj ----"
echo "LangVersion/TargetFramework:"
rg -n "LangVersion|TargetFramework" "$csproj" || true
echo "PropertyGroup snippet:"
head -n 120 "$csproj" | sed -n '1,120p'
done
echo "== Fallback: any csproj under LogIngestion.Application.UnitTests =="
# If the above pattern didn't match, list csproj under that directory
for csproj in $(fd -t f "*.csproj" "src/LogIngestion/LogIngestion.Application.UnitTests" || true); do
echo "---- $csproj ----"
rg -n "LangVersion|TargetFramework" "$csproj" || true
head -n 120 "$csproj" | sed -n '1,120p'
doneRepository: Leroy-Florian/WOW
Length of output: 1743
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for LangVersion settings =="
rg -n --hidden --glob '!**/bin/**' --glob '!**/obj/**' "LangVersion\s*=" . || true
echo "== Check for global.json (SDK/C# defaults) =="
if [ -f "global.json" ]; then
echo "--- global.json ---"
cat global.json
else
echo "no global.json at repo root"
fi
echo "== Check for Directory.Build.props/targets =="
for f in Directory.Build.props Directory.Build.targets; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,200p' "$f"
fi
done
echo "== Also search for Directory.Build.props elsewhere (first-level) =="
for f in $(fd -t f -g "Directory.Build.props" . -a 2>/dev/null | head -n 20); do
echo "--- $f ---"
sed -n '1,200p' "$f"
doneRepository: Leroy-Florian/WOW
Length of output: 1948
Use a collection expression for metadata dictionary initialization.
src/LogIngestion/LogIngestion.Application.UnitTests/UseCases/CompleteResumableUploadHandlerTests.cs (line 42) currently initializes metadata with new Dictionary<string, string>(). Switch to collection-expression syntax.
Suggested change
- var metadata = new Dictionary<string, string>();
+ Dictionary<string, string> metadata = [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var metadata = new Dictionary<string, string>(); | |
| Dictionary<string, string> metadata = []; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/LogIngestion/LogIngestion.Application.UnitTests/UseCases/CompleteResumableUploadHandlerTests.cs`
at line 42, Replace the explicit constructor call for the metadata dictionary
with a collection initializer expression: in CompleteResumableUploadHandlerTests
change the variable metadata (currently declared as var metadata = new
Dictionary<string, string>();) to use a collection-expression initializer (e.g.,
var metadata = new Dictionary<string, string> { }; or include entries like {
["key"] = "value" } as needed).
…UDE.md Adds boundary value-object tests (TargetBreakdown/SpellBreakdown validation matrix + per-component equality + AverageAmount, PatchVersion, PlayerStat rates/clamp/equality/default-empty-breakdowns) and stop-coaching domain tests (RunGrade compute + From boundaries + equality, ClassifiedCast, StopAction, MissedOpportunity validation + equality, StopReport asymmetric counters + snapshot field mapping, RunAnalysis snapshot roundtrip preserving grade and breakdowns). Adds a mandatory Quality Bar section to CLAUDE.md: every BC (Domain and Application) must reach >=90% coverage and 90-95% mutation; only provably equivalent mutants are tolerated, each justifiable in one sentence; we never reason in terms of compute budget. Remaining survivors are equivalent (ThrowIfNull / "?? throw" reconstitution guards, id-guard statements, computed RunGrade thresholds not hittable exactly).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Analysis/Analysis.Application.UnitTests/Domain/StopReportTests.cs`:
- Around line 102-104: The test uses DateTimeOffset.UtcNow directly in
StopReport.Create which makes it non-deterministic; replace those calls with a
fixed deterministic timestamp from the test fixture or a test implementation of
ICurrentDateTimeProvider and pass that into the code path used by
StopReport.Create (or construct the timestamp locally from the shared fixed test
clock). Update the two occurrences (the StopReport.Create call and the other
usage at lines noted) to use the same fixed DateTimeOffset value (e.g.,
TestClock.Now or FixedDateTimeProvider.Now) so tests are deterministic and
follow the policy of injecting ICurrentDateTimeProvider instead of using
DateTimeOffset.UtcNow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bcabfd85-6e72-40d5-acbe-e8b6fed324fe
📒 Files selected for processing (5)
CLAUDE.mdsrc/Analysis/Analysis.Application.UnitTests/Domain/AnalysisBreakdownVoTests.cssrc/Analysis/Analysis.Application.UnitTests/Domain/RunAnalysisTests.cssrc/Analysis/Analysis.Application.UnitTests/Domain/StopAnalysisVoTests.cssrc/Analysis/Analysis.Application.UnitTests/Domain/StopReportTests.cs
Comment on lines
+102
to
+104
| var report = StopReport.Create(StopReportId.New(), Guid.NewGuid(), | ||
| DateTimeOffset.UtcNow, "v1", stops, []).Value!; | ||
|
|
There was a problem hiding this comment.
Replace wall-clock usage with deterministic test timestamps
Line 103 and Line 124 use DateTimeOffset.UtcNow. Please switch to a fixed timestamp fixture to keep tests deterministic and aligned with the date/time policy.
💡 Suggested patch
@@
- var report = StopReport.Create(StopReportId.New(), Guid.NewGuid(),
- DateTimeOffset.UtcNow, "v1", stops, []).Value!;
+ var analyzedAt = new DateTimeOffset(2026, 5, 16, 12, 0, 0, TimeSpan.Zero);
+ var report = StopReport.Create(StopReportId.New(), Guid.NewGuid(),
+ analyzedAt, "v1", stops, []).Value!;
@@
- var snapshot = StopReport.Create(StopReportId.New(), Guid.NewGuid(),
- DateTimeOffset.UtcNow, "v1", [stop], []).Value!.ToSnapshot();
+ var analyzedAt = new DateTimeOffset(2026, 5, 16, 12, 0, 0, TimeSpan.Zero);
+ var snapshot = StopReport.Create(StopReportId.New(), Guid.NewGuid(),
+ analyzedAt, "v1", [stop], []).Value!.ToSnapshot();As per coding guidelines: src/**/*.cs: Always inject ICurrentDateTimeProvider for date/time operations — never use DateTime.Now or DateTime.UtcNow anywhere in the codebase.
Also applies to: 123-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Analysis/Analysis.Application.UnitTests/Domain/StopReportTests.cs` around
lines 102 - 104, The test uses DateTimeOffset.UtcNow directly in
StopReport.Create which makes it non-deterministic; replace those calls with a
fixed deterministic timestamp from the test fixture or a test implementation of
ICurrentDateTimeProvider and pass that into the code path used by
StopReport.Create (or construct the timestamp locally from the shared fixed test
clock). Update the two occurrences (the StopReport.Create call and the other
usage at lines noted) to use the same fixed DateTimeOffset value (e.g.,
TestClock.Now or FixedDateTimeProvider.Now) so tests are deterministic and
follow the policy of injecting ICurrentDateTimeProvider instead of using
DateTimeOffset.UtcNow.
Completes coverage of the Spell and WagoSpellRecord aggregates (Create/Annotate/ SyncFromBlizzard branches, provenance defaults, tag dedup, annotated-event field mapping, FromSnapshot re-validation including zero-boundary durations) and the SpellSchool / SpellAttributes / AffixCatalog value objects (code+name resolution, bit-decode boundaries, defensive group copy, known-vs-synthetic affix lookup). Kills the ArgumentNullException.ThrowIfNull guards by asserting the precise exception type (a removed guard surfaces as NullReferenceException, a different type) rather than treating them as equivalent. Remaining survivors are genuinely equivalent (HasFlag bounds whose fallback indexer throws the same exception type).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/SpellKnowledge/SpellKnowledge.Application.UnitTests/Spells/Domain/SpellKnowledgeAggregateTests.cs`:
- Line 5: Rename the test class SpellKnowledgeAggregateTests to follow the
{UseCase}Tests convention by choosing a concrete use-case-oriented name that
reflects the suite (e.g., SyncSpellFromBlizzardUseCaseTests); update the class
declaration name and any references/usages in the file
(SpellKnowledgeAggregateTests -> SyncSpellFromBlizzardUseCaseTests or another
appropriate UseCase name) so the file name and class name match the repository
test naming guideline.
- Around line 166-291: The tests in SpellKnowledge.Application.UnitTests call
aggregate methods directly (e.g. Spell.Create via NewSpell, Spell.Annotate,
Spell.SyncFromBlizzard and assertions on Spell.DomainEvents), which violates the
rule that Application.UnitTests must exercise aggregate behavior only via
application services/use‑cases; refactor these tests to invoke an application
service/use‑case (a fake or in‑memory repository and driver) that orchestrates
calls to the Spell aggregate (e.g. use a SpellApplicationService or
CreateSpell/AnnotateSpell/SyncSpell use‑case), assert results and emitted events
through the service response or event collector, and remove direct calls to
Spell.Annotate/Spell.SyncFromBlizzard and direct inspections of
Spell.DomainEvents in this test class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: db96373b-7a77-4eb3-b52f-67a8e987fdce
📒 Files selected for processing (1)
src/SpellKnowledge/SpellKnowledge.Application.UnitTests/Spells/Domain/SpellKnowledgeAggregateTests.cs
|
|
||
| namespace SpellKnowledge.Application.UnitTests.Spells.Domain; | ||
|
|
||
| public sealed class SpellKnowledgeAggregateTests |
There was a problem hiding this comment.
Rename test class to the {UseCase}Tests convention.
SpellKnowledgeAggregateTests does not follow the mandated test naming pattern. Rename to a concrete use-case-oriented name (for example, SyncSpellFromBlizzardUseCaseTests) matching what the suite validates.
As per coding guidelines: "**/*Tests.cs: Test class names must follow the pattern {UseCase}Tests".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/SpellKnowledge/SpellKnowledge.Application.UnitTests/Spells/Domain/SpellKnowledgeAggregateTests.cs`
at line 5, Rename the test class SpellKnowledgeAggregateTests to follow the
{UseCase}Tests convention by choosing a concrete use-case-oriented name that
reflects the suite (e.g., SyncSpellFromBlizzardUseCaseTests); update the class
declaration name and any references/usages in the file
(SpellKnowledgeAggregateTests -> SyncSpellFromBlizzardUseCaseTests or another
appropriate UseCase name) so the file name and class name match the repository
test naming guideline.
Comment on lines
+166
to
+291
| private static Spell NewSpell(Provenance? provenance = null) => | ||
| Spell.Create( | ||
| BlizzardSpellId.From(47528), | ||
| PatchVersion.From(11, 0, 5), | ||
| SpellName.From("Pummel"), | ||
| SpellSchool.Physical, | ||
| _at, | ||
| baseCooldown: TimeSpan.FromSeconds(15), | ||
| isInterruptible: false, | ||
| provenance: provenance).Value!; | ||
|
|
||
| [Fact] | ||
| public void Should_RejectNegativeCooldown_When_SpellCreate() | ||
| { | ||
| Spell.Create(BlizzardSpellId.From(1), PatchVersion.From(11, 0, 5), SpellName.From("X"), | ||
| SpellSchool.Fire, _at, baseCooldown: TimeSpan.FromSeconds(-1)).IsFailure.Should().BeTrue(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_RaiseRegisteredEvent_When_SpellCreated() | ||
| { | ||
| NewSpell().DomainEvents.OfType<SpellKnowledge.Domain.Events.SpellRegisteredEvent>().Should().ContainSingle(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_DefaultToManualProvenance_When_NoneProvided() | ||
| { | ||
| NewSpell().Provenance.Source.Should().Be(ProvenanceSource.Manual); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_KeepProvidedProvenance_When_ExplicitlySupplied() | ||
| { | ||
| NewSpell(Provenance.Blizzard(_at)).Provenance.Source.Should().Be(ProvenanceSource.BlizzardApi); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_RejectInvalidTierOrCategory_When_Annotate() | ||
| { | ||
| NewSpell().Annotate((DangerTier)99, null, [], _at).IsFailure.Should().BeTrue(); | ||
| NewSpell().Annotate(null, (DiminishingReturnsCategory)99, [], _at).IsFailure.Should().BeTrue(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_SetAnnotationsDedupeTagsAndRaiseEvent_When_Annotate() | ||
| { | ||
| var spell = NewSpell(); | ||
|
|
||
| var result = spell.Annotate( | ||
| DangerTier.Lethal, DiminishingReturnsCategory.Stun, | ||
| [SpellTag.From("aoe-stun"), SpellTag.From("aoe-stun"), SpellTag.From("raid-cd")], | ||
| _at); | ||
|
|
||
| ThenResultIsSuccess(result); | ||
| spell.DangerTier.Should().Be(DangerTier.Lethal); | ||
| spell.DrCategory.Should().Be(DiminishingReturnsCategory.Stun); | ||
| spell.Tags.Should().HaveCount(2); | ||
| var evt = spell.DomainEvents.OfType<SpellKnowledge.Domain.Events.SpellAnnotatedEvent>().Should().ContainSingle().Subject; | ||
| evt.DangerTier.Should().Be((int)DangerTier.Lethal); | ||
| evt.DrCategory.Should().Be((int)DiminishingReturnsCategory.Stun); | ||
| evt.Tags.Should().BeEquivalentTo("aoe-stun", "raid-cd"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_EmitNullTierAndCategory_When_AnnotatedWithoutThem() | ||
| { | ||
| var spell = NewSpell(); | ||
|
|
||
| spell.Annotate(null, null, [SpellTag.From("kickable-cast")], _at); | ||
|
|
||
| var evt = spell.DomainEvents.OfType<SpellKnowledge.Domain.Events.SpellAnnotatedEvent>().Should().ContainSingle().Subject; | ||
| evt.DangerTier.Should().BeNull(); | ||
| evt.DrCategory.Should().BeNull(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_RefuseSync_When_SpellManuallyOverridden() | ||
| { | ||
| var spell = NewSpell(Provenance.Manual(_at)); | ||
|
|
||
| var result = spell.SyncFromBlizzard( | ||
| SpellName.From("New"), SpellSchool.Fire, TimeSpan.FromSeconds(10), true, Provenance.Blizzard(_at)); | ||
|
|
||
| ThenResultIsFailureWithError(result, SpellKnowledgeErrors.Spell.ManualOverrideProtected); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_ThrowArgumentNull_When_SyncSchoolNull() | ||
| { | ||
| var spell = NewSpell(Provenance.Blizzard(_at)); | ||
|
|
||
| ((Action)(() => spell.SyncFromBlizzard(SpellName.From("X"), null!, null, false, Provenance.Blizzard(_at)))) | ||
| .Should().Throw<ArgumentNullException>(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_ResolveKnownAndSyntheticAffixes() | ||
| { | ||
| AffixCatalog.Resolve(9).Name.Should().Be("Tyrannical"); | ||
| AffixCatalog.Resolve(999999).Name.Should().Be("Affix #999999"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_RejectNegativeCooldown_When_SyncFromBlizzard() | ||
| { | ||
| var spell = NewSpell(Provenance.Blizzard(_at)); | ||
|
|
||
| spell.SyncFromBlizzard(SpellName.From("New"), SpellSchool.Fire, TimeSpan.FromSeconds(-1), true, Provenance.Blizzard(_at)) | ||
| .IsFailure.Should().BeTrue(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void Should_UpdateBlizzardFields_When_SyncSucceeds() | ||
| { | ||
| var spell = NewSpell(Provenance.Blizzard(_at)); | ||
|
|
||
| var result = spell.SyncFromBlizzard( | ||
| SpellName.From("Counterspell"), SpellSchool.Arcane, TimeSpan.FromSeconds(24), true, Provenance.Blizzard(_at)); | ||
|
|
||
| ThenResultIsSuccess(result); | ||
| spell.Name.Value.Should().Be("Counterspell"); | ||
| spell.School.Should().Be(SpellSchool.Arcane); | ||
| spell.IsInterruptible.Should().BeTrue(); | ||
| spell.BaseCooldown.Should().Be(TimeSpan.FromSeconds(24)); | ||
| spell.Provenance.Source.Should().Be(ProvenanceSource.BlizzardApi); | ||
| } |
There was a problem hiding this comment.
Move aggregate behavior assertions behind a use-case test boundary.
These tests directly exercise the Spell aggregate in an Application.UnitTests suite, which violates the required testing boundary for this path. Please route assertions through an application service/use case (with fake infrastructure as needed) instead of testing aggregate methods directly.
As per coding guidelines: "**/*Application.UnitTests/**/*Tests.cs: Unit tests must not directly test aggregates — test through use cases (application services) instead".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/SpellKnowledge/SpellKnowledge.Application.UnitTests/Spells/Domain/SpellKnowledgeAggregateTests.cs`
around lines 166 - 291, The tests in SpellKnowledge.Application.UnitTests call
aggregate methods directly (e.g. Spell.Create via NewSpell, Spell.Annotate,
Spell.SyncFromBlizzard and assertions on Spell.DomainEvents), which violates the
rule that Application.UnitTests must exercise aggregate behavior only via
application services/use‑cases; refactor these tests to invoke an application
service/use‑case (a fake or in‑memory repository and driver) that orchestrates
calls to the Spell aggregate (e.g. use a SpellApplicationService or
CreateSpell/AnnotateSpell/SyncSpell use‑case), assert results and emitted events
through the service response or event collector, and remove direct calls to
Spell.Annotate/Spell.SyncFromBlizzard and direct inspections of
Spell.DomainEvents in this test class.
Covers DungeonRun Complete/Abandon outcome + event payloads (timed vs depleted, keystone affix ids, abandoned has no completion), visibility-change idempotence, mixed-keystone rejection; OwnerUserId/GuildId guards + equality; UploadJournalEntry null-arg guards and corrupt-snapshot rejection; RunPlayer name-length boundaries and per-component equality; SeasonSlug diacritic/apostrophe normalization, hyphen collapse and length limits; ShareToken url-safe character boundaries.
Strengthens the ingestion-pipeline tests: SegmentHasher now discriminates raid session-start and per-pull start (not just difficulty/instance); the Mythic+ strategy asserts outbox tracking, no-event batching, in-progress (null outcome) persistence and untimed completion; UploadCombatLog covers the strategy-reported duplicate (dedup with empty id) and non-duplicate failure (error propagation) paths; the resumable handler distinguishes non-gzip content encodings; and a new ListUserUploads query test covers the take clamp and projection. Remaining survivors are equivalent (SHA payload prefix / StringBuilder capacity, StreamReader BOM/buffer/leaveOpen flags, unreachable null-strategy guard).
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/LogIngestion/LogIngestion.Application.UnitTests/UseCases/CompleteResumableUploadHandlerTests.cs (1)
90-92: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse explicit typed collection expression for metadata initialization.
Line 90 still uses constructor-based collection initialization. Replace it with the required collection-expression form and explicit target type.
Suggested fix
- var metadata = new Dictionary<string, string>(); + Dictionary<string, string> metadata = []; metadata["contentEncoding"] = "identity";As per coding guidelines
**/*.cs: Use collection expression syntax[...]for all collection initialization, and**/*.cs: when using collection expressions, the target type must be explicit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/LogIngestion/LogIngestion.Application.UnitTests/UseCases/CompleteResumableUploadHandlerTests.cs` around lines 90 - 92, Replace the constructor-based init with an explicit-typed collection expression: change the metadata declaration to use an explicit target type and collection expression, e.g. Dictionary<string, string> metadata = new() { ["contentEncoding"] = "identity" }, keeping the ResumableUploadContext("upload-1", _owner, metadata) usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@src/LogIngestion/LogIngestion.Application.UnitTests/UseCases/CompleteResumableUploadHandlerTests.cs`:
- Around line 90-92: Replace the constructor-based init with an explicit-typed
collection expression: change the metadata declaration to use an explicit target
type and collection expression, e.g. Dictionary<string, string> metadata = new()
{ ["contentEncoding"] = "identity" }, keeping the
ResumableUploadContext("upload-1", _owner, metadata) usage unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7e4f69ca-cbbf-4cd3-bd5b-ac9a48bf4937
📒 Files selected for processing (6)
src/LogIngestion/LogIngestion.Application.UnitTests/Domain/LogIngestionDomainCoverageTests.cssrc/LogIngestion/LogIngestion.Application.UnitTests/Ingestion/LogIngestionStrategyTests.cssrc/LogIngestion/LogIngestion.Application.UnitTests/Ingestion/SegmentHasherTests.cssrc/LogIngestion/LogIngestion.Application.UnitTests/Queries/ListUserUploadsQueryTests.cssrc/LogIngestion/LogIngestion.Application.UnitTests/UseCases/CompleteResumableUploadHandlerTests.cssrc/LogIngestion/LogIngestion.Application.UnitTests/UseCases/UploadCombatLogUseCaseTests.cs
Covers the DeviceAuthorization Create validation matrix (device/user code, client id, scope, lifetime, poll-interval bounds incl. exact boundaries) and its Approve/Deny/MarkConsumed/IsExpired state machine; the User aggregate's password-reset verification, magic-link consume, AddLocalPassword/RehashPassword, CreateFromBattleNet and RecordBattleNetLogin flows (events asserted by type and count); WowCharacter name/level boundaries and class/faction allow-lists; and the BattleTag/DisplayName/Bio/DeviceAuthorizationId value objects.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Identity/Identity.Application.UnitTests/Domain/IdentityDomainCoverageTests.cs`:
- Line 11: The class IdentityDomainCoverageTests is defined outside the Domain
layer and must either be converted to a primary constructor or explicitly
annotated as non-primary; update the class declaration for
IdentityDomainCoverageTests to use a primary constructor form (e.g., public
sealed class IdentityDomainCoverageTests(...) ) if its constructor can be
expressed that way, otherwise add a single-line comment immediately above the
class like "// non-primary: <reason>" explaining why a primary constructor
cannot be used so the declaration complies with the project's
primary-constructor rule.
- Line 78: Rename the unit tests to follow the Should_[Outcome]_When_[Condition]
pattern: update method Should_AcceptNullScopeAsEmpty to something like
Should_AcceptScopeAsEmpty_When_ScopeIsNull, and similarly rename
Should_ApprovePendingAuthorization, Should_AcceptAndCompareBattleTag,
Should_NormalizeNullBioToEmpty and the other test methods referenced (lines 93,
99, 107, 144, 154, 166, 190, 201, 233, 257, 266, 278) to express outcome and
condition; ensure each test method name uses the exact
"Should_[Outcome]_When_[Condition]" format and adjust any usages or references
to those method names accordingly (e.g., in test attributes or runners).
In `@src/Identity/Identity.Application.UnitTests/Domain/UserAggregateTests.cs`:
- Line 8: The class declaration for UserAggregateTests is a non-primary class
outside the Domain layer — add a primary constructor or, if a primary
constructor cannot express the required setup, annotate the class with a
non-primary reason comment; specifically, update the declaration of
UserAggregateTests to use a primary constructor (e.g., include required test
dependencies in the class header) or add a top-line comment like "//
non-primary: <reason>" immediately above the UserAggregateTests class explaining
why a primary constructor isn't possible.
- Line 63: The test method name Should_FailReset_AtExactExpiryInstant doesn't
follow the required pattern; rename the method to adhere to
Should_[Outcome]_When_[Condition], e.g.
Should_FailReset_When_AtExactExpiryInstant, and update any references (test
invocation, attributes, or display names) to the new identifier so the test
framework and callers compile and the naming convention is satisfied; ensure the
method signature in UserAggregateTests.cs and any usages are changed
consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 81aa50ca-550a-40df-88e6-445db2653a10
📒 Files selected for processing (2)
src/Identity/Identity.Application.UnitTests/Domain/IdentityDomainCoverageTests.cssrc/Identity/Identity.Application.UnitTests/Domain/UserAggregateTests.cs
|
|
||
| namespace Identity.Application.UnitTests.Domain; | ||
|
|
||
| public sealed class IdentityDomainCoverageTests |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use a primary constructor (or add required non-primary annotation).
IdentityDomainCoverageTests is outside the Domain layer and currently uses a non-primary class declaration without the required // non-primary: <reason> note.
As per coding guidelines, "!(**/{BC}.Domain/**/*.cs|**/Domain/**/*.cs): All classes and structs outside the Domain layer must use primary constructors (mandatory)" and "If a constructor cannot be expressed as a primary constructor, annotate it with // non-primary: <reason> comment explaining why".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/Identity/Identity.Application.UnitTests/Domain/IdentityDomainCoverageTests.cs`
at line 11, The class IdentityDomainCoverageTests is defined outside the Domain
layer and must either be converted to a primary constructor or explicitly
annotated as non-primary; update the class declaration for
IdentityDomainCoverageTests to use a primary constructor form (e.g., public
sealed class IdentityDomainCoverageTests(...) ) if its constructor can be
expressed that way, otherwise add a single-line comment immediately above the
class like "// non-primary: <reason>" explaining why a primary constructor
cannot be used so the declaration complies with the project's
primary-constructor rule.
| } | ||
|
|
||
| [Fact] | ||
| public void Should_AcceptNullScopeAsEmpty() |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Normalize test names to Should_[Outcome]_When_[Condition].
These methods currently violate the required pattern (examples: Should_AcceptNullScopeAsEmpty, Should_ApprovePendingAuthorization, Should_AcceptAndCompareBattleTag, Should_NormalizeNullBioToEmpty).
As per coding guidelines, "**/*.UnitTests/**/*Tests.cs: Unit test naming convention: Should_[Outcome]_When_[Condition]".
Also applies to: 93-93, 99-99, 107-107, 144-144, 154-154, 166-166, 190-190, 201-201, 233-233, 257-257, 266-266, 278-278
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/Identity/Identity.Application.UnitTests/Domain/IdentityDomainCoverageTests.cs`
at line 78, Rename the unit tests to follow the
Should_[Outcome]_When_[Condition] pattern: update method
Should_AcceptNullScopeAsEmpty to something like
Should_AcceptScopeAsEmpty_When_ScopeIsNull, and similarly rename
Should_ApprovePendingAuthorization, Should_AcceptAndCompareBattleTag,
Should_NormalizeNullBioToEmpty and the other test methods referenced (lines 93,
99, 107, 144, 154, 166, 190, 201, 233, 257, 266, 278) to express outcome and
condition; ensure each test method name uses the exact
"Should_[Outcome]_When_[Condition]" format and adjust any usages or references
to those method names accordingly (e.g., in test attributes or runners).
|
|
||
| namespace Identity.Application.UnitTests.Domain; | ||
|
|
||
| public sealed class UserAggregateTests |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use a primary constructor (or add required non-primary annotation).
UserAggregateTests is outside the Domain layer and currently uses a non-primary class declaration without the required // non-primary: <reason> note.
As per coding guidelines, "!(**/{BC}.Domain/**/*.cs|**/Domain/**/*.cs): All classes and structs outside the Domain layer must use primary constructors (mandatory)" and "If a constructor cannot be expressed as a primary constructor, annotate it with // non-primary: <reason> comment explaining why".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Identity/Identity.Application.UnitTests/Domain/UserAggregateTests.cs` at
line 8, The class declaration for UserAggregateTests is a non-primary class
outside the Domain layer — add a primary constructor or, if a primary
constructor cannot express the required setup, annotate the class with a
non-primary reason comment; specifically, update the declaration of
UserAggregateTests to use a primary constructor (e.g., include required test
dependencies in the class header) or add a top-line comment like "//
non-primary: <reason>" immediately above the UserAggregateTests class explaining
why a primary constructor isn't possible.
| } | ||
|
|
||
| [Fact] | ||
| public void Should_FailReset_AtExactExpiryInstant() |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Rename test to match the mandatory unit-test naming pattern.
Should_FailReset_AtExactExpiryInstant does not follow Should_[Outcome]_When_[Condition].
As per coding guidelines, "**/*.UnitTests/**/*Tests.cs: Unit test naming convention: Should_[Outcome]_When_[Condition]".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Identity/Identity.Application.UnitTests/Domain/UserAggregateTests.cs` at
line 63, The test method name Should_FailReset_AtExactExpiryInstant doesn't
follow the required pattern; rename the method to adhere to
Should_[Outcome]_When_[Condition], e.g.
Should_FailReset_When_AtExactExpiryInstant, and update any references (test
invocation, attributes, or display names) to the new identifier so the test
framework and callers compile and the naming convention is satisfied; ensure the
method signature in UserAggregateTests.cs and any usages are changed
consistently.
The stryker mutate exclusions targeted "**/SharedKernel/..." but the actual folder is "Kernel.Shared/...", so Mediator/Outbox/Localization/UtcDateTimeProvider were never excluded as intended — they dragged the score to 44%. Corrects the globs to the real path and adds the ASP.NET hypermedia integration (Hateoas) and DI marker registration to the framework-infrastructure exclusion set (these are exercised by E2E, like the already-excluded API layer). Also excludes the pure i18n Errors/*Translations.cs data files, consistent with the Localization exclusion. Adds real coverage for the remaining domain/cross-cutting logic: RunOutcome completion + FromInt parsing, WowSpecInfo.FromClassAndSpec resolution, the PvP rating boundary in RequirementEvaluator, TruncatingRedactor truncation, and the AddWowRedaction HMAC-key validation (missing/blank/non-base64/valid).
Adds challenge/encounter start+end metadata extraction (map/instance/key/affixes, encounter/difficulty/group, success, duration), attempt counting across repeated pulls of the same encounter, zone-change abandonment, malformed-payload null fields, and the ignore paths (orphan end, missing timestamp separator, raid not closed by challenge-end). Syncs the audit stryker config exclusions with the committed config (Kernel.Shared path fix + Hateoas/DI framework exclusions).
…coverage gaps Aligns coverlet.runsettings exclusions with the mutation mutate-scope so coverage measures the same code: drops Infrastructure assemblies, i18n Errors/*Translations, Mediator/Outbox/Localization/Hateoas/DI plumbing, Providers, UtcDateTimeProvider, and generated *.g.cs (regex/logging source-gen that was dragging Kernel to ~40%). Closes the resulting gaps: Hotfix snapshot roundtrip (Domain 100%) and the full WowExpansion->slug map (Kernel 97%). Backend BCs are now >=90% line coverage on the in-scope code.
AcceptCandidateUseCase.ResolveEncounterIdAsync was a hardcoded stub that always returned null, so the encounter-resolution + AttachUseCaseCommand branch (and its MapKind helper) was unreachable dead code, and the MergeIntoExistingUseCase flag only gated it. Wiring real resolution needs a product decision (patch selection) and is out of scope here, so the dead branch is removed rather than left as untestable scaffolding. AcceptCandidate now simply flips the candidate to Merged with reviewer + note; the unused IMediator dependency, command flag, API body field and RecordingMediator fake are dropped. KnowledgeMining.Application: line/branch 100%, mutation 89%->94%.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Adds a local dotnet-stryker tool manifest and a solution-less audit config so
mutation testing can run per bounded context (the committed config's single
solution can't build the web workspaces in CI-less environments).
Excludes Domain/Errors/*Translations.cs from the mutate scope: these are pure
FR/EN string-registration tables with no business logic, mirroring the existing
SharedKernel/Localization exclusion.
Summary by CodeRabbit