Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/skills/execute-implement/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ You will receive:
- Apply the minimal change set needed to satisfy acceptance signals.
3. **Self-check**
- Review for style, safety, and regression risk.
- Every new or changed decision/branch needs at least one test that actually exercises it — use `fieldworks-test-coverage` to verify with `test.ps1 -Coverage`, not just "a test file exists nearby." Document any accepted gap (e.g. requires a live/manual round-trip) rather than letting it pass silently.
- For FieldWorks branches that will become PRs, use the review policy in `.github/instructions/review-analyzer.instructions.md` or run `pr-preflight` before handoff when risk or scope warrants it.
4. **Update status**
- Record what changed and any follow-up notes.
Expand Down
43 changes: 43 additions & 0 deletions .claude/skills/fieldworks-test-coverage/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
name: fieldworks-test-coverage
description: Check that new or changed FieldWorks code is actually exercised by tests, using test.ps1 -Coverage. Use whenever writing tests for a change, before claiming a bug fix or feature is covered, or when reviewing whether "tests pass" also means the new logic runs under test.
model: haiku
---

<role>
You confirm that new/changed behavior is exercised by tests, not merely that some test file exists nearby. "Tests pass" is not the same claim as "the new code ran during the test" — this skill closes that gap.
</role>

<inputs>
You will receive:
- The diff or file list for the change
- The test project(s) that build against the changed files
</inputs>

<workflow>
1. **Identify the decision surface**
- For each changed method, find the new/changed branches, guards, and edge cases — not just "a test file touches this class."
- Pure decisions (parsing, gating, mapping) are cheap to cover directly. Side-effecting code wrapped around registry/filesystem/global-singleton calls is often already excluded by design in this repo (e.g. `AvaloniaOptionsDialogLauncher.Apply`) — check whether the surrounding test file already documents that scope boundary before assuming it's a gap.
2. **Run coverage for the touched project(s)**
- `.\test.ps1 -TestProject <project path or name> -Coverage [-NoBuild] [-TestFilter "..."]`
- This collects via coverlet.collector's "XPlat Code Coverage" data collector (works under plain vstest.console.exe on any VS edition — the older `/EnableCodeCoverage` flag in `Build/SetupInclude.targets` requires VS Enterprise and will silently fail to collect on Community/Build Tools installs).
- Reads the merged report from `Output/<Configuration>/TestResults/CoverageReport/Summary.txt` (and `index.html` for line-by-line detail) that the script prints automatically.
3. **Check the new lines specifically**
- Open `index.html` for the changed file(s) and confirm the new/changed lines show as covered (green), not just that the file's overall percentage moved.
- A changed file can show high coverage while the new branch itself is dead — always check the specific lines, not the file average.
4. **Close real gaps, document accepted ones**
- If a new decision is uncovered and cheaply testable, write the test (see repo test file conventions — extract pure decisions into small static/testable methods when the surrounding code is otherwise hard to unit test, as with `AvaloniaDialogHost.ResolveEffectiveOwner`).
- If a gap is accepted (e.g. it requires a live/manual/desktop-only round-trip), say so explicitly in the PR description or handoff — do not let it pass silently as "tests pass."
</workflow>

<constraints>
- Never report coverage as a percentage alone; tie the claim to the specific new/changed lines.
- Do not chase repo-wide coverage — scope to the current change's touched files.
- Prefer extracting a testable pure decision over mocking heavy framework/registry/filesystem dependencies to force coverage of an untestable method.
</constraints>

