Skip to content

Commit e543e63

Browse files
OgeonX-AiAitomates
andauthored
test(coverage): ratchet gsd-orchestrator branch gate (#16)
* fix: add JsonStringEnumConverter and flush-to-disk before atomic rename to prevent checkpoint corruption and replay errors * feat(26-01): add coverlet.runsettings with declarative boilerplate exclusions - Configure XPlat Code Coverage collector: cobertura format, SkipAutoProps, ExcludeByAttribute (Obsolete/GeneratedCode/CompilerGenerated/ExcludeFromCodeCoverage), ExcludeByFile globs for obj/, generated .g.cs, and the Program.cs bootstrap entrypoint - Measured branch-rate baseline under new settings: 0.6913 (line-rate 0.9082) - Gitignore TestResults/ build-artifact output * feat(26-01): rewrite CI gate to ratcheted branch-coverage threshold, add coverage tests - ci.yml "Test" step now passes --settings coverlet.runsettings - Replace the permanently-red line-rate==100% gate with "Enforce branch coverage (ratchet)": parses branch-rate (not line-rate) from coverage.cobertura.xml, compares to $Baseline = 0.7314, fails closed with CAS JSON telemetry on regression or missing coverage file - Add CoverageGapClosingTests.cs: 37 new tests with meaningful assertions (state, telemetry payload, thrown exception type) targeting previously uncovered branches in SdlcWorkflowMap.PhaseIdForState (0.14 -> full switch), SdlcProfile.ResolveRollbackOrigin fallback paths, GsdWorkflowContext.Transition Failed-state branch, WithSdlcVerification passed-branch, LoopPolicyGuard happy-path/unknown-action branches, NativeProcessCommandExecutor (real cross-platform process spawns, was 0% branch-rate), and McpTerminalOutcomePublisher status-mapping switch (was 0% branch-rate) - Branch-rate raised from measured Task-1 baseline 0.6913 to 0.7314 (branches-covered 730/998), line-rate 0.9082 -> 0.9349 - All 268 tests pass (231 existing + 37 new) --------- Co-authored-by: Kim Harjamäki <kim.harjamaki@prosimo.fi>
1 parent a01b130 commit e543e63

5 files changed

Lines changed: 334 additions & 2 deletions

File tree

.github/workflows/ci.yml

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,40 @@ jobs:
4040
run: dotnet test src/GsdOrchestrator.Tests/GsdOrchestrator.Tests.csproj --configuration Release --no-build --filter "FullyQualifiedName~ContractCompatibilityTests"
4141

4242
- name: Test
43-
run: dotnet test src/GsdOrchestrator.Tests/GsdOrchestrator.Tests.csproj --configuration Release --logger trx --no-build --collect:"XPlat Code Coverage" --results-directory ./TestResults
43+
run: dotnet test src/GsdOrchestrator.Tests/GsdOrchestrator.Tests.csproj --configuration Release --logger trx --no-build --settings coverlet.runsettings --collect:"XPlat Code Coverage" --results-directory ./TestResults
4444

4545
- name: Upload coverage
4646
uses: actions/upload-artifact@v7
4747
if: always()
4848
with:
4949
name: coverage-results
5050
path: TestResults/
51+
52+
- name: Enforce branch coverage (ratchet)
53+
shell: pwsh
54+
run: |
55+
# Ratcheted branch-coverage gate (Phase 26-01). Reads branch-rate (NOT
56+
# line-rate) from the cobertura report produced by coverlet's "XPlat
57+
# Code Coverage" collector and fails if it regresses below the last
58+
# measured baseline. Baseline is raised only when new tests push
59+
# coverage strictly higher (see 26-01-SUMMARY.md for history).
60+
$Baseline = 0.7314
61+
62+
$coverageFile = Get-ChildItem -Path TestResults -Recurse -Filter "coverage.cobertura.xml" | Select-Object -First 1
63+
64+
if (-not $coverageFile) {
65+
Write-Output '{"event":"ci_failure","error":"Coverage file not found"}'
66+
exit 1
67+
}
68+
69+
[xml]$xml = Get-Content $coverageFile.FullName
70+
$rate = [double]$xml.coverage.'branch-rate'
71+
$percent = [math]::Round($rate * 100, 2)
72+
$requiredPercent = [math]::Round($Baseline * 100, 2)
73+
74+
if ($rate -lt $Baseline) {
75+
Write-Output "{`"event`":`"ci_failure`",`"metric`":`"branch-rate`",`"coverage_percent`":$percent,`"required`":$requiredPercent}"
76+
exit 1
77+
}
78+
79+
Write-Output "{`"event`":`"ci_success`",`"metric`":`"branch-rate`",`"coverage_percent`":$percent,`"required`":$requiredPercent}"

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ obj/
1111
.vs/
1212
*.suo
1313

14+
# Coverage / test run output
15+
TestResults/
16+
1417
# NuGet
1518
*.nupkg
1619
packages/

coverlet.runsettings

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<!--
3+
Declarative coverlet configuration for the "XPlat Code Coverage" DataCollector.
4+
Centralizes boilerplate-exclusion decisions (26-CONTEXT.md: 100% LOGICAL branch
5+
coverage; framework boilerplate ignored via standard attributes/pragmas) instead
6+
of scattering them across ad-hoc CLI flags.
7+
-->
8+
<RunSettings>
9+
<DataCollectionRunSettings>
10+
<DataCollectors>
11+
<DataCollector friendlyName="XPlat code coverage">
12+
<Configuration>
13+
<Format>cobertura</Format>
14+
<SkipAutoProps>true</SkipAutoProps>
15+
<ExcludeByAttribute>Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute,ExcludeFromCodeCoverageAttribute</ExcludeByAttribute>
16+
<ExcludeByFile>**/obj/**,**/*.g.cs,**/Program.cs</ExcludeByFile>
17+
</Configuration>
18+
</DataCollector>
19+
</DataCollectors>
20+
</DataCollectionRunSettings>
21+
</RunSettings>
Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
using System.Text.Json.Nodes;
2+
using GsdOrchestrator.Loop;
3+
using GsdOrchestrator.Mcp;
4+
using GsdOrchestrator.Scheduling;
5+
using GsdOrchestrator.Verification;
6+
using GsdOrchestrator.Workflows.Models;
7+
using Microsoft.Extensions.Logging.Abstractions;
8+
using NSubstitute;
9+
using Polly.Registry;
10+
using Xunit;
11+
12+
namespace GsdOrchestrator.Tests;
13+
14+
/// <summary>
15+
/// Phase 26-01: targeted tests for previously-uncovered branches identified from the
16+
/// Task-1 cobertura baseline (branch-rate 0.6913). Each test asserts observable
17+
/// behavior (return value, thrown exception type, or published side-effect) per the
18+
/// plan's threat model (T-26-01: no assertion-free coverage padding).
19+
/// </summary>
20+
public class CoverageGapClosingTests
21+
{
22+
// ---- SdlcWorkflowMap.PhaseIdForState: 0.1428 branch-rate (2/14) before this test ----
23+
[Theory]
24+
[InlineData(WorkflowState.Idle, "understand")]
25+
[InlineData(WorkflowState.Triaging, "understand")]
26+
[InlineData(WorkflowState.Analyzing, "research")]
27+
[InlineData(WorkflowState.Branching, "analyze")]
28+
[InlineData(WorkflowState.Editing, "implement")]
29+
[InlineData(WorkflowState.TestGenerating, "verify")]
30+
[InlineData(WorkflowState.Validating, "verify")]
31+
[InlineData(WorkflowState.Committing, "document")]
32+
[InlineData(WorkflowState.PrCreating, "review")]
33+
[InlineData(WorkflowState.Reviewing, "review")]
34+
[InlineData(WorkflowState.Documenting, "update-memory")]
35+
[InlineData(WorkflowState.Done, "finished")]
36+
[InlineData(WorkflowState.Failed, "finished")]
37+
public void PhaseIdForState_MapsEveryWorkflowStateToExpectedSdlcPhase(WorkflowState state, string expectedPhaseId)
38+
{
39+
Assert.Equal(expectedPhaseId, SdlcWorkflowMap.PhaseIdForState(state));
40+
}
41+
42+
[Fact]
43+
public void PhaseIdForState_UnknownEnumValue_FallsBackToUnderstand()
44+
{
45+
// Cast an out-of-range int to exercise the switch expression's default arm.
46+
var unknown = (WorkflowState)999;
47+
Assert.Equal("understand", SdlcWorkflowMap.PhaseIdForState(unknown));
48+
}
49+
50+
// ---- SdlcProfile.ResolveRollbackOrigin: only the "invalidated phase provided" branch was covered ----
51+
[Fact]
52+
public void ResolveRollbackOrigin_NoInvalidatedPhases_FallsBackToPhaseRollbackTo()
53+
{
54+
var profile = SdlcProfile.CasSdlcV1;
55+
56+
var origin = profile.ResolveRollbackOrigin("verify", []);
57+
58+
Assert.Equal("implement", origin);
59+
}
60+
61+
[Fact]
62+
public void ResolveRollbackOrigin_UnknownPhaseAndNoInvalidated_ReturnsPhaseIdItself()
63+
{
64+
var profile = SdlcProfile.CasSdlcV1;
65+
66+
var origin = profile.ResolveRollbackOrigin("no-such-phase", []);
67+
68+
Assert.Equal("no-such-phase", origin);
69+
}
70+
71+
[Fact]
72+
public void GetPhase_KnownPhaseId_ReturnsDefinition()
73+
{
74+
var profile = SdlcProfile.CasSdlcV1;
75+
76+
var phase = profile.GetPhase("implement");
77+
78+
Assert.Equal("Implement", phase.Name);
79+
Assert.Equal(SdlcBatch.Change, phase.Batch);
80+
}
81+
82+
[Fact]
83+
public void TryGetPhase_UnknownPhaseId_ReturnsNull()
84+
{
85+
var profile = SdlcProfile.CasSdlcV1;
86+
87+
Assert.Null(profile.TryGetPhase("no-such-phase"));
88+
}
89+
90+
// ---- GsdWorkflowContext.Transition: Failed-state branch was uncovered ----
91+
[Fact]
92+
public void Transition_ToFailed_RecordsFailedStateAndFailureReason()
93+
{
94+
var ctx = new GsdWorkflowContext { FailureReason = "boom" };
95+
96+
var next = ctx.Transition(WorkflowState.Failed, "verification failed");
97+
98+
Assert.Equal(WorkflowState.Failed, next.CurrentState);
99+
Assert.Equal(WorkflowState.Idle, next.FailedState);
100+
Assert.Equal("boom", next.FailureReason);
101+
Assert.Single(next.History);
102+
Assert.Equal(WorkflowState.Failed, next.History[0].To);
103+
}
104+
105+
[Fact]
106+
public void Transition_ToFailed_PreservesExistingFailedStateAcrossRepeatedFailures()
107+
{
108+
var ctx = new GsdWorkflowContext().Transition(WorkflowState.Failed, "first failure");
109+
110+
var again = ctx.Transition(WorkflowState.Failed, "second failure");
111+
112+
// FailedState should stick to the first-recorded failure origin (Idle), not be overwritten.
113+
Assert.Equal(WorkflowState.Idle, again.FailedState);
114+
}
115+
116+
[Fact]
117+
public void Transition_ToNonFailedState_ClearsFailureMetadataAndResetsRetryCount()
118+
{
119+
var ctx = new GsdWorkflowContext { RetryCount = 3, FailedState = WorkflowState.Editing, FailureReason = "boom" };
120+
121+
var next = ctx.Transition(WorkflowState.Branching);
122+
123+
Assert.Equal(0, next.RetryCount);
124+
Assert.Null(next.FailedState);
125+
Assert.Null(next.FailureReason);
126+
}
127+
128+
// ---- GsdWorkflowContext.WithSdlcVerification: only the "failed" branch was covered ----
129+
[Fact]
130+
public void WithSdlcVerification_Passed_ClearsPendingRollbackAndResetsNoProgressCount()
131+
{
132+
var ctx = new GsdWorkflowContext().WithSdlcRun("Improve the loop");
133+
134+
var next = ctx.WithSdlcVerification(new SdlcVerificationOutcome(
135+
"research", "repo-verifier", Passed: true, InvalidatedPhaseIds: [], Reason: null));
136+
137+
Assert.Null(next.PendingRollback);
138+
Assert.Equal(SdlcPhaseStatus.Passed, next.SdlcRun!.CurrentPhaseStatus);
139+
Assert.Equal(0, next.SdlcRun.NoProgressCount);
140+
}
141+
142+
[Fact]
143+
public void WithSdlcVerification_NoSdlcRun_ReturnsContextUnchanged()
144+
{
145+
var ctx = new GsdWorkflowContext();
146+
147+
var next = ctx.WithSdlcVerification(new SdlcVerificationOutcome(
148+
"research", "repo-verifier", Passed: false, InvalidatedPhaseIds: [], Reason: "n/a"));
149+
150+
Assert.Null(next.SdlcRun);
151+
Assert.Null(next.PendingRollback);
152+
}
153+
154+
[Fact]
155+
public void WithSdlcPhaseExecution_NoSdlcRun_ReturnsContextUnchanged()
156+
{
157+
var ctx = new GsdWorkflowContext();
158+
159+
var next = ctx.WithSdlcPhaseExecution(new SdlcPhaseExecutionRecord(
160+
"understand", SdlcPhaseStatus.Passed, "repo-verifier", "digest-1", "prompt text"));
161+
162+
Assert.Null(next.SdlcRun);
163+
}
164+
165+
// ---- LoopPolicyGuard: happy-path and unknown-action branches were uncovered ----
166+
[Fact]
167+
public void RequireReadablePath_NoEnvSegment_DoesNotThrow()
168+
{
169+
var exception = Record.Exception(() => LoopPolicyGuard.RequireReadablePath("repo/src/Program.cs"));
170+
Assert.Null(exception);
171+
}
172+
173+
[Fact]
174+
public void EvaluateExternalAction_ApprovedKnownAction_ReturnsAuthorized()
175+
{
176+
Assert.Equal(ExternalActionDecision.Authorized, LoopPolicyGuard.EvaluateExternalAction("push", approved: true));
177+
}
178+
179+
[Fact]
180+
public void EvaluateExternalAction_UnknownAction_ThrowsArgumentOutOfRangeException()
181+
{
182+
Assert.Throws<ArgumentOutOfRangeException>(
183+
() => LoopPolicyGuard.EvaluateExternalAction("unknown_action", approved: true));
184+
}
185+
186+
// ---- NativeProcessCommandExecutor: 0% branch-rate before this test (only exercised via FakeExecutor elsewhere) ----
187+
[Fact]
188+
public async Task NativeProcessCommandExecutor_EmptyCommand_ReturnsToolMissingWithoutSpawningProcess()
189+
{
190+
var executor = new NativeProcessCommandExecutor(Environment.CurrentDirectory);
191+
var check = new VerificationCheck("empty", VerificationCategory.Build, [], true, TimeSpan.FromSeconds(5));
192+
193+
var execution = await executor.ExecuteAsync(check, CancellationToken.None);
194+
195+
Assert.Equal(-1, execution.ExitCode);
196+
Assert.True(execution.ToolMissing);
197+
Assert.False(execution.TimedOut);
198+
}
199+
200+
[Fact]
201+
public async Task NativeProcessCommandExecutor_SuccessfulCommand_ReturnsZeroExitCode()
202+
{
203+
var executor = new NativeProcessCommandExecutor(Environment.CurrentDirectory);
204+
var command = OperatingSystem.IsWindows()
205+
? new List<string> { "cmd.exe", "/c", "exit 0" }
206+
: new List<string> { "/bin/sh", "-c", "exit 0" };
207+
var check = new VerificationCheck("success", VerificationCategory.Build, command, true, TimeSpan.FromSeconds(15));
208+
209+
var execution = await executor.ExecuteAsync(check, CancellationToken.None);
210+
211+
Assert.Equal(0, execution.ExitCode);
212+
Assert.False(execution.TimedOut);
213+
Assert.False(execution.ToolMissing);
214+
}
215+
216+
[Fact]
217+
public async Task NativeProcessCommandExecutor_NonZeroExitCommand_ReturnsThatExitCode()
218+
{
219+
var executor = new NativeProcessCommandExecutor(Environment.CurrentDirectory);
220+
var command = OperatingSystem.IsWindows()
221+
? new List<string> { "cmd.exe", "/c", "exit 7" }
222+
: new List<string> { "/bin/sh", "-c", "exit 7" };
223+
var check = new VerificationCheck("nonzero", VerificationCategory.Build, command, true, TimeSpan.FromSeconds(15));
224+
225+
var execution = await executor.ExecuteAsync(check, CancellationToken.None);
226+
227+
Assert.Equal(7, execution.ExitCode);
228+
Assert.False(execution.ToolMissing);
229+
}
230+
231+
[Fact]
232+
public async Task NativeProcessCommandExecutor_MissingExecutable_ReturnsToolMissing()
233+
{
234+
var executor = new NativeProcessCommandExecutor(Environment.CurrentDirectory);
235+
var check = new VerificationCheck(
236+
"missing", VerificationCategory.Build,
237+
[$"definitely-not-a-real-executable-{Guid.NewGuid():N}.exe"], true, TimeSpan.FromSeconds(5));
238+
239+
var execution = await executor.ExecuteAsync(check, CancellationToken.None);
240+
241+
Assert.Equal(-1, execution.ExitCode);
242+
Assert.True(execution.ToolMissing);
243+
Assert.False(execution.TimedOut);
244+
}
245+
246+
// ---- McpTerminalOutcomePublisher: 0% branch-rate before this test (status switch never exercised) ----
247+
[Theory]
248+
[InlineData(GoalStatus.Completed, "completed")]
249+
[InlineData(GoalStatus.Cancelled, "cancelled")]
250+
[InlineData(GoalStatus.Blocked, "blocked")]
251+
[InlineData(GoalStatus.BudgetExhausted, "budget_exhausted")]
252+
[InlineData(GoalStatus.Failed, "failed")]
253+
[InlineData(GoalStatus.Draft, "failed")] // default arm of the switch
254+
public async Task McpTerminalOutcomePublisher_PublishAsync_MapsGoalStatusToWireStatus(GoalStatus status, string expectedWireStatus)
255+
{
256+
var client = Substitute.For<IMcpClient>();
257+
client.CallToolAsync(Arg.Any<string>(), Arg.Any<JsonObject>(), Arg.Any<CancellationToken>())
258+
.Returns(Task.FromResult(new McpToolResult("ok", false)));
259+
var registry = new ResiliencePipelineRegistry<string>();
260+
registry.TryAddBuilder("mcp-tools", (b, _) => { });
261+
var dispatcher = new McpToolDispatcher(client, registry, NullLogger<McpToolDispatcher>.Instance);
262+
var publisher = new McpTerminalOutcomePublisher(dispatcher);
263+
var outcome = new TerminalLoopOutcome("goal-1", "corr-1", status, "done", ["cas://evidence/1"]);
264+
265+
await publisher.PublishAsync(outcome, CancellationToken.None);
266+
267+
await client.Received(1).CallToolAsync(
268+
"record_terminal_outcome",
269+
Arg.Is<JsonObject>(obj =>
270+
obj["goal_id"]!.GetValue<string>() == "goal-1" &&
271+
obj["status"]!.GetValue<string>() == expectedWireStatus &&
272+
obj["summary"]!.GetValue<string>() == "done"),
273+
Arg.Any<CancellationToken>());
274+
}
275+
}

src/GsdOrchestrator/Checkpointing/FileCheckpointStore.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ public sealed class FileCheckpointStore : ICheckpointStore
2626
private static readonly JsonSerializerOptions JsonOpts = new()
2727
{
2828
WriteIndented = true,
29-
PropertyNamingPolicy = JsonNamingPolicy.CamelCase
29+
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
30+
Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() }
3031
};
3132

3233
/// <summary>
@@ -53,7 +54,10 @@ public async Task SaveAsync(GsdWorkflowContext ctx, CancellationToken ct = defau
5354
var tmp = path + ".tmp";
5455

5556
await using (var fs = new FileStream(tmp, FileMode.Create, FileAccess.Write, FileShare.None))
57+
{
5658
await JsonSerializer.SerializeAsync(fs, ctx, JsonOpts, ct);
59+
fs.Flush(flushToDisk: true);
60+
}
5761

5862
// Atomic rename — prevents partial writes leaving corrupt checkpoints
5963
File.Move(tmp, path, overwrite: true);

0 commit comments

Comments
 (0)