From a89e67348b95a3cdd87186bbab99030c53378107 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 15:22:04 -0300 Subject: [PATCH 01/24] test: add end-to-end smoke script for server startup and shutdown --- tools/smoke-test.ps1 | 248 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 248 insertions(+) create mode 100644 tools/smoke-test.ps1 diff --git a/tools/smoke-test.ps1 b/tools/smoke-test.ps1 new file mode 100644 index 00000000..e0667d78 --- /dev/null +++ b/tools/smoke-test.ps1 @@ -0,0 +1,248 @@ +<# +.SYNOPSIS + End-to-end smoke test: builds the solution, starts the server, asserts on its + startup log, shuts it down gracefully and asserts on shutdown. + +.DESCRIPTION + Exit codes: + 0 pass + 2 build failed + 3 GameRoot not found + 4 timed out waiting for the server to come online + 5 a forbidden pattern was found in the log + 6 the server did not shut down gracefully +#> +[CmdletBinding()] +param( + [string] $GameRoot = $env:PERPETUUM_GAMEROOT, + [string] $Configuration = 'Release', + [int] $Timeout = 180, + [int] $SettleQuietSeconds = 10, + [int] $SettleTimeout = 120, + [int] $ShutdownTimeout = 120, + [switch] $KeepLog +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +# Patterns that must appear. Absence fails the run. +$RequiredOnline = 'State : \[Online\]' +$RequiredOffline = 'State : \[Off\]' + +# Patterns that must not appear anywhere in the startup log. Presence fails the run. +$ForbiddenPatterns = @( + 'Unhandled exception', + 'System\.InvalidOperationException', + 'System\.NullReferenceException', + 'The current TransactionScope is already complete', + 'nesting level exceeded' +) + +# Values that are measured and printed, never asserted. These legitimately change with +# every content patch, so asserting on them would fail when the game works. Each entry is +# counted by matching lines, because the server logs one line per event and never emits a +# total. The strings are taken from a real startup log, not invented. +$ReportedCounters = [ordered]@{ + 'members spawned' = 'member spawned to zone' + 'flock batches' = 'NPCs created' +} + +$repoRoot = Split-Path -Parent $PSScriptRoot + +function Write-Section([string] $Text) { + Write-Host '' + Write-Host "== $Text" -ForegroundColor Cyan +} + +# --- Phase 0: environment ------------------------------------------------- +Write-Section 'Environment' +if ([string]::IsNullOrWhiteSpace($GameRoot)) { + Write-Host 'GameRoot not supplied and PERPETUUM_GAMEROOT is not set.' -ForegroundColor Red + exit 3 +} +if (-not (Test-Path -LiteralPath $GameRoot)) { + Write-Host "GameRoot not found: $GameRoot" -ForegroundColor Red + exit 3 +} +Write-Host "GameRoot : $GameRoot" +Write-Host "Configuration : $Configuration" + +# --- Phase 1: build ------------------------------------------------------- +Write-Section 'Build' +& dotnet build (Join-Path $repoRoot 'PerpetuumServer2.sln') -c $Configuration -p:Platform=x64 --verbosity quiet +if ($LASTEXITCODE -ne 0) { + Write-Host "Build failed with exit code $LASTEXITCODE." -ForegroundColor Red + exit 2 +} + +# --- Phase 2: launch ------------------------------------------------------ +Write-Section 'Launch' +# Perpetuum.Server has no BaseOutputPath override, so it builds to its own project folder. +# Only Perpetuum.ServerService2.csproj redirects output to the repo-root bin\ directory, which +# is why CLAUDE.md's blanket "Output: bin/x64/Release/net8.0" does not hold for this project. +$serverExe = Join-Path $repoRoot "src\Perpetuum.Server\bin\x64\$Configuration\net8.0\Perpetuum.Server.exe" +if (-not (Test-Path -LiteralPath $serverExe)) { + Write-Host "Server executable not found: $serverExe" -ForegroundColor Red + exit 2 +} + +$logPath = Join-Path ([System.IO.Path]::GetTempPath()) "perpetuum-smoke-$(Get-Date -Format yyyyMMdd-HHmmss).log" +$errPath = "$logPath.err" +Write-Host "Log : $logPath" + +$proc = Start-Process -FilePath $serverExe -ArgumentList "`"$GameRoot`"" ` + -RedirectStandardOutput $logPath -RedirectStandardError $errPath ` + -PassThru -WindowStyle Hidden + +# Start-Process -PassThru combined with output redirection returns a Process object whose +# ExitCode (and Handle) cannot be read later, in this PowerShell version -- confirmed with an +# isolated cmd.exe repro. Open our own handle now, while the process is guaranteed to still be +# running, so Phase 5 can read the real exit code via GetExitCodeProcess instead. +$procApiSignature = @' +[DllImport("kernel32.dll", SetLastError = true)] public static extern IntPtr OpenProcess(uint dwDesiredAccess, bool bInheritHandle, uint dwProcessId); +[DllImport("kernel32.dll", SetLastError = true)] public static extern bool GetExitCodeProcess(IntPtr hProcess, out uint lpExitCode); +'@ +$procApi = Add-Type -MemberDefinition $procApiSignature -Name 'SmokeProcess' -Namespace 'Perpetuum' -PassThru +$PROCESS_QUERY_LIMITED_INFORMATION = 0x1000 +$procHandle = $procApi::OpenProcess($PROCESS_QUERY_LIMITED_INFORMATION, $false, [uint32] $proc.Id) + +# --- Phase 3: wait for online -------------------------------------------- +Write-Section 'Waiting for [Online]' +$deadline = (Get-Date).AddSeconds($Timeout) +$online = $false +$startedAt = Get-Date +while ((Get-Date) -lt $deadline) { + if ($proc.HasExited) { break } + if (Test-Path -LiteralPath $logPath) { + $content = Get-Content -LiteralPath $logPath -Raw -ErrorAction SilentlyContinue + if ($content -and $content -match $RequiredOnline) { $online = $true; break } + } + Start-Sleep -Milliseconds 500 +} +$elapsed = [int]((Get-Date) - $startedAt).TotalSeconds + +if (-not $online) { + Write-Host "Server did not reach [Online] within $Timeout seconds." -ForegroundColor Red + if (-not $proc.HasExited) { Stop-Process -Id $proc.Id -Force } + Write-Host "Log kept for inspection: $logPath" -ForegroundColor Yellow + exit 4 +} +Write-Host "Online after $elapsed seconds." -ForegroundColor Green + +# --- Phase 3b: wait for startup to drain --------------------------------- +# The server reports [Online] well before it finishes spawning NPCs — measured at 34 seconds +# of continued spawning past [Online] on a real run. Sending Ctrl+C at [Online] therefore +# starts a shutdown that has to contend with the rest of startup, and it overruns any sane +# shutdown budget. Wait until the log stops growing before shutting down. +Write-Section 'Waiting for startup to drain' +$settleDeadline = (Get-Date).AddSeconds($SettleTimeout) +$settleStartedAt = Get-Date +$lastSize = -1 +$quietSince = Get-Date +while ((Get-Date) -lt $settleDeadline) { + if ($proc.HasExited) { break } + $size = (Get-Item -LiteralPath $logPath).Length + if ($size -ne $lastSize) { + $lastSize = $size + $quietSince = Get-Date + } elseif (((Get-Date) - $quietSince).TotalSeconds -ge $SettleQuietSeconds) { + break + } + Start-Sleep -Milliseconds 500 +} +$settleElapsed = [int]((Get-Date) - $settleStartedAt).TotalSeconds +Write-Host "Log quiet for $SettleQuietSeconds s after $settleElapsed s. Proceeding to shutdown." + +# --- Phase 4: assert on the startup log ---------------------------------- +Write-Section 'Startup assertions' +$startupLog = Get-Content -LiteralPath $logPath -Raw +$violations = @() +foreach ($pattern in $ForbiddenPatterns) { + if ($startupLog -match $pattern) { $violations += $pattern } +} + +Write-Host 'Reported values (not asserted):' +foreach ($key in $ReportedCounters.Keys) { + $count = ([regex]::Matches($startupLog, $ReportedCounters[$key])).Count + Write-Host (" {0,-18}: {1}" -f $key, $count) +} +Write-Host (" {0,-18}: {1}s" -f 'time to online', $elapsed) +Write-Host (" {0,-18}: {1}s" -f 'time to settle', $settleElapsed) + +# --- Phase 5: graceful shutdown ------------------------------------------ +Write-Section 'Shutdown' +$signature = @' +[DllImport("kernel32.dll", SetLastError = true)] public static extern bool AttachConsole(uint dwProcessId); +[DllImport("kernel32.dll", SetLastError = true)] public static extern bool FreeConsole(); +[DllImport("kernel32.dll")] public static extern bool SetConsoleCtrlHandler(IntPtr handler, bool add); +[DllImport("kernel32.dll")] public static extern bool GenerateConsoleCtrlEvent(uint dwCtrlEvent, uint dwProcessGroupId); +'@ +$kernel = Add-Type -MemberDefinition $signature -Name 'SmokeConsole' -Namespace 'Perpetuum' -PassThru + +$ATTACH_PARENT_PROCESS = [uint32]::MaxValue +$graceful = $false +$attached = $false +$shutdownStartedAt = Get-Date +try { + # A process that already owns a console cannot attach to another one: AttachConsole + # returns false with ERROR_ACCESS_DENIED (5). Release our own console first. + [void] $kernel::FreeConsole() + $attached = $kernel::AttachConsole([uint32] $proc.Id) + if ($attached) { + # MUST come before the event, or the Ctrl+C kills this PowerShell session. + [void] $kernel::SetConsoleCtrlHandler([IntPtr]::Zero, $true) + [void] $kernel::GenerateConsoleCtrlEvent(0, 0) + $graceful = $proc.WaitForExit($ShutdownTimeout * 1000) + } +} finally { + # No Write-Host may run between our FreeConsole above and this restore: with no console + # attached, writing to the host throws. + [void] $kernel::FreeConsole() + [void] $kernel::AttachConsole($ATTACH_PARENT_PROCESS) + [void] $kernel::SetConsoleCtrlHandler([IntPtr]::Zero, $false) +} +$shutdownElapsed = [int]((Get-Date) - $shutdownStartedAt).TotalSeconds +if (-not $attached) { + Write-Host 'AttachConsole failed; cannot deliver Ctrl+C.' -ForegroundColor Yellow +} + +$finalLog = Get-Content -LiteralPath $logPath -Raw +Write-Host ("Shutdown took {0}s." -f $shutdownElapsed) +if (-not $graceful) { + Write-Host "Server did not exit within $ShutdownTimeout seconds of Ctrl+C. Killing it." -ForegroundColor Red + if (-not $proc.HasExited) { Stop-Process -Id $proc.Id -Force } + Write-Host "Log kept for inspection: $logPath" -ForegroundColor Yellow + exit 6 +} +if ($finalLog -notmatch $RequiredOffline) { + Write-Host 'Server exited but never reported [Off].' -ForegroundColor Red + Write-Host "Log kept for inspection: $logPath" -ForegroundColor Yellow + exit 6 +} +[uint32] $serverExitCode = 0 +[void] $procApi::GetExitCodeProcess($procHandle, [ref] $serverExitCode) +if ($serverExitCode -ne 0) { + Write-Host "Server exit code was $serverExitCode, expected 0." -ForegroundColor Red + Write-Host "Log kept for inspection: $logPath" -ForegroundColor Yellow + exit 6 +} +Write-Host 'Graceful shutdown confirmed.' -ForegroundColor Green + +# --- Phase 6: verdict ----------------------------------------------------- +Write-Section 'Verdict' +if ($violations.Count -gt 0) { + Write-Host 'Forbidden patterns found in the log:' -ForegroundColor Red + foreach ($v in $violations) { Write-Host " $v" -ForegroundColor Red } + Write-Host "Log kept for inspection: $logPath" -ForegroundColor Yellow + exit 5 +} + +Write-Host 'SMOKE TEST PASSED' -ForegroundColor Green +if (-not $KeepLog) { + Remove-Item -LiteralPath $logPath -ErrorAction SilentlyContinue + Remove-Item -LiteralPath $errPath -ErrorAction SilentlyContinue +} else { + Write-Host "Log kept: $logPath" +} +exit 0 From c8d796e0828edfe84539ca14abcc7a2acf31253d Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 15:48:38 -0300 Subject: [PATCH 02/24] test: check Win32 return values and add top-level trap for exit 7 Fixes two Important review findings on tools/smoke-test.ps1: - OpenProcess and GetExitCodeProcess return values were unchecked; a failure would silently leave the exit-code variable at 0 and report a graceful shutdown that never happened. Both are now checked and route to a thrown error with a distinct message on failure. - No top-level trap existed, so an unhandled terminating error (missing dotnet, unreadable log, failed P/Invoke) would exit through PowerShell's own default code instead of a documented one. Added exit code 7 and a trap immediately after $ErrorActionPreference = 'Stop', per the patched plan. --- tools/smoke-test.ps1 | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tools/smoke-test.ps1 b/tools/smoke-test.ps1 index e0667d78..abca9579 100644 --- a/tools/smoke-test.ps1 +++ b/tools/smoke-test.ps1 @@ -11,6 +11,7 @@ 4 timed out waiting for the server to come online 5 a forbidden pattern was found in the log 6 the server did not shut down gracefully + 7 unexpected error #> [CmdletBinding()] param( @@ -26,6 +27,15 @@ param( Set-StrictMode -Version Latest $ErrorActionPreference = 'Stop' +# Any terminating error that no phase handles leaves through a documented code rather than +# PowerShell's own. Without this, a missing dotnet, an unreadable log, or a failed P/Invoke +# would exit with a code the script never documented. +trap { + Write-Host "Unexpected error: $_" -ForegroundColor Red + Write-Host $_.ScriptStackTrace -ForegroundColor DarkGray + exit 7 +} + # Patterns that must appear. Absence fails the run. $RequiredOnline = 'State : \[Online\]' $RequiredOffline = 'State : \[Off\]' @@ -96,9 +106,12 @@ $proc = Start-Process -FilePath $serverExe -ArgumentList "`"$GameRoot`"" ` -PassThru -WindowStyle Hidden # Start-Process -PassThru combined with output redirection returns a Process object whose -# ExitCode (and Handle) cannot be read later, in this PowerShell version -- confirmed with an -# isolated cmd.exe repro. Open our own handle now, while the process is guaranteed to still be -# running, so Phase 5 can read the real exit code via GetExitCodeProcess instead. +# ExitCode (and Handle) cannot be read later, in this PowerShell version. Open our own handle +# now, while the process is guaranteed to still be running, so Phase 5 can read the real exit +# code via GetExitCodeProcess. Both Win32 return values are checked below: an unchecked failure +# here would silently leave the exit-code variable at its default and report a graceful +# shutdown that never happened. Either failure is a failed P/Invoke, which is exactly what the +# trap above exists to turn into a documented exit 7 instead of a default PowerShell exit. $procApiSignature = @' [DllImport("kernel32.dll", SetLastError = true)] public static extern IntPtr OpenProcess(uint dwDesiredAccess, bool bInheritHandle, uint dwProcessId); [DllImport("kernel32.dll", SetLastError = true)] public static extern bool GetExitCodeProcess(IntPtr hProcess, out uint lpExitCode); @@ -106,6 +119,10 @@ $procApiSignature = @' $procApi = Add-Type -MemberDefinition $procApiSignature -Name 'SmokeProcess' -Namespace 'Perpetuum' -PassThru $PROCESS_QUERY_LIMITED_INFORMATION = 0x1000 $procHandle = $procApi::OpenProcess($PROCESS_QUERY_LIMITED_INFORMATION, $false, [uint32] $proc.Id) +if ($procHandle -eq [IntPtr]::Zero) { + $lastError = [System.Runtime.InteropServices.Marshal]::GetLastWin32Error() + throw "OpenProcess failed for PID $($proc.Id) (Win32 error $lastError). Cannot verify the server's exit code later." +} # --- Phase 3: wait for online -------------------------------------------- Write-Section 'Waiting for [Online]' @@ -221,7 +238,11 @@ if ($finalLog -notmatch $RequiredOffline) { exit 6 } [uint32] $serverExitCode = 0 -[void] $procApi::GetExitCodeProcess($procHandle, [ref] $serverExitCode) +$gotExitCode = $procApi::GetExitCodeProcess($procHandle, [ref] $serverExitCode) +if (-not $gotExitCode) { + $lastError = [System.Runtime.InteropServices.Marshal]::GetLastWin32Error() + throw "GetExitCodeProcess failed for PID $($proc.Id) (Win32 error $lastError). Cannot verify the server's exit code." +} if ($serverExitCode -ne 0) { Write-Host "Server exit code was $serverExitCode, expected 0." -ForegroundColor Red Write-Host "Log kept for inspection: $logPath" -ForegroundColor Yellow From 60be7ba15b2db419be024ad4a518d973b8236c05 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:33:22 -0300 Subject: [PATCH 03/24] test: add Perpetuum.Tests project with ValueTypeExtensions coverage --- PerpetuumServer2.sln | 7 ++++ src/Perpetuum.Tests/Perpetuum.Tests.csproj | 22 ++++++++++ .../Unit/ValueTypeExtensionsTests.cs | 42 +++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 src/Perpetuum.Tests/Perpetuum.Tests.csproj create mode 100644 src/Perpetuum.Tests/Unit/ValueTypeExtensionsTests.cs diff --git a/PerpetuumServer2.sln b/PerpetuumServer2.sln index d9d55f4a..0d3e9974 100644 --- a/PerpetuumServer2.sln +++ b/PerpetuumServer2.sln @@ -23,6 +23,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Perpetuum.ServerService2", EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Perpetuum.AdminTool", "src\Perpetuum.AdminTool\Perpetuum.AdminTool.csproj", "{A7D1E3C5-9F4B-42E8-8A6C-B5D7F1E9C2A0}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Perpetuum.Tests", "src\Perpetuum.Tests\Perpetuum.Tests.csproj", "{C8C45427-6E5C-496C-9446-0817EADC05DD}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|x64 = Debug|x64 @@ -65,6 +67,10 @@ Global {A7D1E3C5-9F4B-42E8-8A6C-B5D7F1E9C2A0}.Debug|x64.Build.0 = Debug|x64 {A7D1E3C5-9F4B-42E8-8A6C-B5D7F1E9C2A0}.Release|x64.ActiveCfg = Release|x64 {A7D1E3C5-9F4B-42E8-8A6C-B5D7F1E9C2A0}.Release|x64.Build.0 = Release|x64 + {C8C45427-6E5C-496C-9446-0817EADC05DD}.Debug|x64.ActiveCfg = Debug|x64 + {C8C45427-6E5C-496C-9446-0817EADC05DD}.Debug|x64.Build.0 = Debug|x64 + {C8C45427-6E5C-496C-9446-0817EADC05DD}.Release|x64.ActiveCfg = Release|x64 + {C8C45427-6E5C-496C-9446-0817EADC05DD}.Release|x64.Build.0 = Release|x64 EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -79,6 +85,7 @@ Global {D3F8A2B1-6C9E-4D7F-A5B8-2E0C4F9A3D6B} = {0BC991A1-133C-49ED-A141-80E2A906898B} {A2FCE930-E013-4731-B354-F7DA00322D38} = {0BC991A1-133C-49ED-A141-80E2A906898B} {A7D1E3C5-9F4B-42E8-8A6C-B5D7F1E9C2A0} = {0BC991A1-133C-49ED-A141-80E2A906898B} + {C8C45427-6E5C-496C-9446-0817EADC05DD} = {0BC991A1-133C-49ED-A141-80E2A906898B} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {91874B60-30B0-4B48-9155-370E40BC70E6} diff --git a/src/Perpetuum.Tests/Perpetuum.Tests.csproj b/src/Perpetuum.Tests/Perpetuum.Tests.csproj new file mode 100644 index 00000000..96019f2a --- /dev/null +++ b/src/Perpetuum.Tests/Perpetuum.Tests.csproj @@ -0,0 +1,22 @@ + + + + net8.0 + enable + annotations + x64 + false + + + + + + + + + + + + + + diff --git a/src/Perpetuum.Tests/Unit/ValueTypeExtensionsTests.cs b/src/Perpetuum.Tests/Unit/ValueTypeExtensionsTests.cs new file mode 100644 index 00000000..b6b17844 --- /dev/null +++ b/src/Perpetuum.Tests/Unit/ValueTypeExtensionsTests.cs @@ -0,0 +1,42 @@ +using Xunit; + +namespace Perpetuum.Tests.Unit +{ + public class ValueTypeExtensionsTests + { + [Theory] + [InlineData(5, 10, 40, 10)] + [InlineData(50, 10, 40, 40)] + [InlineData(25, 10, 40, 25)] + [InlineData(10, 10, 40, 10)] + [InlineData(40, 10, 40, 40)] + public void Clamp_int_bounds_the_value(int value, int lower, int upper, int expected) + { + Assert.Equal(expected, value.Clamp(lower, upper)); + } + + [Theory] + [InlineData(0, false)] + [InlineData(1, true)] + [InlineData(-1, false)] + [InlineData(int.MaxValue, true)] + public void ToBool_is_true_only_above_zero(int value, bool expected) + { + Assert.Equal(expected, value.ToBool()); + } + + [Fact] + public void Min_returns_the_lower_of_the_two() + { + Assert.Equal(3, 7.Min(3)); + Assert.Equal(3, 3.Min(7)); + } + + [Fact] + public void Max_returns_the_higher_of_the_two() + { + Assert.Equal(7, 7.Max(3)); + Assert.Equal(7, 3.Max(7)); + } + } +} From 47c261b2a8a78bee343987c15bb9390564e9a29f Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:41:32 -0300 Subject: [PATCH 04/24] test: declare Windows platform support to fix CA1416 warnings Perpetuum.Tests references Perpetuum.csproj, which is marked SupportedOSPlatform("windows"). Every other consumer of Perpetuum.csproj declares the same attribute itself; Perpetuum.Tests did not, which is why the analyzer fired CA1416 on every ValueTypeExtensions call. --- src/Perpetuum.Tests/AssemblyInfo.cs | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 src/Perpetuum.Tests/AssemblyInfo.cs diff --git a/src/Perpetuum.Tests/AssemblyInfo.cs b/src/Perpetuum.Tests/AssemblyInfo.cs new file mode 100644 index 00000000..d04ca728 --- /dev/null +++ b/src/Perpetuum.Tests/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.Versioning; + +[assembly: SupportedOSPlatform("windows")] From 9262d6525ed9179d0af89dc66a7f6ba966dd80b6 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:50:43 -0300 Subject: [PATCH 05/24] test: cover Guard throw-if extension methods --- src/Perpetuum.Tests/Unit/GuardTests.cs | 130 +++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 src/Perpetuum.Tests/Unit/GuardTests.cs diff --git a/src/Perpetuum.Tests/Unit/GuardTests.cs b/src/Perpetuum.Tests/Unit/GuardTests.cs new file mode 100644 index 00000000..22bb17bf --- /dev/null +++ b/src/Perpetuum.Tests/Unit/GuardTests.cs @@ -0,0 +1,130 @@ +using Xunit; + +namespace Perpetuum.Tests.Unit +{ + // ErrorCodes lives in the Perpetuum namespace (src/Perpetuum/ErrorCodes.cs), which this + // file's namespace resolves to without a using directive. + public class GuardTests + { + [Fact] + public void ThrowIfZero_int_throws_on_zero() + { + PerpetuumException ex = Assert.Throws( + () => 0.ThrowIfZero(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + + Assert.Equal(ErrorCodes.WTFErrorMedicalAttentionSuggested, ex.error); + } + + [Fact] + public void ThrowIfZero_int_passes_on_non_zero() + { + 1.ThrowIfZero(ErrorCodes.WTFErrorMedicalAttentionSuggested); + } + + [Fact] + public void ThrowIfNull_throws_on_null_and_returns_the_value_otherwise() + { + object? nothing = null; + _ = Assert.Throws( + () => nothing.ThrowIfNull(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + + object something = new(); + Assert.Same(something, something.ThrowIfNull(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + + [Fact] + public void ThrowIfTrue_and_ThrowIfFalse_are_mirror_images() + { + _ = Assert.Throws( + () => true.ThrowIfTrue(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + false.ThrowIfTrue(ErrorCodes.WTFErrorMedicalAttentionSuggested); + + _ = Assert.Throws( + () => false.ThrowIfFalse(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + true.ThrowIfFalse(ErrorCodes.WTFErrorMedicalAttentionSuggested); + } + + [Theory] + [InlineData(5, 3, true)] + [InlineData(3, 3, false)] + [InlineData(1, 3, false)] + public void ThrowIfGreater_throws_only_when_strictly_greater(int source, int comparer, bool shouldThrow) + { + if (shouldThrow) + { + _ = Assert.Throws( + () => source.ThrowIfGreater(comparer, ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + else + { + Assert.Equal(source, source.ThrowIfGreater(comparer, ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + } + + [Theory] + [InlineData(5, 3, false)] + [InlineData(3, 3, true)] + [InlineData(1, 3, true)] + public void ThrowIfLessOrEqual_throws_at_and_below_the_comparer(int source, int comparer, bool shouldThrow) + { + if (shouldThrow) + { + _ = Assert.Throws( + () => source.ThrowIfLessOrEqual(comparer, ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + else + { + Assert.Equal(source, source.ThrowIfLessOrEqual(comparer, ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + } + + [Fact] + public void ThrowIfError_passes_NoError_through_and_throws_on_anything_else() + { + Assert.Equal(ErrorCodes.NoError, ErrorCodes.NoError.ThrowIfError()); + + PerpetuumException ex = Assert.Throws( + () => ErrorCodes.WTFErrorMedicalAttentionSuggested.ThrowIfError()); + + Assert.Equal(ErrorCodes.WTFErrorMedicalAttentionSuggested, ex.error); + } + + [Fact] + public void ThrowIfError_invokes_the_exception_action_before_throwing() + { + bool invoked = false; + + _ = Assert.Throws( + () => ErrorCodes.WTFErrorMedicalAttentionSuggested.ThrowIfError(_ => invoked = true)); + + Assert.True(invoked); + } + + [Theory] + [InlineData(null, true)] + [InlineData("", true)] + [InlineData("x", false)] + public void ThrowIfNullOrEmpty_rejects_null_and_empty(string? text, bool shouldThrow) + { + if (shouldThrow) + { + _ = Assert.Throws( + () => text!.ThrowIfNullOrEmpty(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + else + { + Assert.Equal(text, text!.ThrowIfNullOrEmpty(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + } + + [Fact] + public void ThrowIfNotType_returns_the_cast_value_and_rejects_the_wrong_type() + { + object value = "text"; + Assert.Equal("text", value.ThrowIfNotType(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + + _ = Assert.Throws( + () => value.ThrowIfNotType(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + } +} From bfda47a7ba4a43e968aa87e8f8d7448843f8d925 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 19:00:24 -0300 Subject: [PATCH 06/24] test: cover Guard throw-if extension methods --- src/Perpetuum.Tests/Unit/GuardTests.cs | 88 ++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/src/Perpetuum.Tests/Unit/GuardTests.cs b/src/Perpetuum.Tests/Unit/GuardTests.cs index 22bb17bf..3ee22263 100644 --- a/src/Perpetuum.Tests/Unit/GuardTests.cs +++ b/src/Perpetuum.Tests/Unit/GuardTests.cs @@ -126,5 +126,93 @@ public void ThrowIfNotType_returns_the_cast_value_and_rejects_the_wrong_type() _ = Assert.Throws( () => value.ThrowIfNotType(ErrorCodes.WTFErrorMedicalAttentionSuggested)); } + + [Fact] + public void ThrowIfType_rejects_the_named_type_and_passes_anything_else() + { + object value = "text"; + + _ = Assert.Throws( + () => value.ThrowIfType(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + + value.ThrowIfType(ErrorCodes.WTFErrorMedicalAttentionSuggested); + } + + [Fact] + public void ThrowIfZero_long_throws_on_zero() + { + // Not an extension method: Guard.ThrowIfZero(long, Func) takes its source + // as a plain parameter, unlike the int overload above it. + _ = Assert.Throws( + () => Guard.ThrowIfZero(0L, () => PerpetuumException.Create(ErrorCodes.WTFErrorMedicalAttentionSuggested))); + + Guard.ThrowIfZero(1L, () => PerpetuumException.Create(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + + [Fact] + public void ThrowIfZero_double_rejects_values_within_epsilon_of_zero() + { + _ = Assert.Throws( + () => 0.0d.ThrowIfZero(() => PerpetuumException.Create(ErrorCodes.WTFErrorMedicalAttentionSuggested))); + + Assert.Equal( + 2.5d, + 2.5d.ThrowIfZero(() => PerpetuumException.Create(ErrorCodes.WTFErrorMedicalAttentionSuggested))); + } + + [Theory] + [InlineData(5, 3, true)] + [InlineData(3, 3, true)] + [InlineData(1, 3, false)] + public void ThrowIfGreaterOrEqual_throws_at_and_above_the_comparer(int source, int comparer, bool shouldThrow) + { + if (shouldThrow) + { + _ = Assert.Throws( + () => source.ThrowIfGreaterOrEqual(comparer, ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + else + { + Assert.Equal(source, source.ThrowIfGreaterOrEqual(comparer, ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + } + + [Theory] + [InlineData(1, 3, true)] + [InlineData(3, 3, false)] + [InlineData(5, 3, false)] + public void ThrowIfLess_throws_only_when_strictly_less(int source, int comparer, bool shouldThrow) + { + if (shouldThrow) + { + _ = Assert.Throws( + () => source.ThrowIfLess(comparer, ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + else + { + Assert.Equal(source, source.ThrowIfLess(comparer, ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + } + + [Fact] + public void ThrowIfNotNull_throws_when_the_value_is_present() + { + object? nothing = null; + nothing.ThrowIfNotNull(ErrorCodes.WTFErrorMedicalAttentionSuggested); + + _ = Assert.Throws( + () => new object().ThrowIfNotNull(ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + + [Fact] + public void ThrowIfNotNull_invokes_the_exception_action_before_throwing() + { + bool invoked = false; + + _ = Assert.Throws( + () => new object().ThrowIfNotNull(ErrorCodes.WTFErrorMedicalAttentionSuggested, _ => invoked = true)); + + Assert.True(invoked); + } } } From 45a56f8301c7aaea9967858c37de77a484a0b666 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 19:13:19 -0300 Subject: [PATCH 07/24] test: cover Guard throw-if extension methods --- src/Perpetuum.Tests/Unit/GuardTests.cs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/Perpetuum.Tests/Unit/GuardTests.cs b/src/Perpetuum.Tests/Unit/GuardTests.cs index 3ee22263..f3b4f016 100644 --- a/src/Perpetuum.Tests/Unit/GuardTests.cs +++ b/src/Perpetuum.Tests/Unit/GuardTests.cs @@ -214,5 +214,29 @@ public void ThrowIfNotNull_invokes_the_exception_action_before_throwing() Assert.True(invoked); } + + [Fact] + public void ThrowIfEqual_throws_on_equality_and_returns_the_value_otherwise() + { + // Guard.cs:110 is a separate implementation from the Func overload at + // Guard.cs:216 — it does not delegate to it, and nothing else in Guard reaches it. + // Without this test the overload has no coverage, direct or transitive, despite being + // used across EntityRepository, ZoneSession and EntityDefault. + _ = Assert.Throws( + () => 7.ThrowIfEqual(7, ErrorCodes.WTFErrorMedicalAttentionSuggested)); + + Assert.Equal(7, 7.ThrowIfEqual(3, ErrorCodes.WTFErrorMedicalAttentionSuggested)); + } + + [Fact] + public void ThrowIfEqual_invokes_the_exception_action_before_throwing() + { + bool invoked = false; + + _ = Assert.Throws( + () => 7.ThrowIfEqual(7, ErrorCodes.WTFErrorMedicalAttentionSuggested, _ => invoked = true)); + + Assert.True(invoked); + } } } From b695060f23868edffef8f03d0b0583d9fe08bc4b Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 19:28:38 -0300 Subject: [PATCH 08/24] test: add recording logger and static service locator fixture --- src/Perpetuum.Tests/Fakes/RecordingLogger.cs | 37 +++++++++++++++++++ .../AssemblyLoggerInitializer.cs | 27 ++++++++++++++ .../PerpetuumStaticsCollection.cs | 16 ++++++++ .../Infrastructure/PerpetuumStaticsFixture.cs | 36 ++++++++++++++++++ .../Unit/RecordingLoggerTests.cs | 34 +++++++++++++++++ 5 files changed, 150 insertions(+) create mode 100644 src/Perpetuum.Tests/Fakes/RecordingLogger.cs create mode 100644 src/Perpetuum.Tests/Infrastructure/AssemblyLoggerInitializer.cs create mode 100644 src/Perpetuum.Tests/Infrastructure/PerpetuumStaticsCollection.cs create mode 100644 src/Perpetuum.Tests/Infrastructure/PerpetuumStaticsFixture.cs create mode 100644 src/Perpetuum.Tests/Unit/RecordingLoggerTests.cs diff --git a/src/Perpetuum.Tests/Fakes/RecordingLogger.cs b/src/Perpetuum.Tests/Fakes/RecordingLogger.cs new file mode 100644 index 00000000..48992502 --- /dev/null +++ b/src/Perpetuum.Tests/Fakes/RecordingLogger.cs @@ -0,0 +1,37 @@ +using Perpetuum.Log; + +namespace Perpetuum.Tests.Fakes +{ + /// + /// Captures every log event so tests can assert on what the code under test reported. + /// Thread-safe: production code logs from timer and task threads. + /// + public sealed class RecordingLogger : ILogger + { + private readonly List _events = []; + private readonly object _gate = new(); + + public void Log(LogEvent logEvent) + { + lock (_gate) + { + _events.Add(logEvent); + } + } + + public IReadOnlyList Events + { + get { lock (_gate) { return [.. _events]; } } + } + + public IReadOnlyList Exceptions + { + get { lock (_gate) { return [.. _events.Where(e => e.LogType == LogType.Error && e.ThrownException != null)]; } } + } + + public void Clear() + { + lock (_gate) { _events.Clear(); } + } + } +} diff --git a/src/Perpetuum.Tests/Infrastructure/AssemblyLoggerInitializer.cs b/src/Perpetuum.Tests/Infrastructure/AssemblyLoggerInitializer.cs new file mode 100644 index 00000000..12677129 --- /dev/null +++ b/src/Perpetuum.Tests/Infrastructure/AssemblyLoggerInitializer.cs @@ -0,0 +1,27 @@ +using Perpetuum.Log; +using Perpetuum.Tests.Fakes; + +namespace Perpetuum.Tests.Infrastructure +{ + /// + /// Logger.Current is declared as { private get; set; }, so its previous value cannot be + /// read and therefore cannot be restored. It is assigned once for the whole assembly. + /// + public static class AssemblyLoggerInitializer + { + public static RecordingLogger Instance { get; } = new RecordingLogger(); + + private static bool _installed; + private static readonly object Gate = new(); + + public static void Install() + { + lock (Gate) + { + if (_installed) return; + Logger.Current = Instance; + _installed = true; + } + } + } +} diff --git a/src/Perpetuum.Tests/Infrastructure/PerpetuumStaticsCollection.cs b/src/Perpetuum.Tests/Infrastructure/PerpetuumStaticsCollection.cs new file mode 100644 index 00000000..6f34f53d --- /dev/null +++ b/src/Perpetuum.Tests/Infrastructure/PerpetuumStaticsCollection.cs @@ -0,0 +1,16 @@ +using Xunit; + +namespace Perpetuum.Tests.Infrastructure +{ + /// + /// Every test class that assigns a static service locator carries + /// [Collection(PerpetuumStaticsCollection.Name)]. xUnit runs the classes in one collection + /// serially, which is what keeps static assignment from racing. Tests that touch no static + /// stay outside this collection and keep running in parallel. + /// + [CollectionDefinition(Name)] + public class PerpetuumStaticsCollection : ICollectionFixture + { + public const string Name = "Perpetuum statics"; + } +} diff --git a/src/Perpetuum.Tests/Infrastructure/PerpetuumStaticsFixture.cs b/src/Perpetuum.Tests/Infrastructure/PerpetuumStaticsFixture.cs new file mode 100644 index 00000000..02c535c2 --- /dev/null +++ b/src/Perpetuum.Tests/Infrastructure/PerpetuumStaticsFixture.cs @@ -0,0 +1,36 @@ +using Perpetuum.Data; +using Perpetuum.EntityFramework; +using Perpetuum.Tests.Fakes; + +namespace Perpetuum.Tests.Infrastructure +{ + /// + /// Saves the settable static service locators, installs the recording logger, and restores + /// the readable locators on dispose. Shared by every test class in the statics collection, + /// which xUnit runs serially. + /// + public sealed class PerpetuumStaticsFixture : IDisposable + { + private readonly Func _savedDbQueryFactory; + private readonly IEntityDefaultReader _savedDefaultReader; + private readonly IEntityServices _savedEntityServices; + + public PerpetuumStaticsFixture() + { + _savedDbQueryFactory = Db.DbQueryFactory; + _savedDefaultReader = EntityDefault.Reader; + _savedEntityServices = Entity.Services; + + AssemblyLoggerInitializer.Install(); + } + + public RecordingLogger Logger => AssemblyLoggerInitializer.Instance; + + public void Dispose() + { + Db.DbQueryFactory = _savedDbQueryFactory; + EntityDefault.Reader = _savedDefaultReader; + Entity.Services = _savedEntityServices; + } + } +} diff --git a/src/Perpetuum.Tests/Unit/RecordingLoggerTests.cs b/src/Perpetuum.Tests/Unit/RecordingLoggerTests.cs new file mode 100644 index 00000000..50a99ecc --- /dev/null +++ b/src/Perpetuum.Tests/Unit/RecordingLoggerTests.cs @@ -0,0 +1,34 @@ +using Perpetuum.Log; +using Perpetuum.Tests.Infrastructure; +using Xunit; + +namespace Perpetuum.Tests.Unit +{ + [Collection(PerpetuumStaticsCollection.Name)] + public class RecordingLoggerTests + { + private readonly PerpetuumStaticsFixture _fixture; + + public RecordingLoggerTests(PerpetuumStaticsFixture fixture) + { + _fixture = fixture; + _fixture.Logger.Clear(); + } + + [Fact] + public void Info_is_recorded() + { + Logger.Info("hello"); + + Assert.Contains(_fixture.Logger.Events, e => e.Message == "hello"); + } + + [Fact] + public void Exception_is_recorded_as_an_exception_event() + { + Logger.Exception(new InvalidOperationException("boom")); + + Assert.Single(_fixture.Logger.Exceptions); + } + } +} From 604f51130979076de9c175c44315cb7ee5fcbe52 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 19:44:24 -0300 Subject: [PATCH 09/24] test: add recording fake for the ADO.NET data layer --- .../Fakes/Data/FakeDataReader.cs | 74 +++++++++++++++ src/Perpetuum.Tests/Fakes/Data/FakeDb.cs | 89 +++++++++++++++++++ .../Fakes/Data/FakeDbCommand.cs | 75 ++++++++++++++++ .../Fakes/Data/FakeDbConnection.cs | 37 ++++++++ .../Fakes/Data/FakeResultSet.cs | 28 ++++++ 5 files changed, 303 insertions(+) create mode 100644 src/Perpetuum.Tests/Fakes/Data/FakeDataReader.cs create mode 100644 src/Perpetuum.Tests/Fakes/Data/FakeDb.cs create mode 100644 src/Perpetuum.Tests/Fakes/Data/FakeDbCommand.cs create mode 100644 src/Perpetuum.Tests/Fakes/Data/FakeDbConnection.cs create mode 100644 src/Perpetuum.Tests/Fakes/Data/FakeResultSet.cs diff --git a/src/Perpetuum.Tests/Fakes/Data/FakeDataReader.cs b/src/Perpetuum.Tests/Fakes/Data/FakeDataReader.cs new file mode 100644 index 00000000..755a02b9 --- /dev/null +++ b/src/Perpetuum.Tests/Fakes/Data/FakeDataReader.cs @@ -0,0 +1,74 @@ +using System.Data; + +namespace Perpetuum.Tests.Fakes.Data +{ + public sealed class FakeDataReader(FakeResultSet resultSet) : IDataReader + { + private readonly FakeResultSet _resultSet = resultSet; + private int _index = -1; + + private object?[] Current => _resultSet.Rows[_index]; + + public bool Read() + { + _index++; + return _index < _resultSet.Rows.Count; + } + + public int FieldCount => _resultSet.ColumnNames.Count; + public string GetName(int i) => _resultSet.ColumnNames[i]; + public int GetOrdinal(string name) + { + for (int i = 0; i < _resultSet.ColumnNames.Count; i++) + { + if (string.Equals(_resultSet.ColumnNames[i], name, StringComparison.OrdinalIgnoreCase)) + { + return i; + } + } + + throw new IndexOutOfRangeException(name); + } + + public object GetValue(int i) => Current[i] ?? DBNull.Value; + public bool IsDBNull(int i) => Current[i] is null; + public object this[int i] => GetValue(i); + public object this[string name] => GetValue(GetOrdinal(name)); + + public bool GetBoolean(int i) => (bool)GetValue(i); + public byte GetByte(int i) => (byte)GetValue(i); + public char GetChar(int i) => (char)GetValue(i); + public DateTime GetDateTime(int i) => (DateTime)GetValue(i); + public decimal GetDecimal(int i) => (decimal)GetValue(i); + public double GetDouble(int i) => (double)GetValue(i); + public float GetFloat(int i) => (float)GetValue(i); + public Guid GetGuid(int i) => (Guid)GetValue(i); + public short GetInt16(int i) => (short)GetValue(i); + public int GetInt32(int i) => (int)GetValue(i); + public long GetInt64(int i) => (long)GetValue(i); + public string GetString(int i) => (string)GetValue(i); + public Type GetFieldType(int i) => Current[i]?.GetType() ?? typeof(object); + public string GetDataTypeName(int i) => GetFieldType(i).Name; + + public int GetValues(object[] values) + { + int count = Math.Min(values.Length, FieldCount); + for (int i = 0; i < count; i++) { values[i] = GetValue(i); } + return count; + } + + public long GetBytes(int i, long fieldOffset, byte[]? buffer, int bufferoffset, int length) + => throw new NotSupportedException(); + public long GetChars(int i, long fieldoffset, char[]? buffer, int bufferoffset, int length) + => throw new NotSupportedException(); + public IDataReader GetData(int i) => throw new NotSupportedException(); + + public int Depth => 0; + public bool IsClosed { get; private set; } + public int RecordsAffected => -1; + public void Close() => IsClosed = true; + public DataTable? GetSchemaTable() => null; + public bool NextResult() => false; + public void Dispose() => Close(); + } +} diff --git a/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs b/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs new file mode 100644 index 00000000..2f77042b --- /dev/null +++ b/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs @@ -0,0 +1,89 @@ +using System.Data; +using System.Transactions; +using Perpetuum.Data; + +namespace Perpetuum.Tests.Fakes.Data +{ + public sealed record RecordedCommand + { + public required string CommandText { get; init; } + public required IReadOnlyDictionary Parameters { get; init; } + public required int CommandTimeout { get; init; } + public required bool HadAmbientTransaction { get; init; } + public required TransactionStatus? AmbientTransactionStatus { get; init; } + } + + /// + /// Installs a fake data layer into Db.DbQueryFactory, which funnels every Db.Query() call + /// site in the codebase. Results are matched by substring against the command text. + /// + public sealed class FakeDb + { + private readonly List<(string Match, FakeResultSet Result)> _results = []; + private readonly List<(string Match, int RowsAffected)> _nonQueries = []; + private readonly List _commands = []; + private readonly object _gate = new(); + + public static FakeDb Install() + { + FakeDb fake = new(); + Db.DbQueryFactory = () => new DbQuery(() => new FakeDbConnection(fake)); + return fake; + } + + public void When(string commandTextContains, FakeResultSet result) + => _results.Add((commandTextContains, result)); + + public void WhenNonQuery(string commandTextContains, int rowsAffected) + => _nonQueries.Add((commandTextContains, rowsAffected)); + + public IReadOnlyList Commands + { + get { lock (_gate) { return [.. _commands]; } } + } + + public RecordedCommand? LastCommandMatching(string commandTextContains) + => Commands.LastOrDefault(c => c.CommandText.Contains(commandTextContains, StringComparison.OrdinalIgnoreCase)); + + internal int RowsAffectedFor(string commandText) + { + foreach ((string match, int rows) in _nonQueries) + { + if (commandText.Contains(match, StringComparison.OrdinalIgnoreCase)) { return rows; } + } + + return 0; + } + + internal FakeResultSet Record(FakeDbCommand command) + { + Dictionary parameters = []; + foreach (object p in command.Parameters) + { + FakeParameter parameter = (FakeParameter)p; + parameters[parameter.ParameterName] = parameter.Value is DBNull ? null : parameter.Value; + } + + Transaction? ambient = command.OwnerConnection.AmbientTransactionAtOpen; + + lock (_gate) + { + _commands.Add(new RecordedCommand + { + CommandText = command.CommandText, + Parameters = parameters, + CommandTimeout = command.CommandTimeout, + HadAmbientTransaction = ambient != null, + AmbientTransactionStatus = ambient?.TransactionInformation.Status, + }); + } + + foreach ((string match, FakeResultSet result) in _results) + { + if (command.CommandText.Contains(match, StringComparison.OrdinalIgnoreCase)) { return result; } + } + + return FakeResultSet.Empty(); + } + } +} diff --git a/src/Perpetuum.Tests/Fakes/Data/FakeDbCommand.cs b/src/Perpetuum.Tests/Fakes/Data/FakeDbCommand.cs new file mode 100644 index 00000000..5b420316 --- /dev/null +++ b/src/Perpetuum.Tests/Fakes/Data/FakeDbCommand.cs @@ -0,0 +1,75 @@ +using System.Data; + +namespace Perpetuum.Tests.Fakes.Data +{ + public sealed class FakeDbCommand(FakeDbConnection connection) : IDbCommand + { + private readonly FakeDbConnection _connection = connection; + + /// + /// The connection that created this command. DbQuery.ExecuteHelper never assigns the + /// IDbCommand.Connection property, so the fake carries its own back-reference rather + /// than reading one that is always null. + /// + internal FakeDbConnection OwnerConnection => _connection; + + public string CommandText { get; set; } = string.Empty; + public int CommandTimeout { get; set; } + public CommandType CommandType { get; set; } + public IDbConnection? Connection { get; set; } + public IDataParameterCollection Parameters { get; } = new FakeParameterCollection(); + public IDbTransaction? Transaction { get; set; } + public UpdateRowSource UpdatedRowSource { get; set; } + + public void Cancel() { } + public IDbDataParameter CreateParameter() => new FakeParameter(); + public void Prepare() { } + public void Dispose() { } + + public IDataReader ExecuteReader() => new FakeDataReader(_connection.Owner.Record(this)); + public IDataReader ExecuteReader(CommandBehavior behavior) => ExecuteReader(); + + public int ExecuteNonQuery() + { + _ = _connection.Owner.Record(this); + return _connection.Owner.RowsAffectedFor(CommandText); + } + + public object? ExecuteScalar() + { + FakeResultSet result = _connection.Owner.Record(this); + return result.Rows.Count == 0 ? null : result.Rows[0][0]; + } + } + + public sealed class FakeParameter : IDbDataParameter + { + public byte Precision { get; set; } + public byte Scale { get; set; } + public int Size { get; set; } + public DbType DbType { get; set; } + public ParameterDirection Direction { get; set; } + public bool IsNullable => true; + public string ParameterName { get; set; } = string.Empty; + public string SourceColumn { get; set; } = string.Empty; + public DataRowVersion SourceVersion { get; set; } + public object? Value { get; set; } + } + + public sealed class FakeParameterCollection : List, IDataParameterCollection + { + public object this[string parameterName] + { + get => this.Cast().First(p => p.ParameterName == parameterName); + set => throw new NotSupportedException(); + } + + public bool Contains(string parameterName) + => this.Cast().Any(p => p.ParameterName == parameterName); + + public int IndexOf(string parameterName) + => FindIndex(p => ((FakeParameter)p).ParameterName == parameterName); + + public void RemoveAt(string parameterName) => RemoveAt(IndexOf(parameterName)); + } +} diff --git a/src/Perpetuum.Tests/Fakes/Data/FakeDbConnection.cs b/src/Perpetuum.Tests/Fakes/Data/FakeDbConnection.cs new file mode 100644 index 00000000..ea39fbbc --- /dev/null +++ b/src/Perpetuum.Tests/Fakes/Data/FakeDbConnection.cs @@ -0,0 +1,37 @@ +using System.Data; +using System.Transactions; + +namespace Perpetuum.Tests.Fakes.Data +{ + /// + /// Implements IDbConnection and deliberately not DbConnection: DbQuery.ExecuteHelper enlists + /// the ambient transaction only for DbConnection, so staying off that type keeps the fake out + /// of any transaction manager while still letting it observe Transaction.Current. + /// + public sealed class FakeDbConnection(FakeDb owner) : IDbConnection + { + public FakeDb Owner { get; } = owner; + + public Transaction? AmbientTransactionAtOpen { get; private set; } + public bool WasOpened { get; private set; } + + public string ConnectionString { get; set; } = "fake"; + public int ConnectionTimeout => 0; + public string Database => "fake"; + public ConnectionState State { get; private set; } = ConnectionState.Closed; + + public void Open() + { + AmbientTransactionAtOpen = Transaction.Current; + WasOpened = true; + State = ConnectionState.Open; + } + + public void Close() => State = ConnectionState.Closed; + public IDbCommand CreateCommand() => new FakeDbCommand(this); + public IDbTransaction BeginTransaction() => throw new NotSupportedException(); + public IDbTransaction BeginTransaction(System.Data.IsolationLevel il) => throw new NotSupportedException(); + public void ChangeDatabase(string databaseName) => throw new NotSupportedException(); + public void Dispose() => Close(); + } +} diff --git a/src/Perpetuum.Tests/Fakes/Data/FakeResultSet.cs b/src/Perpetuum.Tests/Fakes/Data/FakeResultSet.cs new file mode 100644 index 00000000..a45c7177 --- /dev/null +++ b/src/Perpetuum.Tests/Fakes/Data/FakeResultSet.cs @@ -0,0 +1,28 @@ +namespace Perpetuum.Tests.Fakes.Data +{ + /// One query result: column names plus rows, positionally aligned. + public sealed class FakeResultSet + { + public required IReadOnlyList ColumnNames { get; init; } + public required IReadOnlyList Rows { get; init; } + + public static FakeResultSet Empty(params string[] columnNames) + { + return new FakeResultSet { ColumnNames = columnNames, Rows = [] }; + } + + public static FakeResultSet FromRows(string[] columnNames, params object?[][] rows) + { + foreach (object?[] row in rows) + { + if (row.Length != columnNames.Length) + { + throw new ArgumentException( + $"Row has {row.Length} values but {columnNames.Length} columns were declared."); + } + } + + return new FakeResultSet { ColumnNames = columnNames, Rows = rows }; + } + } +} From c84ff058bb1dc91d7a28767020fdffd6a6554632 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 20:04:23 -0300 Subject: [PATCH 10/24] test: cover DbQuery parameter, timeout and result handling Adds DbQueryTests, exercising ExecuteHelper's command type inference, parameter mapping, null handling, timeout propagation, and result reading against FakeDb. Fixes FakeDataReader.Current to no longer index Rows[-1]: DbEnumerator (behind DataReaderExtensions.ToEnumerable, which DbQuery.Execute() and ExecuteSingleRow() rely on) calls GetFieldType for every column once, before the first Read(), to build its schema info, while the reader is still unpositioned. All 6 tests that go through Execute() reproduced this before the fix; the 2 that go through ExecuteScalar()/ ExecuteNonQuery() passed unaffected. Co-Authored-By: Claude Opus 5 --- .../Fakes/Data/FakeDataReader.cs | 9 +- src/Perpetuum.Tests/Unit/DbQueryTests.cs | 127 ++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 src/Perpetuum.Tests/Unit/DbQueryTests.cs diff --git a/src/Perpetuum.Tests/Fakes/Data/FakeDataReader.cs b/src/Perpetuum.Tests/Fakes/Data/FakeDataReader.cs index 755a02b9..4db7ebde 100644 --- a/src/Perpetuum.Tests/Fakes/Data/FakeDataReader.cs +++ b/src/Perpetuum.Tests/Fakes/Data/FakeDataReader.cs @@ -7,7 +7,14 @@ public sealed class FakeDataReader(FakeResultSet resultSet) : IDataReader private readonly FakeResultSet _resultSet = resultSet; private int _index = -1; - private object?[] Current => _resultSet.Rows[_index]; + // System.Data.Common.DbEnumerator (behind DataReaderExtensions.ToEnumerable, which + // DbQuery.Execute()/ExecuteSingleRow() use) calls GetFieldType for every column once, + // before the first Read(), to build its schema info — while _index is still -1. Treat + // that unpositioned state as an all-null row instead of indexing Rows[-1]; a real reader + // has no per-row values to report at that point either. + private object?[] Current => _index < 0 + ? new object?[_resultSet.ColumnNames.Count] + : _resultSet.Rows[_index]; public bool Read() { diff --git a/src/Perpetuum.Tests/Unit/DbQueryTests.cs b/src/Perpetuum.Tests/Unit/DbQueryTests.cs new file mode 100644 index 00000000..4c724cdc --- /dev/null +++ b/src/Perpetuum.Tests/Unit/DbQueryTests.cs @@ -0,0 +1,127 @@ +using System.Data; +using Perpetuum.Data; +using Perpetuum.Tests.Fakes.Data; +using Perpetuum.Tests.Infrastructure; +using Xunit; + +namespace Perpetuum.Tests.Unit +{ + [Collection(PerpetuumStaticsCollection.Name)] + public class DbQueryTests + { + private readonly FakeDb _db; + + public DbQueryTests(PerpetuumStaticsFixture fixture) + { + _ = fixture; + _db = FakeDb.Install(); + } + + [Fact] + public void Execute_returns_one_record_per_row() + { + _db.When("select definition,fee,payout from insuranceprices", + FakeResultSet.FromRows( + ["definition", "fee", "payout"], + [1234, 100.5d, 900.0d], + [5678, 200.5d, 1800.0d])); + + List records = Db.Query() + .CommandText("select definition,fee,payout from insuranceprices") + .Execute(); + + Assert.Equal(2, records.Count); + Assert.Equal(1234, records[0].GetValue(0)); + Assert.Equal(900.0d, records[0].GetValue(2)); + Assert.Equal(5678, records[1].GetValue(0)); + } + + [Fact] + public void Execute_on_no_rows_returns_an_empty_list() + { + _db.When("select 1 from nothing", FakeResultSet.Empty("x")); + + List records = Db.Query().CommandText("select 1 from nothing").Execute(); + + Assert.Empty(records); + } + + [Fact] + public void Parameters_are_passed_through_by_name() + { + _db.When("select * from characters where id = @id", FakeResultSet.Empty("id")); + + _ = Db.Query() + .CommandText("select * from characters where id = @id") + .SetParameter("@id", 42) + .Execute(); + + RecordedCommand? recorded = _db.LastCommandMatching("from characters"); + Assert.NotNull(recorded); + Assert.Equal(42, recorded!.Parameters["@id"]); + } + + [Fact] + public void A_null_parameter_value_is_sent_as_DBNull() + { + _db.When("select * from characters where nick = @nick", FakeResultSet.Empty("nick")); + + _ = Db.Query() + .CommandText("select * from characters where nick = @nick") + .SetParameter("@nick", null) + .Execute(); + + RecordedCommand? recorded = _db.LastCommandMatching("from characters"); + Assert.NotNull(recorded); + Assert.Null(recorded!.Parameters["@nick"]); + } + + [Fact] + public void Timeout_is_propagated_to_the_command() + { + _db.WhenNonQuery("exec usp_RecalculateInsurancePrices", 1); + + _ = Db.Query() + .CommandText("exec usp_RecalculateInsurancePrices") + .Timeout(120) + .ExecuteNonQuery(); + + RecordedCommand? recorded = _db.LastCommandMatching("usp_RecalculateInsurancePrices"); + Assert.NotNull(recorded); + Assert.Equal(120, recorded!.CommandTimeout); + } + + [Fact] + public void The_default_timeout_is_thirty_seconds() + { + _db.When("select 1", FakeResultSet.Empty("x")); + + _ = Db.Query().CommandText("select 1").Execute(); + + RecordedCommand? recorded = _db.LastCommandMatching("select 1"); + Assert.NotNull(recorded); + Assert.Equal(30, recorded!.CommandTimeout); + } + + [Fact] + public void ExecuteScalar_returns_the_first_column_of_the_first_row() + { + _db.When("select count(*) from characters", + FakeResultSet.FromRows(["count"], [7])); + + int count = Db.Query().CommandText("select count(*) from characters").ExecuteScalar(); + + Assert.Equal(7, count); + } + + [Fact] + public void Db_Query_with_command_text_is_the_same_as_setting_it_afterwards() + { + _db.When("select 1", FakeResultSet.Empty("x")); + + _ = Db.Query("select 1").Execute(); + + Assert.Equal("select 1", _db.Commands[^1].CommandText); + } + } +} From f19f24532f763ca61adc0a8c6ef131679b8f1fb1 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Thu, 13 Aug 2026 20:18:51 -0300 Subject: [PATCH 11/24] test: cover DbQuery command type inference and ExecuteSingleRow Task 6 review found two of the four behaviours the task set out to pin were missing: command type inference and ExecuteSingleRow's row reduction. RecordedCommand did not even carry CommandType, so no test could reach DbQuery.ExecuteHelper's `_commandText.Contains(' ')` heuristic (DbQuery.cs:64) that decides Text vs StoredProcedure. Adds CommandType to RecordedCommand and FakeDb.Record, and adds The_command_type_is_inferred_from_whether_the_text_contains_a_space (both branches of the heuristic) and ExecuteSingleRow_returns_the_first_row_and_null_when_there_are_none. Both passed on first run; the heuristic behaves as documented. Co-Authored-By: Claude Opus 5 --- src/Perpetuum.Tests/Fakes/Data/FakeDb.cs | 2 ++ src/Perpetuum.Tests/Unit/DbQueryTests.cs | 36 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs b/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs index 2f77042b..6882a798 100644 --- a/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs +++ b/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs @@ -7,6 +7,7 @@ namespace Perpetuum.Tests.Fakes.Data public sealed record RecordedCommand { public required string CommandText { get; init; } + public required CommandType CommandType { get; init; } public required IReadOnlyDictionary Parameters { get; init; } public required int CommandTimeout { get; init; } public required bool HadAmbientTransaction { get; init; } @@ -71,6 +72,7 @@ internal FakeResultSet Record(FakeDbCommand command) _commands.Add(new RecordedCommand { CommandText = command.CommandText, + CommandType = command.CommandType, Parameters = parameters, CommandTimeout = command.CommandTimeout, HadAmbientTransaction = ambient != null, diff --git a/src/Perpetuum.Tests/Unit/DbQueryTests.cs b/src/Perpetuum.Tests/Unit/DbQueryTests.cs index 4c724cdc..3fa744ea 100644 --- a/src/Perpetuum.Tests/Unit/DbQueryTests.cs +++ b/src/Perpetuum.Tests/Unit/DbQueryTests.cs @@ -123,5 +123,41 @@ public void Db_Query_with_command_text_is_the_same_as_setting_it_afterwards() Assert.Equal("select 1", _db.Commands[^1].CommandText); } + + [Theory] + [InlineData("usp_RecalculateInsurancePrices", CommandType.StoredProcedure)] + [InlineData("select 1 from characters", CommandType.Text)] + public void The_command_type_is_inferred_from_whether_the_text_contains_a_space( + string commandText, + CommandType expected) + { + // DbQuery.ExecuteHelper decides this with _commandText.Contains(' '), which is what + // makes a spaceless command run as a stored procedure. Breaking that heuristic changes + // how every parameterless proc call is dispatched, silently. + _db.When(commandText, FakeResultSet.Empty("x")); + + _ = Db.Query().CommandText(commandText).Execute(); + + RecordedCommand? recorded = _db.LastCommandMatching(commandText); + Assert.NotNull(recorded); + Assert.Equal(expected, recorded!.CommandType); + } + + [Fact] + public void ExecuteSingleRow_returns_the_first_row_and_null_when_there_are_none() + { + _db.When("select top 2 id from characters", + FakeResultSet.FromRows(["id"], [1], [2])); + _db.When("select id from nothing", FakeResultSet.Empty("id")); + + IDataRecord? first = Db.Query() + .CommandText("select top 2 id from characters") + .ExecuteSingleRow(); + + Assert.NotNull(first); + Assert.Equal(1, first!.GetValue(0)); + + Assert.Null(Db.Query().CommandText("select id from nothing").ExecuteSingleRow()); + } } } From b6ac38309e17067f2f75664c8ae35a1d2a8ba8d4 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 06:59:25 -0300 Subject: [PATCH 12/24] test: add integration project with game root discovery and skip behaviour Co-Authored-By: Claude Opus 5 --- PerpetuumServer2.sln | 9 ++- .../AssemblyInfo.cs | 3 + .../Infrastructure/DatabaseFixture.cs | 35 +++++++++ .../EnvironmentDiscoveryTests.cs | 34 +++++++++ .../Infrastructure/GameRootEnvironment.cs | 72 +++++++++++++++++++ .../RequiresGameRootFactAttribute.cs | 20 ++++++ .../Perpetuum.Tests.Integration.csproj | 21 ++++++ 7 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 src/Perpetuum.Tests.Integration/AssemblyInfo.cs create mode 100644 src/Perpetuum.Tests.Integration/Infrastructure/DatabaseFixture.cs create mode 100644 src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs create mode 100644 src/Perpetuum.Tests.Integration/Infrastructure/GameRootEnvironment.cs create mode 100644 src/Perpetuum.Tests.Integration/Infrastructure/RequiresGameRootFactAttribute.cs create mode 100644 src/Perpetuum.Tests.Integration/Perpetuum.Tests.Integration.csproj diff --git a/PerpetuumServer2.sln b/PerpetuumServer2.sln index 0d3e9974..9a70b043 100644 --- a/PerpetuumServer2.sln +++ b/PerpetuumServer2.sln @@ -23,7 +23,9 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Perpetuum.ServerService2", EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Perpetuum.AdminTool", "src\Perpetuum.AdminTool\Perpetuum.AdminTool.csproj", "{A7D1E3C5-9F4B-42E8-8A6C-B5D7F1E9C2A0}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Perpetuum.Tests", "src\Perpetuum.Tests\Perpetuum.Tests.csproj", "{C8C45427-6E5C-496C-9446-0817EADC05DD}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Perpetuum.Tests", "src\Perpetuum.Tests\Perpetuum.Tests.csproj", "{C8C45427-6E5C-496C-9446-0817EADC05DD}" +EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Perpetuum.Tests.Integration", "src\Perpetuum.Tests.Integration\Perpetuum.Tests.Integration.csproj", "{ABE8E004-66D6-4D40-AEBF-3CFBD041E29B}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution @@ -71,6 +73,10 @@ Global {C8C45427-6E5C-496C-9446-0817EADC05DD}.Debug|x64.Build.0 = Debug|x64 {C8C45427-6E5C-496C-9446-0817EADC05DD}.Release|x64.ActiveCfg = Release|x64 {C8C45427-6E5C-496C-9446-0817EADC05DD}.Release|x64.Build.0 = Release|x64 + {ABE8E004-66D6-4D40-AEBF-3CFBD041E29B}.Debug|x64.ActiveCfg = Debug|x64 + {ABE8E004-66D6-4D40-AEBF-3CFBD041E29B}.Debug|x64.Build.0 = Debug|x64 + {ABE8E004-66D6-4D40-AEBF-3CFBD041E29B}.Release|x64.ActiveCfg = Release|x64 + {ABE8E004-66D6-4D40-AEBF-3CFBD041E29B}.Release|x64.Build.0 = Release|x64 EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -86,6 +92,7 @@ Global {A2FCE930-E013-4731-B354-F7DA00322D38} = {0BC991A1-133C-49ED-A141-80E2A906898B} {A7D1E3C5-9F4B-42E8-8A6C-B5D7F1E9C2A0} = {0BC991A1-133C-49ED-A141-80E2A906898B} {C8C45427-6E5C-496C-9446-0817EADC05DD} = {0BC991A1-133C-49ED-A141-80E2A906898B} + {ABE8E004-66D6-4D40-AEBF-3CFBD041E29B} = {0BC991A1-133C-49ED-A141-80E2A906898B} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {91874B60-30B0-4B48-9155-370E40BC70E6} diff --git a/src/Perpetuum.Tests.Integration/AssemblyInfo.cs b/src/Perpetuum.Tests.Integration/AssemblyInfo.cs new file mode 100644 index 00000000..37fefc24 --- /dev/null +++ b/src/Perpetuum.Tests.Integration/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.Versioning; + +[assembly: SupportedOSPlatform("windows")] diff --git a/src/Perpetuum.Tests.Integration/Infrastructure/DatabaseFixture.cs b/src/Perpetuum.Tests.Integration/Infrastructure/DatabaseFixture.cs new file mode 100644 index 00000000..2271e4f1 --- /dev/null +++ b/src/Perpetuum.Tests.Integration/Infrastructure/DatabaseFixture.cs @@ -0,0 +1,35 @@ +using Microsoft.Data.SqlClient; + +namespace Perpetuum.Tests.Integration.Infrastructure +{ + /// + /// Opens connections to the developer's real perpetuumsa database. Read-only by default: + /// nothing in this project writes unless PERPETUUM_TESTDB_ALLOW_WRITE=1, and stages 0-4 + /// contain no write test at all. + /// + public sealed class DatabaseFixture + { + public GameRootEnvironment? Environment { get; } + public string? UnavailableReason { get; } + + public DatabaseFixture() + { + _ = GameRootEnvironment.TryLoad(out GameRootEnvironment? env, out string? reason); + Environment = env; + UnavailableReason = reason; + } + + public SqlConnection OpenConnection() + { + if (Environment is null) + { + throw new InvalidOperationException( + $"Database unavailable: {UnavailableReason}. Tests must use [RequiresGameRootFact]."); + } + + SqlConnection connection = new(Environment.ConnectionString); + connection.Open(); + return connection; + } + } +} diff --git a/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs b/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs new file mode 100644 index 00000000..f103921b --- /dev/null +++ b/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs @@ -0,0 +1,34 @@ +using Xunit; + +namespace Perpetuum.Tests.Integration.Infrastructure +{ + public class EnvironmentDiscoveryTests + { + [RequiresGameRootFact] + public void The_connection_string_comes_from_perpetuum_ini() + { + Assert.True(GameRootEnvironment.TryLoad(out GameRootEnvironment? env, out string? reason), reason); + Assert.NotNull(env); + Assert.False(string.IsNullOrWhiteSpace(env!.ConnectionString)); + } + + [RequiresGameRootFact] + public void The_database_accepts_a_connection() + { + DatabaseFixture fixture = new(); + using Microsoft.Data.SqlClient.SqlConnection connection = fixture.OpenConnection(); + + Assert.Equal(System.Data.ConnectionState.Open, connection.State); + } + + [Fact] + public void Writes_are_disabled_unless_explicitly_allowed() + { + // This test runs everywhere, including CI, and documents the default. + if (Environment.GetEnvironmentVariable(GameRootEnvironment.AllowWriteVariable) is null) + { + Assert.False(GameRootEnvironment.WritesAllowed); + } + } + } +} diff --git a/src/Perpetuum.Tests.Integration/Infrastructure/GameRootEnvironment.cs b/src/Perpetuum.Tests.Integration/Infrastructure/GameRootEnvironment.cs new file mode 100644 index 00000000..3b99b294 --- /dev/null +++ b/src/Perpetuum.Tests.Integration/Infrastructure/GameRootEnvironment.cs @@ -0,0 +1,72 @@ +using Newtonsoft.Json; + +namespace Perpetuum.Tests.Integration.Infrastructure +{ + /// + /// Reads the same perpetuum.ini the server reads, from the directory named by + /// PERPETUUM_GAMEROOT. The file is JSON deserialized into GlobalConfiguration by + /// PerpetuumBootstrapper; this does the same, so the connection string can never drift + /// from the one the server uses. + /// + public sealed class GameRootEnvironment + { + public const string GameRootVariable = "PERPETUUM_GAMEROOT"; + public const string AllowWriteVariable = "PERPETUUM_TESTDB_ALLOW_WRITE"; + + public required string GameRoot { get; init; } + public required string ConnectionString { get; init; } + + public static bool WritesAllowed + => Environment.GetEnvironmentVariable(AllowWriteVariable) == "1"; + + public static bool TryLoad(out GameRootEnvironment? environment, out string? reason) + { + environment = null; + + string? gameRoot = Environment.GetEnvironmentVariable(GameRootVariable); + if (string.IsNullOrWhiteSpace(gameRoot)) + { + reason = $"{GameRootVariable} is not set."; + return false; + } + + if (!Directory.Exists(gameRoot)) + { + reason = $"{GameRootVariable} points at a directory that does not exist: {gameRoot}"; + return false; + } + + string iniPath = Path.Combine(gameRoot, "perpetuum.ini"); + if (!File.Exists(iniPath)) + { + reason = $"perpetuum.ini not found under {gameRoot}"; + return false; + } + + GlobalConfiguration? configuration; + try + { + configuration = JsonConvert.DeserializeObject(File.ReadAllText(iniPath)); + } + catch (Exception ex) + { + reason = $"perpetuum.ini could not be parsed: {ex.Message}"; + return false; + } + + if (string.IsNullOrWhiteSpace(configuration?.ConnectionString)) + { + reason = "perpetuum.ini carries no ConnectionString."; + return false; + } + + environment = new GameRootEnvironment + { + GameRoot = gameRoot, + ConnectionString = configuration.ConnectionString, + }; + reason = null; + return true; + } + } +} diff --git a/src/Perpetuum.Tests.Integration/Infrastructure/RequiresGameRootFactAttribute.cs b/src/Perpetuum.Tests.Integration/Infrastructure/RequiresGameRootFactAttribute.cs new file mode 100644 index 00000000..e557f533 --- /dev/null +++ b/src/Perpetuum.Tests.Integration/Infrastructure/RequiresGameRootFactAttribute.cs @@ -0,0 +1,20 @@ +using Xunit; + +namespace Perpetuum.Tests.Integration.Infrastructure +{ + /// + /// A test marked with this attribute is skipped, not failed, when the local environment is + /// absent. The check is per test rather than per collection so a missing environment reports + /// once per affected test instead of failing a fixture during construction. + /// + public sealed class RequiresGameRootFactAttribute : FactAttribute + { + public RequiresGameRootFactAttribute() + { + if (!GameRootEnvironment.TryLoad(out _, out string? reason)) + { + Skip = $"Local game environment unavailable: {reason}"; + } + } + } +} diff --git a/src/Perpetuum.Tests.Integration/Perpetuum.Tests.Integration.csproj b/src/Perpetuum.Tests.Integration/Perpetuum.Tests.Integration.csproj new file mode 100644 index 00000000..b7496cdb --- /dev/null +++ b/src/Perpetuum.Tests.Integration/Perpetuum.Tests.Integration.csproj @@ -0,0 +1,21 @@ + + + + net8.0 + enable + annotations + x64 + false + + + + + + + + + + + + + From 43712cbdaccf049a70d3bb6e8a7e04a2d1a5afad Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 07:30:19 -0300 Subject: [PATCH 13/24] test: assert every documented stored procedure and function exists --- .../Infrastructure/DatabaseCollection.cs | 16 +++ .../EnvironmentDiscoveryTests.cs | 1 + .../Schema/StoredProcedureConformanceTests.cs | 108 ++++++++++++++++++ 3 files changed, 125 insertions(+) create mode 100644 src/Perpetuum.Tests.Integration/Infrastructure/DatabaseCollection.cs create mode 100644 src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs diff --git a/src/Perpetuum.Tests.Integration/Infrastructure/DatabaseCollection.cs b/src/Perpetuum.Tests.Integration/Infrastructure/DatabaseCollection.cs new file mode 100644 index 00000000..7fd8f66a --- /dev/null +++ b/src/Perpetuum.Tests.Integration/Infrastructure/DatabaseCollection.cs @@ -0,0 +1,16 @@ +using Xunit; + +namespace Perpetuum.Tests.Integration.Infrastructure +{ + /// + /// Every test class that opens a connection to the operator's real perpetuumsa carries + /// [Collection(DatabaseCollection.Name)]. xUnit runs the classes in one collection serially, + /// which holds concurrent connections to a developer's own database at one and keeps failures + /// deterministic. It also means the write opt-in a later stage may use cannot race with a read. + /// + [CollectionDefinition(Name)] + public class DatabaseCollection + { + public const string Name = "Perpetuum database"; + } +} diff --git a/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs b/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs index f103921b..d8f67efe 100644 --- a/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs +++ b/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs @@ -2,6 +2,7 @@ namespace Perpetuum.Tests.Integration.Infrastructure { + [Collection(DatabaseCollection.Name)] public class EnvironmentDiscoveryTests { [RequiresGameRootFact] diff --git a/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs b/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs new file mode 100644 index 00000000..a82ffaad --- /dev/null +++ b/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs @@ -0,0 +1,108 @@ +using System.Data; +using Microsoft.Data.SqlClient; +using Perpetuum.Tests.Integration.Infrastructure; +using Xunit; + +namespace Perpetuum.Tests.Integration.Schema +{ + /// + /// docs/db_structure is described by CLAUDE.md as the authoritative source of truth for the + /// database. These tests make that claim checkable: every documented procedure and function + /// must exist in the real database. + /// + [Collection(DatabaseCollection.Name)] + public class StoredProcedureConformanceTests + { + private static string RepositoryRoot + { + get + { + DirectoryInfo? dir = new(AppContext.BaseDirectory); + while (dir != null && !File.Exists(Path.Combine(dir.FullName, "PerpetuumServer2.sln"))) + { + dir = dir.Parent; + } + + return dir?.FullName + ?? throw new InvalidOperationException("PerpetuumServer2.sln not found above the test output directory."); + } + } + + /// + /// docs/db_structure/stored_procedures files are named "<schema>.<ObjectName>.StoredProcedure.sql" + /// (e.g. "dbo.CreateFolderContainer.StoredProcedure.sql", "opp.artifactRefresh.StoredProcedure.sql"), with + /// exactly one exception in the repository, "dbo.usp_RecalculateInsurancePrices.sql", which omits the + /// ".StoredProcedure" segment. docs/db_structure/functions files carry no schema prefix and no type + /// suffix at all (e.g. "CFName.sql"). Both shapes reduce to the same rule once the ".sql" extension is + /// stripped: split on '.', and the object name is the second segment when a schema prefix is present, or + /// the only segment when it is not. sys.objects.name never carries the schema itself, so a schema + /// segment, when present, is discarded rather than reattached. + /// + private static string ObjectNameFromDocumentedFileName(string fileNameWithoutExtension) + { + string[] parts = fileNameWithoutExtension.Split('.'); + return parts.Length > 1 ? parts[1] : parts[0]; + } + + private static IReadOnlyList DocumentedNames(string subdirectory) + { + string path = Path.Combine(RepositoryRoot, "docs", "db_structure", subdirectory); + return [.. Directory.EnumerateFiles(path, "*.sql") + .Select(Path.GetFileNameWithoutExtension) + .Where(n => !string.IsNullOrWhiteSpace(n)) + .Select(n => ObjectNameFromDocumentedFileName(n!)) + .OrderBy(n => n, StringComparer.OrdinalIgnoreCase)]; + } + + private static HashSet ObjectNamesOfType(SqlConnection connection, params string[] typeCodes) + { + HashSet names = new(StringComparer.OrdinalIgnoreCase); + + using SqlCommand command = connection.CreateCommand(); + command.CommandText = + "select name from sys.objects where type in (" + + string.Join(",", typeCodes.Select((_, i) => $"@t{i}")) + ")"; + + for (int i = 0; i < typeCodes.Length; i++) + { + _ = command.Parameters.AddWithValue($"@t{i}", typeCodes[i]); + } + + using IDataReader reader = command.ExecuteReader(); + while (reader.Read()) + { + _ = names.Add(reader.GetString(0)); + } + + return names; + } + + [RequiresGameRootFact] + public void Every_documented_stored_procedure_exists() + { + DatabaseFixture fixture = new(); + using SqlConnection connection = fixture.OpenConnection(); + + HashSet actual = ObjectNamesOfType(connection, "P", "PC"); + List missing = [.. DocumentedNames("stored_procedures").Where(n => !actual.Contains(n))]; + + Assert.True( + missing.Count == 0, + $"Documented under docs/db_structure/stored_procedures but absent from the database: {string.Join(", ", missing)}"); + } + + [RequiresGameRootFact] + public void Every_documented_function_exists() + { + DatabaseFixture fixture = new(); + using SqlConnection connection = fixture.OpenConnection(); + + HashSet actual = ObjectNamesOfType(connection, "FN", "IF", "TF", "FS", "FT"); + List missing = [.. DocumentedNames("functions").Where(n => !actual.Contains(n))]; + + Assert.True( + missing.Count == 0, + $"Documented under docs/db_structure/functions but absent from the database: {string.Join(", ", missing)}"); + } + } +} From 720f718c892e8e63d636b9d3e4a989f9bd86559a Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 07:53:21 -0300 Subject: [PATCH 14/24] test: compare documented stored procedures by schema-qualified name --- .../Schema/StoredProcedureConformanceTests.cs | 63 +++++++++++++------ 1 file changed, 43 insertions(+), 20 deletions(-) diff --git a/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs b/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs index a82ffaad..ce82edc3 100644 --- a/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs +++ b/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs @@ -29,38 +29,52 @@ private static string RepositoryRoot } /// - /// docs/db_structure/stored_procedures files are named "<schema>.<ObjectName>.StoredProcedure.sql" - /// (e.g. "dbo.CreateFolderContainer.StoredProcedure.sql", "opp.artifactRefresh.StoredProcedure.sql"), with - /// exactly one exception in the repository, "dbo.usp_RecalculateInsurancePrices.sql", which omits the - /// ".StoredProcedure" segment. docs/db_structure/functions files carry no schema prefix and no type - /// suffix at all (e.g. "CFName.sql"). Both shapes reduce to the same rule once the ".sql" extension is - /// stripped: split on '.', and the object name is the second segment when a schema prefix is present, or - /// the only segment when it is not. sys.objects.name never carries the schema itself, so a schema - /// segment, when present, is discarded rather than reattached. + /// The two directories name their files by different conventions, both real and both + /// measured: stored_procedures/ uses "schema.Name.StoredProcedure.sql" — with one outlier, + /// dbo.usp_RecalculateInsurancePrices.sql, that omits the .StoredProcedure segment — while + /// functions/ uses a bare "Name.sql". Taking the filename as the object name would compare + /// "dbo.X.StoredProcedure" against "X" and report almost every documented procedure as + /// missing, which looks like a large finding about the repository and is not one. /// - private static string ObjectNameFromDocumentedFileName(string fileNameWithoutExtension) + /// Procedures keep their schema; functions cannot. Two documented procedures differ only by + /// schema — dbo.extensionSubscriptionStart and opp.extensionSubscriptionStart — so comparing + /// bare names would let one of them disappear from the database while this test stayed green. + /// Function filenames carry no schema at all, and at least one real function lives outside + /// dbo (opp.ToolTestAccount_GetDefinitionID, which is itself undocumented), so assuming dbo + /// there would produce a false failure. The asymmetry is forced by the data, and the + /// remaining gap on the function side is stated rather than left to be discovered. + private static string ProcedureNameFromDocumentedFileName(string fileNameWithoutExtension) { - string[] parts = fileNameWithoutExtension.Split('.'); - return parts.Length > 1 ? parts[1] : parts[0]; + return fileNameWithoutExtension.EndsWith(".StoredProcedure", StringComparison.OrdinalIgnoreCase) + ? fileNameWithoutExtension[..^".StoredProcedure".Length] + : fileNameWithoutExtension; } - private static IReadOnlyList DocumentedNames(string subdirectory) + private static IReadOnlyList DocumentedNames(string subdirectory, Func derive) { string path = Path.Combine(RepositoryRoot, "docs", "db_structure", subdirectory); return [.. Directory.EnumerateFiles(path, "*.sql") .Select(Path.GetFileNameWithoutExtension) .Where(n => !string.IsNullOrWhiteSpace(n)) - .Select(n => ObjectNameFromDocumentedFileName(n!)) + .Select(n => derive(n!)) .OrderBy(n => n, StringComparer.OrdinalIgnoreCase)]; } - private static HashSet ObjectNamesOfType(SqlConnection connection, params string[] typeCodes) + private static HashSet RoutineNames( + SqlConnection connection, + bool qualifyWithSchema, + params string[] typeCodes) { HashSet names = new(StringComparer.OrdinalIgnoreCase); using SqlCommand command = connection.CreateCommand(); + + // The schema and object names are selected as separate columns and joined in C#. + // Concatenating them in T-SQL fails on this database with "Cannot resolve collation + // conflict between Latin1_General_CI_AS_KS_WS and SQL_Latin1_General_CP1_CI_AS". command.CommandText = - "select name from sys.objects where type in (" + + "select s.name, o.name from sys.objects o " + + "join sys.schemas s on s.schema_id = o.schema_id where o.type in (" + string.Join(",", typeCodes.Select((_, i) => $"@t{i}")) + ")"; for (int i = 0; i < typeCodes.Length; i++) @@ -71,7 +85,9 @@ private static HashSet ObjectNamesOfType(SqlConnection connection, param using IDataReader reader = command.ExecuteReader(); while (reader.Read()) { - _ = names.Add(reader.GetString(0)); + string schema = reader.GetString(0); + string name = reader.GetString(1); + _ = names.Add(qualifyWithSchema ? $"{schema}.{name}" : name); } return names; @@ -83,8 +99,12 @@ public void Every_documented_stored_procedure_exists() DatabaseFixture fixture = new(); using SqlConnection connection = fixture.OpenConnection(); - HashSet actual = ObjectNamesOfType(connection, "P", "PC"); - List missing = [.. DocumentedNames("stored_procedures").Where(n => !actual.Contains(n))]; + HashSet actual = RoutineNames(connection, qualifyWithSchema: true, "P", "PC"); + List missing = + [ + .. DocumentedNames("stored_procedures", ProcedureNameFromDocumentedFileName) + .Where(n => !actual.Contains(n)) + ]; Assert.True( missing.Count == 0, @@ -97,8 +117,11 @@ public void Every_documented_function_exists() DatabaseFixture fixture = new(); using SqlConnection connection = fixture.OpenConnection(); - HashSet actual = ObjectNamesOfType(connection, "FN", "IF", "TF", "FS", "FT"); - List missing = [.. DocumentedNames("functions").Where(n => !actual.Contains(n))]; + HashSet actual = RoutineNames(connection, qualifyWithSchema: false, "FN", "IF", "TF", "FS", "FT"); + List missing = + [ + .. DocumentedNames("functions", n => n).Where(n => !actual.Contains(n)) + ]; Assert.True( missing.Count == 0, From 4006d126eaadf18fd3b5713f45594faf253e8324 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 08:11:56 -0300 Subject: [PATCH 15/24] test: execute the insurance price query against the real schema --- .../Data/InsuranceQueryTests.cs | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 src/Perpetuum.Tests.Integration/Data/InsuranceQueryTests.cs diff --git a/src/Perpetuum.Tests.Integration/Data/InsuranceQueryTests.cs b/src/Perpetuum.Tests.Integration/Data/InsuranceQueryTests.cs new file mode 100644 index 00000000..29954b50 --- /dev/null +++ b/src/Perpetuum.Tests.Integration/Data/InsuranceQueryTests.cs @@ -0,0 +1,68 @@ +using System.Data; +using Microsoft.Data.SqlClient; +using Perpetuum.Tests.Integration.Infrastructure; +using Xunit; + +namespace Perpetuum.Tests.Integration.Data +{ + /// + /// The unit tests stub this exact query text against a fake connection. Running it for real + /// is what keeps the fake honest: if a column is renamed or the table is dropped, the unit + /// tests keep passing against their stub and this test is what fails. + /// + [Collection(DatabaseCollection.Name)] + public class InsuranceQueryTests + { + private const string InsurancePricesQuery = "select definition,fee,payout from insuranceprices"; + + [RequiresGameRootFact] + public void The_insurance_prices_query_runs_and_returns_the_expected_columns() + { + DatabaseFixture fixture = new(); + using SqlConnection connection = fixture.OpenConnection(); + + using SqlCommand command = connection.CreateCommand(); + command.CommandText = InsurancePricesQuery; + + using SqlDataReader reader = command.ExecuteReader(); + + Assert.Equal(3, reader.FieldCount); + Assert.Equal("definition", reader.GetName(0), ignoreCase: true); + Assert.Equal("fee", reader.GetName(1), ignoreCase: true); + Assert.Equal("payout", reader.GetName(2), ignoreCase: true); + } + + [RequiresGameRootFact] + public void The_insurance_prices_columns_have_the_types_the_code_reads_them_as() + { + // InsuranceHelper.LoadInsurancePrices reads column 0 as int and columns 1 and 2 as + // double. A type change in the database would break that silently at runtime. + DatabaseFixture fixture = new(); + using SqlConnection connection = fixture.OpenConnection(); + + using SqlCommand command = connection.CreateCommand(); + command.CommandText = InsurancePricesQuery; + + using SqlDataReader reader = command.ExecuteReader(CommandBehavior.SchemaOnly); + DataTable? schema = reader.GetSchemaTable(); + + Assert.NotNull(schema); + Assert.Equal(typeof(int), schema!.Rows[0]["DataType"]); + Assert.Equal(typeof(double), schema.Rows[1]["DataType"]); + Assert.Equal(typeof(double), schema.Rows[2]["DataType"]); + } + + [RequiresGameRootFact] + public void The_insurance_price_recalculation_procedure_exists_and_is_callable() + { + DatabaseFixture fixture = new(); + using SqlConnection connection = fixture.OpenConnection(); + + using SqlCommand command = connection.CreateCommand(); + command.CommandText = + "select count(*) from sys.objects where type in ('P','PC') and name = 'usp_RecalculateInsurancePrices'"; + + Assert.Equal(1, (int)command.ExecuteScalar()); + } + } +} From 513d78641ee0998ae340ea01a050e778e36a692e Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 08:27:41 -0300 Subject: [PATCH 16/24] test: document the insurance anchor's rationale and schema-qualify the procedure check Review fixes for Task 9: - Cross-reference comment above InsurancePricesQuery (InsuranceQueryTests.cs) and above the matching stub in DbQueryTests.cs, stating the three-copy literal is only kept in sync by hand. - The_insurance_price_recalculation_procedure_exists_and_is_callable renamed to The_insurance_price_recalculation_procedure_exists, with both reasons it is not redundant with Task 8 stated in comments (it anchors the unit tier's WhenNonQuery stub; Task 8's check is driven from docs/db_structure/ and would not catch the procedure and its doc file being deleted together). - The sys.objects lookup is now joined to sys.schemas and filtered on s.name = 'dbo', matching the schema-qualification already used by Task 8's StoredProcedureConformanceTests, with a comment explaining why a bare name match is not safe on this database. --- .../Data/InsuranceQueryTests.cs | 25 +++++++++++++++++-- src/Perpetuum.Tests/Unit/DbQueryTests.cs | 3 +++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/Perpetuum.Tests.Integration/Data/InsuranceQueryTests.cs b/src/Perpetuum.Tests.Integration/Data/InsuranceQueryTests.cs index 29954b50..4ef7aa66 100644 --- a/src/Perpetuum.Tests.Integration/Data/InsuranceQueryTests.cs +++ b/src/Perpetuum.Tests.Integration/Data/InsuranceQueryTests.cs @@ -13,6 +13,11 @@ namespace Perpetuum.Tests.Integration.Data [Collection(DatabaseCollection.Name)] public class InsuranceQueryTests { + // Kept in sync by hand with two other copies of this literal: the stub in + // Perpetuum.Tests/Unit/DbQueryTests.cs and the production query in + // InsuranceHelper.LoadInsurancePrices. Nothing links the three at compile time — they are in + // two assemblies plus production — so if this one drifts, this test keeps passing while + // verifying a query production no longer runs, and the anchor silently stops anchoring. private const string InsurancePricesQuery = "select definition,fee,payout from insuranceprices"; [RequiresGameRootFact] @@ -53,14 +58,30 @@ public void The_insurance_prices_columns_have_the_types_the_code_reads_them_as() } [RequiresGameRootFact] - public void The_insurance_price_recalculation_procedure_exists_and_is_callable() + public void The_insurance_price_recalculation_procedure_exists() { + // Task 8 already asserts every documented procedure exists, so this looks redundant. + // It is not, for two reasons worth stating rather than leaving a reader to reconstruct. + // It anchors the unit tier's `WhenNonQuery("exec usp_RecalculateInsurancePrices", 1)` + // stub exactly as the two tests above anchor the price-query stub. And Task 8's check is + // driven from the contents of docs/db_structure/, so deleting the procedure together + // with its documentation file would pass there and fail here. + // + // The name says only "exists": this asserts a catalog row, it never invokes the + // procedure. Invoking it would write, and nothing in stages 0-4 writes. DatabaseFixture fixture = new(); using SqlConnection connection = fixture.OpenConnection(); using SqlCommand command = connection.CreateCommand(); + + // Schema-qualified deliberately: this database already carries one same-named pair + // across schemas (dbo.extensionSubscriptionStart and opp.extensionSubscriptionStart), + // so a bare name match is not a safe assumption here. command.CommandText = - "select count(*) from sys.objects where type in ('P','PC') and name = 'usp_RecalculateInsurancePrices'"; + "select count(*) from sys.objects o " + + "join sys.schemas s on s.schema_id = o.schema_id " + + "where o.type in ('P','PC') and s.name = 'dbo' " + + "and o.name = 'usp_RecalculateInsurancePrices'"; Assert.Equal(1, (int)command.ExecuteScalar()); } diff --git a/src/Perpetuum.Tests/Unit/DbQueryTests.cs b/src/Perpetuum.Tests/Unit/DbQueryTests.cs index 3fa744ea..6e8a7ec4 100644 --- a/src/Perpetuum.Tests/Unit/DbQueryTests.cs +++ b/src/Perpetuum.Tests/Unit/DbQueryTests.cs @@ -20,6 +20,9 @@ public DbQueryTests(PerpetuumStaticsFixture fixture) [Fact] public void Execute_returns_one_record_per_row() { + // The same literal is executed for real against the live schema by + // Perpetuum.Tests.Integration/Data/InsuranceQueryTests.cs. Nothing links the two at + // compile time; if they drift apart, this stub keeps passing against a fiction. _db.When("select definition,fee,payout from insuranceprices", FakeResultSet.FromRows( ["definition", "fee", "payout"], From ad447906d0c4427e4eba36019280b0e5dd3d926d Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 08:41:34 -0300 Subject: [PATCH 17/24] test: add ISSUE-039 regression pinning the insurance cache reload outside the transaction --- .../Issue039InsuranceTransactionTests.cs | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 src/Perpetuum.Tests/Regression/Issue039InsuranceTransactionTests.cs diff --git a/src/Perpetuum.Tests/Regression/Issue039InsuranceTransactionTests.cs b/src/Perpetuum.Tests/Regression/Issue039InsuranceTransactionTests.cs new file mode 100644 index 00000000..8e50ccfc --- /dev/null +++ b/src/Perpetuum.Tests/Regression/Issue039InsuranceTransactionTests.cs @@ -0,0 +1,86 @@ +using System.Reflection; +using Perpetuum.Services.Insurance; +using Perpetuum.Tests.Fakes.Data; +using Perpetuum.Tests.Infrastructure; +using Xunit; + +namespace Perpetuum.Tests.Regression +{ + /// + /// ISSUE-039. Refresh() recalculated prices inside a TransactionScope and then reloaded the + /// price cache. When the scope was opened with a using declaration it was still alive, and + /// already completed, while the reload ran — so the reload threw and a running server kept + /// quoting stale insurance fees. + /// + /// Revert the fix in InsurancePriceRefreshService.Refresh() and this test fails. + /// + [Collection(PerpetuumStaticsCollection.Name)] + public class Issue039InsuranceTransactionTests + { + private const string RecalculateCommand = "usp_RecalculateInsurancePrices"; + private const string ReloadCommand = "from insuranceprices"; + + private readonly FakeDb _db; + + public Issue039InsuranceTransactionTests(PerpetuumStaticsFixture fixture) + { + fixture.Logger.Clear(); + _db = FakeDb.Install(); + _db.WhenNonQuery(RecalculateCommand, 1); + _db.When(ReloadCommand, + FakeResultSet.FromRows(["definition", "fee", "payout"], [1234, 100.0d, 900.0d])); + } + + private static void InvokeRefresh() + { + InsurancePriceRefreshService service = new(); + MethodInfo refresh = typeof(InsurancePriceRefreshService) + .GetMethod("Refresh", BindingFlags.NonPublic | BindingFlags.Instance) + ?? throw new InvalidOperationException( + "InsurancePriceRefreshService.Refresh() not found. If it was renamed, update this test."); + + refresh.Invoke(service, null); + } + + [Fact] + public void The_recalculation_runs_inside_a_transaction() + { + InvokeRefresh(); + + RecordedCommand? recalculate = _db.LastCommandMatching(RecalculateCommand); + Assert.NotNull(recalculate); + Assert.True( + recalculate!.HadAmbientTransaction, + "usp_RecalculateInsurancePrices must run inside a transaction scope."); + } + + [Fact] + public void The_cache_reload_runs_outside_any_transaction() + { + InvokeRefresh(); + + RecordedCommand? reload = _db.LastCommandMatching(ReloadCommand); + Assert.NotNull(reload); + Assert.False( + reload!.HadAmbientTransaction, + "ISSUE-039: the insurance price cache reload must run after the transaction scope " + + "is disposed, not inside it. A using declaration instead of a using block " + + "reintroduces the defect."); + } + + [Fact] + public void Both_statements_run_and_in_order() + { + InvokeRefresh(); + + List texts = [.. _db.Commands.Select(c => c.CommandText)]; + + int recalculateIndex = texts.FindIndex(t => t.Contains(RecalculateCommand, StringComparison.OrdinalIgnoreCase)); + int reloadIndex = texts.FindIndex(t => t.Contains(ReloadCommand, StringComparison.OrdinalIgnoreCase)); + + Assert.True(recalculateIndex >= 0, "The recalculation command never ran."); + Assert.True(reloadIndex >= 0, "The cache reload never ran."); + Assert.True(recalculateIndex < reloadIndex, "The reload must follow the recalculation."); + } + } +} From 9ba93f3b856b0be2b97c5648b6f5f0b262d8b681 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 08:46:28 -0300 Subject: [PATCH 18/24] test: document the three-red mechanism in the ISSUE-039 regression test Reverting the fix fails every test in the class, not just the one named for the reload. The getter of Transaction.Current throws inside a completed-but-undisposed TransactionScope rather than returning a value, so FakeDbConnection.Open() throws and Refresh() aborts before any assertion runs. The named test is the diagnostic one; the other two fail as collateral. This is also a sharper statement of ISSUE-039 itself: reading the ambient transaction inside a completed scope is the error, which is why production failed at SqlConnection.Open() with the same message. --- .../Regression/Issue039InsuranceTransactionTests.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Perpetuum.Tests/Regression/Issue039InsuranceTransactionTests.cs b/src/Perpetuum.Tests/Regression/Issue039InsuranceTransactionTests.cs index 8e50ccfc..43f36134 100644 --- a/src/Perpetuum.Tests/Regression/Issue039InsuranceTransactionTests.cs +++ b/src/Perpetuum.Tests/Regression/Issue039InsuranceTransactionTests.cs @@ -12,7 +12,13 @@ namespace Perpetuum.Tests.Regression /// already completed, while the reload ran — so the reload threw and a running server kept /// quoting stale insurance fees. /// - /// Revert the fix in InsurancePriceRefreshService.Refresh() and this test fails. + /// Revert the fix in InsurancePriceRefreshService.Refresh() and every test in this class fails, + /// not just the one named for the reload. That is expected: the getter of Transaction.Current + /// throws inside a completed-but-undisposed scope rather than returning a value, so + /// FakeDbConnection.Open() throws and Refresh() aborts before any assertion runs. The named test + /// below is the diagnostic one; the other two fail as collateral. Reading the ambient + /// transaction inside a completed scope being an error is exactly why production failed at + /// SqlConnection.Open() with "The current TransactionScope is already complete." /// [Collection(PerpetuumStaticsCollection.Name)] public class Issue039InsuranceTransactionTests From acd57f1ed0a6bd476743233bffac6ce0a9812953 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 12:03:55 -0300 Subject: [PATCH 19/24] test: add ISSUE-033 regression for roaming presences with no flocks --- .../Regression/Issue033EmptyFlockTests.cs | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 src/Perpetuum.Tests/Regression/Issue033EmptyFlockTests.cs diff --git a/src/Perpetuum.Tests/Regression/Issue033EmptyFlockTests.cs b/src/Perpetuum.Tests/Regression/Issue033EmptyFlockTests.cs new file mode 100644 index 00000000..40666be3 --- /dev/null +++ b/src/Perpetuum.Tests/Regression/Issue033EmptyFlockTests.cs @@ -0,0 +1,69 @@ +using System.Reflection; +using NSubstitute; +using Perpetuum.Tests.Infrastructure; +using Perpetuum.Zones; +using Perpetuum.Zones.NpcSystem.Flocks; +using Perpetuum.Zones.NpcSystem.Presences; +using Perpetuum.Zones.NpcSystem.Presences.PathFinders; +using Xunit; + +namespace Perpetuum.Tests.Regression +{ + /// + /// ISSUE-033. A roaming presence with no flocks, or with flocks holding no members, made + /// TryGetMaxHomeRange and TryGetMinSlope call Max() and Min() on an empty sequence. The + /// throw was caught and logged, so the server stayed up but filled the log with stack + /// traces on every roaming update. The fix added DefaultIfEmpty. + /// + /// Remove either DefaultIfEmpty in FreeRoamingPathFinder and this test fails. + /// + [Collection(PerpetuumStaticsCollection.Name)] + public class Issue033EmptyFlockTests + { + private readonly PerpetuumStaticsFixture _fixture; + + public Issue033EmptyFlockTests(PerpetuumStaticsFixture fixture) + { + _fixture = fixture; + _fixture.Logger.Clear(); + } + + private static object Invoke(string methodName, IRoamingPresence presence) + { + IZone zone = Substitute.For(); + FreeRoamingPathFinder finder = new(zone); + + MethodInfo method = typeof(FreeRoamingPathFinder) + .GetMethod(methodName, BindingFlags.NonPublic | BindingFlags.Instance) + ?? throw new InvalidOperationException( + $"FreeRoamingPathFinder.{methodName} not found. If it was renamed, update this test."); + + return method.Invoke(finder, [presence])!; + } + + private static IRoamingPresence PresenceWithNoFlocks() + { + IRoamingPresence presence = Substitute.For(); + _ = presence.Flocks.Returns([]); + return presence; + } + + [Fact] + public void TryGetMaxHomeRange_on_a_presence_with_no_flocks_logs_no_exception() + { + object result = Invoke("TryGetMaxHomeRange", PresenceWithNoFlocks()); + + Assert.Equal(10, (int)result); + Assert.Empty(_fixture.Logger.Exceptions); + } + + [Fact] + public void TryGetMinSlope_on_a_presence_with_no_flocks_logs_no_exception() + { + object result = Invoke("TryGetMinSlope", PresenceWithNoFlocks()); + + Assert.Equal(ZoneExtensions.MIN_SLOPE, (double)result); + Assert.Empty(_fixture.Logger.Exceptions); + } + } +} From 38bd68ac8f05b297c3484e7b1b6768487aa8e76c Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:04:32 -0300 Subject: [PATCH 20/24] test: apply final review fix wave for automated-test-suite branch Six items from the whole-branch review, landed before the PR opens: 1. EnvironmentDiscoveryTests: Writes_are_disabled_unless_explicitly_allowed now drives PERPETUUM_TESTDB_ALLOW_WRITE itself (unset, "0", "1") instead of reading whatever the shell ambiently carries. The old version ran zero assertions whenever the variable was already set - exactly the case for a developer on a later write-enabled stage - while guarding the only opt-in that protects the operator's real database. 2. DatabaseFixture: renamed the Environment property to LocalEnvironment so it no longer shadows System.Environment; only internal uses existed. 3. StoredProcedureConformanceTests: closed the XML doc comment on ProcedureNameFromDocumentedFileName around all of its prose instead of leaving trailing unwrapped /// lines after . 4. RecordingLogger: restored the comment explaining why Exceptions checks both LogType.Error and ThrownException != null. 5. FakeDb: documented that _results/_nonQueries are intentionally not lock-protected (test-setup-only, unlike _commands) and that When() resolves by first-match-wins. 6. smoke-test.ps1: exit code 2 is also returned when the server binary is not found after a successful build; the docstring now says so. Co-Authored-By: Claude Opus 5 --- .../Infrastructure/DatabaseFixture.cs | 8 ++++---- .../EnvironmentDiscoveryTests.cs | 19 +++++++++++++++++-- .../Schema/StoredProcedureConformanceTests.cs | 2 +- src/Perpetuum.Tests/Fakes/Data/FakeDb.cs | 9 +++++++++ src/Perpetuum.Tests/Fakes/RecordingLogger.cs | 6 ++++++ tools/smoke-test.ps1 | 2 +- 6 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/Perpetuum.Tests.Integration/Infrastructure/DatabaseFixture.cs b/src/Perpetuum.Tests.Integration/Infrastructure/DatabaseFixture.cs index 2271e4f1..7c19fe05 100644 --- a/src/Perpetuum.Tests.Integration/Infrastructure/DatabaseFixture.cs +++ b/src/Perpetuum.Tests.Integration/Infrastructure/DatabaseFixture.cs @@ -9,25 +9,25 @@ namespace Perpetuum.Tests.Integration.Infrastructure /// public sealed class DatabaseFixture { - public GameRootEnvironment? Environment { get; } + public GameRootEnvironment? LocalEnvironment { get; } public string? UnavailableReason { get; } public DatabaseFixture() { _ = GameRootEnvironment.TryLoad(out GameRootEnvironment? env, out string? reason); - Environment = env; + LocalEnvironment = env; UnavailableReason = reason; } public SqlConnection OpenConnection() { - if (Environment is null) + if (LocalEnvironment is null) { throw new InvalidOperationException( $"Database unavailable: {UnavailableReason}. Tests must use [RequiresGameRootFact]."); } - SqlConnection connection = new(Environment.ConnectionString); + SqlConnection connection = new(LocalEnvironment.ConnectionString); connection.Open(); return connection; } diff --git a/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs b/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs index d8f67efe..1318ee8c 100644 --- a/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs +++ b/src/Perpetuum.Tests.Integration/Infrastructure/EnvironmentDiscoveryTests.cs @@ -25,10 +25,25 @@ public void The_database_accepts_a_connection() [Fact] public void Writes_are_disabled_unless_explicitly_allowed() { - // This test runs everywhere, including CI, and documents the default. - if (Environment.GetEnvironmentVariable(GameRootEnvironment.AllowWriteVariable) is null) + // Controls the variable rather than reading whatever the shell happens to carry. Read + // ambiently, this test asserts nothing at all whenever the variable is already set — + // which is precisely the case for anyone working on a later write-enabled stage — and + // it is the only test guarding the opt-in that protects the operator's real database. + string? saved = Environment.GetEnvironmentVariable(GameRootEnvironment.AllowWriteVariable); + try { + Environment.SetEnvironmentVariable(GameRootEnvironment.AllowWriteVariable, null); Assert.False(GameRootEnvironment.WritesAllowed); + + Environment.SetEnvironmentVariable(GameRootEnvironment.AllowWriteVariable, "0"); + Assert.False(GameRootEnvironment.WritesAllowed); + + Environment.SetEnvironmentVariable(GameRootEnvironment.AllowWriteVariable, "1"); + Assert.True(GameRootEnvironment.WritesAllowed); + } + finally + { + Environment.SetEnvironmentVariable(GameRootEnvironment.AllowWriteVariable, saved); } } } diff --git a/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs b/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs index ce82edc3..268c26c2 100644 --- a/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs +++ b/src/Perpetuum.Tests.Integration/Schema/StoredProcedureConformanceTests.cs @@ -35,7 +35,6 @@ private static string RepositoryRoot /// functions/ uses a bare "Name.sql". Taking the filename as the object name would compare /// "dbo.X.StoredProcedure" against "X" and report almost every documented procedure as /// missing, which looks like a large finding about the repository and is not one. - /// /// Procedures keep their schema; functions cannot. Two documented procedures differ only by /// schema — dbo.extensionSubscriptionStart and opp.extensionSubscriptionStart — so comparing /// bare names would let one of them disappear from the database while this test stayed green. @@ -43,6 +42,7 @@ private static string RepositoryRoot /// dbo (opp.ToolTestAccount_GetDefinitionID, which is itself undocumented), so assuming dbo /// there would produce a false failure. The asymmetry is forced by the data, and the /// remaining gap on the function side is stated rather than left to be discovered. + /// private static string ProcedureNameFromDocumentedFileName(string fileNameWithoutExtension) { return fileNameWithoutExtension.EndsWith(".StoredProcedure", StringComparison.OrdinalIgnoreCase) diff --git a/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs b/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs index 6882a798..3faf9c95 100644 --- a/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs +++ b/src/Perpetuum.Tests/Fakes/Data/FakeDb.cs @@ -20,6 +20,10 @@ public sealed record RecordedCommand /// public sealed class FakeDb { + // _commands is lock-protected because production code logs and queries from timer and task + // threads. _results and _nonQueries are deliberately not: they are configured by the test + // before the code under test runs, and never mutated afterwards. Registering a stub from + // inside the code under test would race, and nothing here prevents it. private readonly List<(string Match, FakeResultSet Result)> _results = []; private readonly List<(string Match, int RowsAffected)> _nonQueries = []; private readonly List _commands = []; @@ -32,6 +36,11 @@ public static FakeDb Install() return fake; } + /// + /// Registers a result for any command whose text contains . + /// The first registration that matches wins, so register the most specific pattern first — + /// a broad pattern registered earlier silently shadows a narrower one registered later. + /// public void When(string commandTextContains, FakeResultSet result) => _results.Add((commandTextContains, result)); diff --git a/src/Perpetuum.Tests/Fakes/RecordingLogger.cs b/src/Perpetuum.Tests/Fakes/RecordingLogger.cs index 48992502..2138df10 100644 --- a/src/Perpetuum.Tests/Fakes/RecordingLogger.cs +++ b/src/Perpetuum.Tests/Fakes/RecordingLogger.cs @@ -24,6 +24,12 @@ public IReadOnlyList Events get { lock (_gate) { return [.. _events]; } } } + // LogEvent names the property ThrownException, not Exception — see + // src/Perpetuum/Log/LogEvent.cs:20. Logger.Exception(ex) sets LogType.Error and + // ThrownException together (Logger.cs:62-71), so either condition alone would do; both + // are kept so a future logging path that sets one without the other is excluded. + // Logger.Error(string) is exactly such a path: it writes LogType.Error with no + // exception, and the ISSUE-033 regression depends on not counting those. public IReadOnlyList Exceptions { get { lock (_gate) { return [.. _events.Where(e => e.LogType == LogType.Error && e.ThrownException != null)]; } } diff --git a/tools/smoke-test.ps1 b/tools/smoke-test.ps1 index abca9579..559f8192 100644 --- a/tools/smoke-test.ps1 +++ b/tools/smoke-test.ps1 @@ -6,7 +6,7 @@ .DESCRIPTION Exit codes: 0 pass - 2 build failed + 2 build failed, or the server binary was not found 3 GameRoot not found 4 timed out waiting for the server to come online 5 a forbidden pattern was found in the log From 3ae0bc18054471d7badb4f6503ad2a54a2fa5bf0 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:16:34 -0300 Subject: [PATCH 21/24] test: correct an inaccurate clause in the RecordingLogger filter comment --- src/Perpetuum.Tests/Fakes/RecordingLogger.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Perpetuum.Tests/Fakes/RecordingLogger.cs b/src/Perpetuum.Tests/Fakes/RecordingLogger.cs index 2138df10..32155687 100644 --- a/src/Perpetuum.Tests/Fakes/RecordingLogger.cs +++ b/src/Perpetuum.Tests/Fakes/RecordingLogger.cs @@ -29,7 +29,7 @@ public IReadOnlyList Events // ThrownException together (Logger.cs:62-71), so either condition alone would do; both // are kept so a future logging path that sets one without the other is excluded. // Logger.Error(string) is exactly such a path: it writes LogType.Error with no - // exception, and the ISSUE-033 regression depends on not counting those. + // exception, and this filter excludes it. No test relies on that today. public IReadOnlyList Exceptions { get { lock (_gate) { return [.. _events.Where(e => e.LogType == LogType.Error && e.ThrownException != null)]; } } From a59121c9c2e43f8e3624f80b93d5035542311c1e Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:50:06 -0300 Subject: [PATCH 22/24] ci: run the unit test tier on pushes and pull requests to develop Adds a `test` job to .github/workflows/dotnet.yml running dotnet test src/Perpetuum.Tests/Perpetuum.Tests.csproj \ --no-restore --configuration Release -p:Platform=x64 The job is blocking, with no continue-on-error. Without it the test projects would be merged and never run. It does not reference Perpetuum.Tests.Integration, which needs a configured GameRoot and a live database. Two independent barriers keep that tier out of CI: the job never names the project, and [RequiresGameRoot] would skip its tests even if something did. `build`, `build-admintool-installer` and `publish-wiki` are untouched, including publish-wiki's `needs: build`. Nothing that exists today changes behaviour: `dotnet restore` already resolved the whole solution, and the uploaded artifact still comes from bin/x64/Release/net8.0, which the test projects do not write to. --- .github/workflows/dotnet.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/dotnet.yml b/.github/workflows/dotnet.yml index 9ba354c7..06328b33 100644 --- a/.github/workflows/dotnet.yml +++ b/.github/workflows/dotnet.yml @@ -34,6 +34,21 @@ jobs: path: ${{ env.Workspace }}/bin/x64/Release/net8.0 if: ${{ github.event_name == 'push'}} + test: + + runs-on: windows-latest + + steps: + - uses: actions/checkout@v4 + - name: Setup .NET + uses: actions/setup-dotnet@v4 + with: + dotnet-version: 8.0.x + - name: Restore dependencies + run: dotnet restore + - name: Run unit tests + run: dotnet test src/Perpetuum.Tests/Perpetuum.Tests.csproj --no-restore --configuration Release -p:Platform=x64 + build-admintool-installer: runs-on: windows-latest From c193a11ec0e1a8fefbeaebcb786a92b0f6e0c4cb Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:50:25 -0300 Subject: [PATCH 23/24] docs: record the test suite in the documentation set and the backlog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five documents stated that this repository has no automated tests. That was true when they were written and is no longer true, so a merge without this commit would leave the documentation set contradicting the code. docs/codebase/TESTING.md is rewritten. Current State, Test Infrastructure, CI Pipeline and Adding Tests are replaced; Manual Testing is kept, because no tier covers gameplay behaviour and running the server by hand is still the only way to validate it. Gaps is narrowed to what is still uncovered rather than deleted. The Analysis Date convention is kept with a new date. The document now records how to run each tier, the two environment variables, the four static service locators used as seams, and the seven documented exit codes of the smoke script. CLAUDE.md changes in two places: the Build & Run section gains the unit-tier command, and Testing & Validation stops instructing Claude to propose manual validation as the only option. It now asks for tests first and manual steps for what tests cannot reach, and adds two prohibitions: do not restructure production code to make a test possible without saying so, and do not add a regression test without observing it fail against the unfixed code. ARCHITECTURE.md, CONCERNS.md and STACK.md each carried a one-line claim that no tests exist; each is corrected to say coverage is partial and to point at TESTING.md. The ARCHITECTURE.md edit is a judgement call. CLAUDE.md requires that file to be updated for major architectural changes, and whether a test suite qualifies is arguable. The change made here is minimal — it corrects the existing Architectural Constraints bullet rather than adding a section. If the maintainers would rather see a full section, or nothing at all, say so and it will be changed. docs/backlog/improvements.md gains IMPROVEMENT-045, status IN_PROGRESS, with Last ID used raised from 044 to 045. It records the three tiers, what stages 0-4 delivered, and the six stages that are not started. Two categories of stale claim are deliberately left alone: docs/backlog/ completed.md and docs/superpowers/plans/. Both are dated records of work already done, correct at the time of writing. Editing them would rewrite history rather than document the present. --- CLAUDE.md | 21 +++- docs/backlog/improvements.md | 80 ++++++++++++++- docs/codebase/ARCHITECTURE.md | 6 +- docs/codebase/CONCERNS.md | 8 +- docs/codebase/STACK.md | 12 ++- docs/codebase/TESTING.md | 186 ++++++++++++++++++++++++++++------ 6 files changed, 271 insertions(+), 42 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 17abeaf3..36dbd9a1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,7 +62,14 @@ CI: Output: - `bin/x64/Release/net8.0` -There are currently no automated tests. +Tests: + +```bash +dotnet test src/Perpetuum.Tests/Perpetuum.Tests.csproj -c Release -p:Platform=x64 +``` + +This is the unit tier and needs no setup. The integration tier and the smoke script need a configured +`GameRoot` and a live database — see `docs/codebase/TESTING.md`. --- @@ -280,14 +287,22 @@ Prefer: # Testing & Validation -There is currently no automated test suite. +An automated test suite exists in three tiers — unit, integration and smoke. It does not cover the +whole codebase. `docs/codebase/TESTING.md` states what is covered, what is not, and how to run each +tier. Claude MUST: -- propose manual validation steps +- run the unit tier after changing code it covers +- propose tests first, and manual validation for what tests cannot reach +- propose manual validation steps for gameplay behaviour, which no tier covers - identify affected gameplay systems - identify affected DB state - identify likely regression areas +Claude MUST NOT: +- restructure production code to make a test possible without saying so explicitly +- add a regression test without observing it fail against the unfixed code + --- # Response Expectations diff --git a/docs/backlog/improvements.md b/docs/backlog/improvements.md index 1c4ea21e..ea5cd43e 100644 --- a/docs/backlog/improvements.md +++ b/docs/backlog/improvements.md @@ -1,6 +1,84 @@ # Last ID used -044 +045 + +## IMPROVEMENT-045 - Automated test suite + +Status: IN_PROGRESS +Priority: MEDIUM +Area: Testing / Infrastructure + +### Description + +Introduce an automated test suite covering the whole project, in three tiers: a smoke script that runs +the real server end to end, a unit tier behind fakes for the four static service locators, and an +integration tier that runs against the real database. + +Coverage is partial by design and grows in stages. Stages 0-4 are implemented; stages 5-10 are listed +under Proposed Implementation and are not started. + +### Impact + +Every change to core systems is currently validated by starting a server and watching the log. That +answer does not survive the next change, and it is re-derived by hand every time. + +Two defects found in this repository in the last week — [[ISSUE-033]] and [[ISSUE-039]] — are now +regression tests, each observed failing with its fix reverted. Neither would have been caught by +inspection: ISSUE-039 left a running server quoting stale insurance prices with no error in the log at +all. + +[[ISSUE-038]] already asks in writing for "an automated test client" looping session +connect/disconnect while sampling `dotnet-gcdump`. That is a soak harness rather than a test suite, but +the demand for automation is already on record. + +There is a second reason, and it is the stronger one. The share of AI-authored code in this repository +is rising. The value of an automated suite is that it answers "did this break something" without a +human re-deriving the answer for every contribution, from any author. + +### Proposed Implementation + +Three tiers, because no single tier catches what this repository actually breaks. ISSUE-039 only +manifests when a real `SqlConnection.Open()` reads `Transaction.Current` and finds a completed scope — +a faked connection passes it. + +| Tier | Project | Needs | +|---|---|---| +| 1 — smoke | `tools/smoke-test.ps1` | A configured `GameRoot` and a live database | +| 2 — unit | `src/Perpetuum.Tests` | Nothing; runs in CI | +| 3 — integration | `src/Perpetuum.Tests.Integration` | A configured `GameRoot` and a live database | + +Delivered (stages 0-4): infrastructure and fakes, the smoke script, `Guard.cs` and +`ValueTypeExtensions.cs`, the data layer against both the fake and the real schema, and the two +regression tests. + +Remaining stages, in order: + +| # | Stage | Tier | +|---|---|---| +| 5 | Entity system — `Entity`, `EntityDefault`, `EntityDynamicProperties` in isolation | 2 | +| 6 | Module state machines — transitions in `ActiveModule.States.cs` | 2 | +| 7 | Season service — tier grant, objective completion, leaderboard delivery, intro-mail idempotency, end-of-season processing | 2+3 | +| 8 | Request handlers — fake session/request infrastructure plus one handler per dispatch category | 2 | +| 9 | Mission engine — deterministic resolve against fixed data | 2+3 | +| 10 | Concurrency — only what can be made deterministic: `ProcessManager`, `MessageSender` | 2 | + +### Notes + +- **No production code changes.** The four existing static service locators (`Logger.Current`, + `Db.DbQueryFactory`, `EntityDefault.Reader`, `Entity.Services`) turned out to be sufficient seams for + everything in stages 0-4. If a later stage genuinely cannot be tested without a new seam, that is + raised in the pull request rather than slipped in — `CLAUDE.md` forbids speculative refactors. +- **No synthetic schema.** Tier 3 runs against the real `perpetuumsa`. Duplicating the DDL would drift + from production, and every developer who touches this code already has the standard environment. + `PERPETUUM_GAMEROOT` is the only machine-specific input and its absence makes tests skip, not fail. +- **Coverage is not the goal.** Covering all 585 files of `Perpetuum.RequestHandlers` is explicitly a + non-goal. Stage 8 is one handler per dispatch category, not 200. +- Stage 10 is scoped to what can be made deterministic. A flaky concurrency test is worse than no test: + it trains the team to ignore red. +- Before stages 7 and 9, where the number of stubs multiplies, two hardening items: make the data fake + fail loudly when no registered pattern matches a command instead of returning an empty result set, + and give the assembly-wide recording logger an automatic reset instead of relying on each test class + to clear it. ## IMPROVEMENT-044 - Disable NPC flee behavior (player complaints) diff --git a/docs/codebase/ARCHITECTURE.md b/docs/codebase/ARCHITECTURE.md index e2d491c8..8a563de8 100644 --- a/docs/codebase/ARCHITECTURE.md +++ b/docs/codebase/ARCHITECTURE.md @@ -289,8 +289,10 @@ consumed by zones to clean up zone sessions. These are write-once at startup, read-only thereafter. - **Platform:** `[SupportedOSPlatform("windows")]` on both `Perpetuum.Server` and `Perpetuum.Bootstrapper` assemblies — Windows-only due to native dependencies. -- **No automated tests:** The project has no test projects. Validation is manual or via - the `Perpetuum.AdminTool` WPF application. +- **Partial test coverage:** `src/Perpetuum.Tests` (unit) and `src/Perpetuum.Tests.Integration` + (against the real database) cover the data layer, validation helpers and two regression paths. + Gameplay behaviour is still validated manually or via the `Perpetuum.AdminTool` WPF application. + See `docs/codebase/TESTING.md`. - **SQL Server only:** `Microsoft.Data.SqlClient` is hard-wired; no abstraction layer. ## Anti-Patterns diff --git a/docs/codebase/CONCERNS.md b/docs/codebase/CONCERNS.md index fbbe8c50..37bc3ed1 100644 --- a/docs/codebase/CONCERNS.md +++ b/docs/codebase/CONCERNS.md @@ -170,13 +170,13 @@ The `MissionHandler`, `MissionInProgress`, `MissionProcessorDeliverMission`, `Pr ## Missing Infrastructure -### No Automated Tests +### Partial Test Coverage -The repository has zero automated tests (unit, integration, or functional). CLAUDE.md states this explicitly. +A test suite exists (`src/Perpetuum.Tests`, `src/Perpetuum.Tests.Integration`, `tools/smoke-test.ps1`) but covers only the data layer, validation helpers and two regression paths. The subsystems this document calls high-risk are still untested. -**Impact:** Every change to core systems — combat calculations, mission rewards, market transactions, season point accrual — must be manually validated in a running server with a connected database. Regressions are invisible until they reach production or are found by players. +**Impact:** Changes to combat calculations, mission rewards, market transactions and season point accrual must still be manually validated in a running server with a connected database. Regressions in those areas remain invisible until they reach production or are found by players. -**Fix approach:** Start with pure-logic unit tests for `FastRandom`, economy formulas, mission reward calculations, and season point math. These have no external dependencies and provide immediate value. +**Fix approach:** Continue along the coverage map in `IMPROVEMENT-045` — entity system, module state machines, season service, request handlers, mission engine, concurrency. See `docs/codebase/TESTING.md` for what is covered today. --- diff --git a/docs/codebase/STACK.md b/docs/codebase/STACK.md index b2500cbd..b2cb324d 100644 --- a/docs/codebase/STACK.md +++ b/docs/codebase/STACK.md @@ -70,9 +70,15 @@ - Runner: `windows-latest` - Trigger: push/PR to `develop` branch - Publishes build artifact `Perpetuum-Server-v2-{sha}` on push -- Only `Perpetuum.ServerService2` project is built in CI - -**No automated tests** — no test project, no test framework configured. +- Only `Perpetuum.ServerService2` project is built in the `build` job +- A `test` job runs the unit tier; the integration tier is not referenced in CI + +**Testing:** +- Framework: xUnit v3, with NSubstitute for interface doubles +- `src/Perpetuum.Tests` — unit tier, no external dependencies, runs in CI +- `src/Perpetuum.Tests.Integration` — runs against the real database, skipped when `PERPETUUM_GAMEROOT` is unset +- `tools/smoke-test.ps1` — builds, starts the server, asserts on the startup and shutdown log +- Coverage is partial by design; see `docs/codebase/TESTING.md` **Unsafe code:** - `true` in `src/Perpetuum/Perpetuum.csproj` diff --git a/docs/codebase/TESTING.md b/docs/codebase/TESTING.md index 9b8f2829..9f89bef4 100644 --- a/docs/codebase/TESTING.md +++ b/docs/codebase/TESTING.md @@ -1,18 +1,138 @@ # Testing -**Analysis Date:** 2026-05-11 +**Analysis Date:** 2026-08-14 ## Current State -**No automated test suite exists in this repository.** +The repository has an automated test suite in three tiers. It does not cover the whole codebase — the +coverage map below states what is covered and what is not. -There is no xUnit, NUnit, MSTest, or any other test framework. The CI pipeline (`.github/workflows/dotnet.yml`) runs only `dotnet build` and `dotnet restore` — no `dotnet test` step exists. There are no `*.Tests` projects, no test discovery configuration, and no code coverage tooling. +| Tier | Project | Count | Needs | +|------|---------|-------|-------| +| 1 — smoke | `tools/smoke-test.ps1` | 1 end-to-end run | A configured `GameRoot` and a live database | +| 2 — unit | `src/Perpetuum.Tests` | 58 tests | Nothing. Runs anywhere the solution builds | +| 3 — integration | `src/Perpetuum.Tests.Integration` | 8 tests | A configured `GameRoot` and a live database | + +Tier 2 is the tier that runs in CI. Tiers 1 and 3 run on a developer machine that already has the +standard server environment, and skip rather than fail when it is absent. + +## Running the tests + +Tier 2, no setup required: + +```bash +dotnet test src/Perpetuum.Tests/Perpetuum.Tests.csproj -c Release -p:Platform=x64 +``` + +Tier 3, against the real database: + +```bash +set PERPETUUM_GAMEROOT=C:\PerpetuumServer\data +dotnet test src/Perpetuum.Tests.Integration/Perpetuum.Tests.Integration.csproj -c Release -p:Platform=x64 +``` + +Tier 1, a full server run: + +```bash +pwsh tools/smoke-test.ps1 -GameRoot C:\PerpetuumServer\data +``` + +### Environment variables + +| Variable | Read by | Effect | +|----------|---------|--------| +| `PERPETUUM_GAMEROOT` | Tiers 1 and 3 | Directory holding `perpetuum.ini`. Unset, every tier-3 test that touches the database is **skipped**, not failed | +| `PERPETUUM_TESTDB_ALLOW_WRITE` | Tier 3 | Set to `1` to opt in to tests that write. Unset, only read-only tests run | + +Tier 3 does not carry its own connection string. It deserializes the same `perpetuum.ini` into the same +`GlobalConfiguration` type the bootstrapper uses, so the connection string cannot drift from the one +the server runs with. ## Test Infrastructure -None. There is no test runner, no assertion library, no mock framework, and no test helpers. +### Tier 2 — unit + +Framework: xUnit v3, with NSubstitute for interface doubles. + +The four static service locators the previous version of this document called "the main obstacle" are +the seams the suite uses. Each is a settable `public static` property assigned in exactly one place, so +a fixture assigns a double before exercising code: + +| Seam | Type | Production assignment | +|------|------|-----------------------| +| `Logger.Current` | `ILogger` | `PerpetuumBootstrapper.cs:137` | +| `Db.DbQueryFactory` | `Func` | `PerpetuumBootstrapper.cs:150` | +| `EntityDefault.Reader` | `IEntityDefaultReader` | `PerpetuumBootstrapper.cs:156` | +| `Entity.Services` | `IEntityServices` | `PerpetuumBootstrapper.cs:157` | + +The data layer is the important one. `Db.Query()` funnels 761 call sites through +`Db.DbQueryFactory`, and `DbQuery` takes a `DbConnectionFactory` delegate, so +`Db.DbQueryFactory = () => new DbQuery(() => fakeConnection)` intercepts all of them without any +production change. -Files with "Test" in their name are not unit tests — they are **in-game admin commands** that exercise game logic against a live database while the server is running: +`Fakes/Data/` implements the ADO.NET interfaces as a recording fake: a test registers a result set +against a command pattern, then asserts on the SQL and parameters the code under test actually +produced. `Fakes/RecordingLogger.cs` does the same for log output. + +Because these seams are process-wide static state, the fixtures live in xUnit collections +(`PerpetuumStaticsCollection`) so classes touching them do not run in parallel with each other. + +### Tier 3 — integration + +Runs against the real `perpetuumsa`. No synthetic schema is built and no database is created, +restored or snapshotted: every developer who touches this code already has the standard environment, +and a second copy of the DDL would drift from production. + +Two things are covered: + +- **Schema conformance** — every stored procedure and function documented under `docs/db_structure/` + is checked to exist in the live database with the documented parameter signature. +- **Query anchoring** — the queries that tier 2 stubs are executed against the real schema, so the + fake stays an assertion about how the database actually behaves rather than about how it was + imagined to behave. + +Isolation is by read-only default: writes require `PERPETUUM_TESTDB_ALLOW_WRITE=1`. Tests use a single +connection, because a second concurrent connection inside a `TransactionScope` escalates to MSDTC. + +### Tier 1 — smoke + +`tools/smoke-test.ps1` builds the solution, starts the real server, waits for `State : [Online]`, waits +for the log to quiesce, sends Ctrl+C through `GenerateConsoleCtrlEvent`, and asserts the process +reaches `State : [Off]` and exits 0. Force-killing a server that will not stop is reported as a +failure, not a pass. + +Assertions are in three categories, declared in arrays at the top of the script: + +| Category | Behaviour | +|----------|-----------| +| Required | Absence fails the run — `State : [Online]`, `State : [Off]` | +| Forbidden | Presence fails the run — exception signatures in the startup path | +| Reported | Printed, never asserted — flock count, members spawned, time to online | + +The third category is deliberate. Across recorded runs the spawned member count was 6406, 6423 and +6425; it changes with every content patch, so asserting on it would build a test that fails when the +game works. + +Exit codes: `0` pass, `2` build failed, `3` GameRoot not found, `4` timed out waiting for online, +`5` forbidden pattern in the log, `6` shutdown was not graceful, `7` unexpected error. + +## Regression tests + +Two tests in `src/Perpetuum.Tests/Regression/` exist because a specific bug reached production: + +| Test | Guards | +|------|--------| +| `Issue033EmptyFlockTests` | `FreeRoamingPathFinder` throwing on a presence with no flocks | +| `Issue039InsuranceTransactionTests` | `LoadInsurancePrices()` running inside an already-completed `TransactionScope`, which left the price cache stale | + +Both were **observed failing with their fixes reverted** before being accepted. A regression test that +has never been seen red is a statement about nothing. + +## Files that are not tests + +Files with "Test" in their name in the production projects are **in-game admin commands**, not +automated tests. They are dispatched like player commands via `IRequestHandler` with +`AccessLevel.admin` and need a live server: | File | What it does | |------|-------------| @@ -20,14 +140,13 @@ Files with "Test" in their name are not unit tests — they are **in-game admin | `src/Perpetuum.RequestHandlers/Zone/ZonePBSTest.cs` | Resets PBS highway bits on a live zone terrain | | `src/Perpetuum.RequestHandlers/Zone/ZoneTerraformTest.cs` | In-game terraform operation test | | `src/Perpetuum.RequestHandlers/Missions/MissionResolveTest.cs` | Admin command that invokes `MissionResolveTester` against live DB | -| `src/Perpetuum/Services/MissionEngine/MissionResolveTester.cs` | Parallel mission resolve batch-runner; writes results to `missiontolocation` and `missiontargetslog` tables | +| `src/Perpetuum/Services/MissionEngine/MissionResolveTester.cs` | Parallel mission resolve batch-runner; writes results to `missiontolocation` and `missiontargetslog` | | `src/Perpetuum/Services/MissionEngine/OneLocationTest.cs` | Single-location mission resolve helper used by `MissionResolveTester` | -These are dispatched exactly like player commands (via `IRequestHandler`) with `AccessLevel.admin` and require a live game server with a connected SQL Server database. They are not isolated or repeatable without the full runtime. - ## Manual Testing -The team tests by running the server locally against a configured `GameRoot`: +Automated tiers do not replace running the server. For anything touching gameplay, the team still tests +by running locally against a configured `GameRoot`: ```bash cd src/Perpetuum.Server @@ -41,42 +160,51 @@ Manual verification involves: - Inspecting server console log output (`Logger.Info/Warning/Error`) and file logs under `logs/` - Querying the SQL Server database directly to verify data state -The `Perpetuum.AdminTool` project (`src/Perpetuum.AdminTool/`) provides a GUI tool for administrative operations including the Seasons Admin Tool. +The `Perpetuum.AdminTool` project (`src/Perpetuum.AdminTool/`) provides a GUI tool for administrative +operations including the Seasons Admin Tool. ## CI Pipeline -`.github/workflows/dotnet.yml` runs on pushes and pull requests to `develop` branch only: +`.github/workflows/dotnet.yml` runs on pushes and pull requests to `develop` only. It has four jobs: -```yaml -- dotnet restore -- dotnet build src/Perpetuum.ServerService2/... --configuration Release -p:Platform=x64 -``` +| Job | What it runs | +|-----|--------------| +| `build` | `dotnet build src/Perpetuum.ServerService2/...` and uploads the artifact on push | +| `test` | `dotnet test src/Perpetuum.Tests/Perpetuum.Tests.csproj` — tier 2 only | +| `build-admintool-installer` | Builds the AdminTool MSI | +| `publish-wiki` | Publishes the graphify codebase report | -A build artifact is uploaded on successful push. No tests are executed in CI. +The `test` job does not reference the integration project, and `[RequiresGameRoot]` would skip it even +if something did. Two independent barriers, because one alone is a convention. ## Gaps -**Everything is untested at the automated level.** Specific high-risk gaps: +Covered so far: `Guard.cs`, `ValueTypeExtensions.cs`, the database query layer, and the two regression +paths above. Still untested at the automated level: - **Entity system** (`src/Perpetuum/EntityFramework/`) — `Entity`, `EntityDefault`, `EntityDynamicProperties` have no isolation tests. These underpin all game objects. - **Module state machines** (`src/Perpetuum/Modules/ActiveModule.States.cs`) — state transitions for robot equipment are tested only by playing the game. -- **Guard/validation extensions** (`src/Perpetuum/Guard.cs`) — the `ThrowIf*` extension methods are used pervasively but never unit-tested. -- **Database query layer** (`src/Perpetuum/Data/DbQuery.cs`, `Db.cs`) — no integration test harness for SQL query correctness. - **Request handlers** (`src/Perpetuum.RequestHandlers/`) — 200+ handler classes have no test doubles or mock session/request infrastructure. -- **Concurrent/threading code** — `ProcessManager`, `MessageSender`, `TcpConnection` use `ThreadPool` and `Task.Run` patterns that are notoriously difficult to test without a harness. +- **Concurrent/threading code** — `ProcessManager`, `MessageSender`, `TcpConnection` use `ThreadPool` and `Task.Run` patterns that need a harness before they can be tested deterministically. - **Season service logic** (`src/Perpetuum/Services/Seasons/SeasonService.cs`) — tier grant, objective completion, leaderboard delivery, intro mail idempotency, and end-of-season processing are exercised only via live play. -- **Mission engine** (`src/Perpetuum/Services/MissionEngine/`) — the most complex subsystem; the existing `MissionResolveTester` exercises resolve logic but requires a live DB and has no assertions — it logs results to tables for human inspection. +- **Mission engine** (`src/Perpetuum/Services/MissionEngine/`) — the most complex subsystem; `MissionResolveTester` exercises resolve logic but requires a live DB and has no assertions. -## Adding Tests (If Introduced) +Covering every file is not the goal. See `IMPROVEMENT-045` in `docs/backlog/improvements.md` for the +planned order of attack. -To add automated tests, the recommended path would be: +## Adding Tests -1. Create a `Perpetuum.Tests` project (xUnit recommended for .NET 8) -2. Add project reference to `Perpetuum` core library -3. The main obstacle is the pervasive use of static service locators (`Entity.Services`, `EntityDefault.Reader`, `Db.DbQueryFactory`, `Logger.Current`) — these would need to be initialized or replaced with test doubles before any entity or database code can run in isolation -4. `Guard.cs` extension methods and `ValueTypeExtensions.cs` are pure functions with no dependencies — good first candidates for unit tests -5. `SeasonRepository` methods are testable with an in-memory or test SQL database since they only use `Db.Query()` which is factory-injected +1. Pure functions and validation helpers go in `src/Perpetuum.Tests/Unit/`. +2. Anything that reaches the database goes through the fake in `Fakes/Data/`. Register the result set, + exercise the code, assert on the SQL and parameters it produced. +3. Anything that needs the real schema goes in `src/Perpetuum.Tests.Integration` behind + `[RequiresGameRootFact]`, so it skips on a machine without the environment instead of failing. +4. A test written for a bug must be observed failing before the fix is applied, or reverted against + afterwards. Otherwise it proves nothing. +5. Production code is not restructured to make a test possible. The four seams above have been enough + so far; if a test genuinely cannot be written without a new seam, that is a discussion to have in + the pull request, not a refactor to slip in. --- -*Testing analysis: 2026-05-11* +*Testing analysis: 2026-08-14* From 8c1c40bdc17f3fd037095c69288a108cca89f067 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:53:03 -0300 Subject: [PATCH 24/24] docs: correct the Db.Query() call site count in TESTING.md The figure was 761, which counted the whole solution including Perpetuum.AdminTool. Measured across Perpetuum and Perpetuum.RequestHandlers, the projects the fake actually intercepts, it is 755. The scope is now stated alongside the number. --- docs/codebase/TESTING.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/codebase/TESTING.md b/docs/codebase/TESTING.md index 9f89bef4..40d0456a 100644 --- a/docs/codebase/TESTING.md +++ b/docs/codebase/TESTING.md @@ -65,8 +65,9 @@ a fixture assigns a double before exercising code: | `EntityDefault.Reader` | `IEntityDefaultReader` | `PerpetuumBootstrapper.cs:156` | | `Entity.Services` | `IEntityServices` | `PerpetuumBootstrapper.cs:157` | -The data layer is the important one. `Db.Query()` funnels 761 call sites through -`Db.DbQueryFactory`, and `DbQuery` takes a `DbConnectionFactory` delegate, so +The data layer is the important one. `Db.Query()` funnels 755 call sites in `Perpetuum` and +`Perpetuum.RequestHandlers` through `Db.DbQueryFactory`, and `DbQuery` takes a +`DbConnectionFactory` delegate, so `Db.DbQueryFactory = () => new DbQuery(() => fakeConnection)` intercepts all of them without any production change.