<notes>
- See `verify-test` for the broader build/test verification flow this slots into.
- See `execute-implement`'s self-check step, which references this skill for new logic.
- `test.ps1 -Coverage` requires the local dotnet tool manifest to be restored once per checkout: `dotnet tool restore` (installs ReportGenerator, used to render the summary/HTML from the raw `coverage.cobertura.xml`).
</notes>
2 changes: 2 additions & 0 deletions .claude/skills/verify-test/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ You will receive:
- Execute builds/tests or manual checks as appropriate.
- For FieldWorks, run `./build.ps1 -Clean` before validation when switching branches or worktrees, upgrading package versions, suspecting stale `Obj/` or `Output/` artifacts, or any time you need a fully clean validation baseline.
- After the clean step, rerun the normal scripted verification commands such as `./build.ps1`, `./test.ps1`, or the narrow scripted slice you are validating.
- When the change adds or edits behavior (not docs/test-only changes), also run `./test.ps1 -Coverage` for the touched project(s) — see `fieldworks-test-coverage` — and confirm the new/changed lines are actually exercised, not just that the suite passes.
3. **Capture results**
- Record pass/fail outcomes and any artifacts.
4. **Report**
Expand All @@ -38,4 +39,5 @@ You will receive:
<notes>
- Use FieldWorks build/test scripts when applicable.
- Use `rubric-verify` when execution-free scoring or explicit hard-gate review is requested.
- Use `fieldworks-test-coverage` to check that new/changed lines are exercised, not just that tests pass.
</notes>
13 changes: 13 additions & 0 deletions .config/dotnet-tools.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"version": 1,
"isRoot": true,
"tools": {
"dotnet-reportgenerator-globaltool": {
"version": "5.3.11",
"commands": [
"reportgenerator"
],
"rollForward": false
}
}
}
4 changes: 2 additions & 2 deletions .github/rubrics/fieldworks-rubric.base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ categories:
evidence: API diff or behavioral comparison note
- id: test_coverage
weight: 0.20
check: New or changed behavior has at least one corresponding test.
evidence: test names covering the change
check: New or changed behavior has at least one corresponding test that actually exercises the new/changed lines (not merely a nearby test file).
evidence: test names covering the change, ideally cross-checked with `test.ps1 -Coverage` (see `fieldworks-test-coverage`)

- id: integrity
weight: 0.25
Expand Down
10 changes: 9 additions & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,17 @@ jobs:
id: test
shell: powershell
run: |
.\test.ps1 -Configuration Debug -NoBuild -TestFilter 'TestCategory!=LongRunning&TestCategory!=ByHand&TestCategory!=SmokeTest&TestCategory!=DesktopRequired'
.\test.ps1 -Configuration Debug -NoBuild -Coverage -TestFilter 'TestCategory!=LongRunning&TestCategory!=ByHand&TestCategory!=SmokeTest&TestCategory!=DesktopRequired'
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }

- name: Upload coverage report
if: ${{ !cancelled() }}
uses: actions/upload-artifact@v7
with:
name: coverage-report
path: Output/Debug/TestResults/CoverageReport
if-no-files-found: warn

- name: Summarize native test results
if: ${{ always() }}
shell: powershell
Expand Down
4 changes: 4 additions & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@
<PackageVersion Include="NUnit3TestAdapter" Version="4.6.0" />
<PackageVersion Include="NUnitForms" Version="1.3.1" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.14.1" />
<!-- Cross-platform coverage collector (works under plain vstest.console.exe, unlike the VS-Enterprise-
only "Code Coverage" collector) — registers the "XPlat Code Coverage" data collector used by
test.ps1 -Coverage. -->
<PackageVersion Include="coverlet.collector" Version="6.0.4" />
<PackageVersion Include="SkiaSharp" Version="2.88.9" />
<PackageVersion Include="SkiaSharp.HarfBuzz" Version="2.88.9" />
<PackageVersion Include="SkiaSharp.NativeAssets.Win32" Version="2.88.9" />
Expand Down
1 change: 1 addition & 0 deletions Src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,5 @@
<PackageReference Include="NUnit" />
<PackageReference Include="NUnit3TestAdapter" />
<PackageReference Include="Microsoft.NET.Test.Sdk" />
<PackageReference Include="coverlet.collector" />
</ItemGroup></Project>
123 changes: 123 additions & 0 deletions test.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@
Internal switch used when test.ps1 is invoked from build.ps1 -RunTests.
Skips acquiring/releasing the same-worktree lock because the parent build already owns it.

.PARAMETER Coverage
Collect code coverage via the coverlet.collector "XPlat Code Coverage" data collector (works under
plain vstest.console.exe on any Visual Studio edition, unlike the VS-Enterprise-only "Code Coverage"
collector). Writes one coverage.cobertura.xml per test host under Output/<Configuration>/TestResults,
then renders a merged summary + HTML report via the local ReportGenerator tool
(Output/<Configuration>/TestResults/CoverageReport).

.EXAMPLE
.\test.ps1
Runs all tests in Debug configuration (builds first if needed).
Expand All @@ -69,6 +76,10 @@
.\test.ps1 -SkipManaged -TestProject TestGeneric
Uses the environment variable equivalent of -AllowAssertDialogs. Clear the variable after debugging.

.EXAMPLE
.\test.ps1 -TestProject "Src/Common/FwAvalonia/FwAvaloniaTests" -Coverage
Runs FwAvaloniaTests with code coverage collection and prints a coverage summary.

.NOTES
FieldWorks is x64-only. Tests run in 64-bit mode.
#>
Expand All @@ -86,6 +97,7 @@ param(
[switch]$AllowAssertDialogs,
[switch]$SkipDependencyCheck,
[switch]$SkipWorktreeLock,
[switch]$Coverage,
[ValidateSet('user', 'agent', 'unknown')]
[string]$StartedBy = 'unknown'
)
Expand Down Expand Up @@ -179,6 +191,36 @@ function New-TestRunSettingsForAssertDialogMode {
return $runSettingsPath
}

function New-TestRunSettingsForCoverage {
param(
[string]$SourcePath,
[string]$Configuration,
[bool]$Coverage
)

if (-not $Coverage) {
return $SourcePath
}

# coverlet.collector's "XPlat Code Coverage" data collector instruments every module with a
# matching .pdb next to the test assembly, and restores it afterward. FieldWorks test projects
# all share a single Output/<Configuration> folder rather than per-project bin folders, so with
# the default MaxCpuCount=0 (parallel testhosts), two testhosts can end up instrumenting/restoring
# the same shared product DLL (e.g. XMLViews.dll) at the same time, which fails with
# "being used by another process". Force serial testhost execution only for coverage runs.
[xml]$runSettings = Get-Content -LiteralPath $SourcePath -Raw
$runSettings.RunSettings.RunConfiguration.MaxCpuCount = '1'

$runSettingsDir = Join-Path $PSScriptRoot "Output/$Configuration"
if (-not (Test-Path $runSettingsDir)) {
New-Item -ItemType Directory -Force -Path $runSettingsDir | Out-Null
}

$runSettingsPath = Join-Path $runSettingsDir 'Test.coverage.runsettings'
$runSettings.Save($runSettingsPath)
return $runSettingsPath
}

$allowAssertDialogsForRun = $AllowAssertDialogs -or (Test-EnvironmentSwitchEnabled -Name 'FW_TEST_ALLOW_ASSERT_DIALOGS')

function Add-UniquePath {
Expand Down Expand Up @@ -280,6 +322,28 @@ function Get-NUnitTestAdapterPaths {
return $adapterPaths.ToArray()
}

function Get-CoverageAdapterPath {
param(
[string]$RepoRoot
)

# vstest.console.exe (unlike `dotnet test`) does not auto-discover data collectors from NuGet
# packages; the coverlet.collector build folder must be passed explicitly via /TestAdapterPath,
# same as the NUnit adapter above.
$packagesPropsPath = Join-Path $RepoRoot 'Directory.Packages.props'
$version = Get-CentralPackageVersion -PackagesPropsPath $packagesPropsPath -PackageName 'coverlet.collector'
if ([string]::IsNullOrWhiteSpace($version)) {
return $null
}

$adapterPath = Join-Path $RepoRoot "packages/coverlet.collector/$version/build/netstandard2.0"
if (Test-Path -LiteralPath (Join-Path $adapterPath 'coverlet.collector.dll') -PathType Leaf) {
return $adapterPath
}

return $null
}

# =============================================================================
# Environment Setup
# =============================================================================
Expand Down Expand Up @@ -641,6 +705,10 @@ try {
-SourcePath $runSettingsSourcePath `
-Configuration $Configuration `
-AllowDialogs $allowAssertDialogsForRun
$runSettingsPath = New-TestRunSettingsForCoverage `
-SourcePath $runSettingsPath `
-Configuration $Configuration `
-Coverage $Coverage

$vstestArgs = @()
$vstestArgs += $testDlls
Expand All @@ -653,6 +721,16 @@ try {
$vstestArgs += "/TestAdapterPath:$adapterPath"
}

if ($Coverage) {
$coverageAdapterPath = Get-CoverageAdapterPath -RepoRoot $PSScriptRoot
if ($coverageAdapterPath) {
$vstestArgs += "/TestAdapterPath:$coverageAdapterPath"
}
else {
Write-Host "[WARN] -Coverage requested but coverlet.collector build folder was not found under packages/coverlet.collector. Run a build first so it restores." -ForegroundColor Yellow
}
}

# Logger configuration - verbosity goes with the console logger
$verbosityMap = @{
'quiet' = 'quiet'; 'q' = 'quiet'
Expand All @@ -672,6 +750,10 @@ try {
$vstestArgs += "/ListTests"
}

if ($Coverage) {
$vstestArgs += '/Collect:XPlat Code Coverage'
}

# =============================================================================
# Run Tests
# =============================================================================
Expand Down Expand Up @@ -736,13 +818,20 @@ try {
foreach ($adapterPath in $nunitAdapterPaths) {
$singleArgs += "/TestAdapterPath:$adapterPath"
}
if ($Coverage -and $coverageAdapterPath) {
$singleArgs += "/TestAdapterPath:$coverageAdapterPath"
}
$singleArgs += "/Logger:trx;LogFileName=${dllName}_${timestamp}.trx"
$singleArgs += "/Logger:console;verbosity=$vstestVerbosity"

if ($TestFilter) {
$singleArgs += "/TestCaseFilter:$TestFilter"
}

if ($Coverage) {
$singleArgs += '/Collect:XPlat Code Coverage'
}

& $vstestPath $singleArgs 2>&1 | Tee-Object -Variable singleTestOutput
$singleExitCode = $LASTEXITCODE
if ($singleExitCode -ne 0 -and $overallExitCode -eq 0) {
Expand All @@ -767,6 +856,40 @@ try {

$script:testExitCode = $overallExitCode
}

# =============================================================================
# Coverage report (only when -Coverage was requested)
# =============================================================================

if ($Coverage -and -not $ListTests) {
$coverageFiles = Get-ChildItem -Path $resultsDir -Filter "coverage.cobertura.xml" -Recurse -ErrorAction SilentlyContinue
if (-not $coverageFiles -or $coverageFiles.Count -eq 0) {
Write-Host "[WARN] -Coverage was requested but no coverage.cobertura.xml files were produced under $resultsDir." -ForegroundColor Yellow
}
else {
Write-Host ""
Write-Host "Generating coverage report from $($coverageFiles.Count) file(s)..." -ForegroundColor Cyan
$coverageReportDir = Join-Path $resultsDir "CoverageReport"
$coverageReportsGlob = Join-Path $resultsDir "*/coverage.cobertura.xml"
try {
# Restore the local tool manifest first (cheap no-op once already restored) so a fresh
# checkout/CI runner doesn't just warn and skip the report.
& dotnet tool restore 2>&1 | Out-Null
& dotnet tool run reportgenerator "-reports:$coverageReportsGlob" "-targetdir:$coverageReportDir" "-reporttypes:TextSummary;Html" 2>&1 |
Where-Object { $_ -notmatch '^\d{4}-\d\d-\d\dT' } # drop ReportGenerator's timestamped progress lines
$summaryPath = Join-Path $coverageReportDir "Summary.txt"
if (Test-Path -LiteralPath $summaryPath) {
Write-Host ""
Get-Content -LiteralPath $summaryPath | Write-Host
Write-Host ""
Write-Host "Full HTML coverage report: $(Join-Path $coverageReportDir 'index.html')" -ForegroundColor Gray
}
}
catch {
Write-Host "[WARN] ReportGenerator failed (is '.config/dotnet-tools.json' restored? run 'dotnet tool restore'): $_" -ForegroundColor Yellow
}
}
}
}
}
finally {
Expand Down
Loading