fix: a closed issue marks the failing test case Passed

Closed means fixed. The case that was failing on that bug passes, and reopening
the issue undoes it:

  closed + Failed -> Passed
  open   + Passed -> Failed

Replaces the previous closed -> NotYetTested, which sent the case back for
another run instead of recording the outcome.

The guard that matters is unchanged: only those two states are the
reconciler's to move, so a case a tester has Claimed — or one nobody has run
yet — is never rewritten underneath them.
This commit is contained in:
Alexa
2026-07-28 17:16:44 +03:00
parent 7ce106ed1b
commit 0af23c26b1
3 changed files with 28 additions and 30 deletions
@@ -30,9 +30,9 @@ public sealed class TestCaseGitHubIssueTests : IClassFixture<DorkNetServerFactor
public TestCaseGitHubIssueTests(DorkNetServerFactory factory) => _factory = factory; public TestCaseGitHubIssueTests(DorkNetServerFactory factory) => _factory = factory;
private const int StatusNotYetTested = 0;
private const int StatusClaimed = 1; private const int StatusClaimed = 1;
private const int StatusFailed = 2; private const int StatusFailed = 2;
private const int StatusPassed = 3;
[Fact] [Fact]
public async Task Filing_is_idempotent_and_stores_the_issue_url() public async Task Filing_is_idempotent_and_stores_the_issue_url()
@@ -69,7 +69,7 @@ public sealed class TestCaseGitHubIssueTests : IClassFixture<DorkNetServerFactor
} }
[Fact] [Fact]
public async Task A_closed_issue_sends_the_case_back_for_retesting_not_to_passed() public async Task A_closed_issue_marks_the_failing_case_passed()
{ {
var github = new FakeGitHub(); var github = new FakeGitHub();
using var factory = WithGitHub(github); using var factory = WithGitHub(github);
@@ -87,9 +87,9 @@ public sealed class TestCaseGitHubIssueTests : IClassFixture<DorkNetServerFactor
var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>(); var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();
var tc = await db.TestCases.SingleAsync(c => c.Id == caseId); var tc = await db.TestCases.SingleAsync(c => c.Id == caseId);
// Closing an issue is a developer saying the fix landed — not a tester // A closed issue means the bug is fixed, so the case that was failing
// saying the case passes. It goes back in the queue, not to Passed. // on it passes.
Assert.Equal(StatusNotYetTested, tc.Status); Assert.Equal(StatusPassed, tc.Status);
} }
[Fact] [Fact]
@@ -143,7 +143,8 @@ public sealed class TestCaseGitHubIssueTests : IClassFixture<DorkNetServerFactor
var tc = await db2.TestCases.SingleAsync(c => c.Id == caseId); var tc = await db2.TestCases.SingleAsync(c => c.Id == caseId);
// An open issue against a claimed case says nothing new; flipping it to // An open issue against a claimed case says nothing new; flipping it to
// Failed would yank it out from under whoever is testing it. // Failed would yank it out from under whoever is testing it. Only
// Failed<->Passed are the reconciler's to move.
Assert.Equal(StatusClaimed, tc.Status); Assert.Equal(StatusClaimed, tc.Status);
} }
@@ -33,8 +33,8 @@ public class TestCaseIssuesController(
{ {
/// <summary>Test-case status values, matching the client's /// <summary>Test-case status values, matching the client's
/// <c>TestCaseStatus</c> enum.</summary> /// <c>TestCaseStatus</c> enum.</summary>
private const int StatusNotYetTested = 0;
private const int StatusFailed = 2; private const int StatusFailed = 2;
private const int StatusPassed = 3;
/// <summary>POST <c>api/testcasemanagement/v1/testcase/{id}/issue</c> — /// <summary>POST <c>api/testcasemanagement/v1/testcase/{id}/issue</c> —
/// file a GitHub issue for a test case and link it. /// file a GitHub issue for a test case and link it.
@@ -102,17 +102,15 @@ public class TestCaseIssuesController(
/// only: /// only:
/// ///
/// <list type="bullet"> /// <list type="bullet">
/// <item>Issue CLOSED and the case is still Failed → back to NotYetTested. /// <item>Issue CLOSED and the case is Failed → Passed. A closed issue means
/// The bug is fixed, so the case is due for another run. It is deliberately /// the bug is fixed, and the case that was failing on it therefore passes.</item>
/// NOT marked Passed: closing an issue is a developer's claim that the fix /// <item>Issue REOPENED and the case is Passed → Failed. The bug came back,
/// landed, not a tester's observation that the case passes.</item> /// so the close transition above is undone.</item>
/// <item>Issue REOPENED and the case is NotYetTested → back to Failed.
/// The bug came back, so the close transition above is undone.</item>
/// </list> /// </list>
/// ///
/// Nothing else moves. Only these two states are the reconciler's to /// Nothing else moves. Only these two states are the reconciler's to
/// change, so a case a tester has Claimed — or marked Passed after /// change, so a case a tester has Claimed, or one nobody has run yet, is
/// verifying the fix — is never dragged back underneath them.</summary> /// never rewritten underneath them.</summary>
[HttpPost("api/testcasemanagement/v1/issues/sync")] [HttpPost("api/testcasemanagement/v1/issues/sync")]
public async Task<IActionResult> Sync(CancellationToken ct) public async Task<IActionResult> Sync(CancellationToken ct)
{ {
@@ -144,15 +142,15 @@ public class TestCaseIssuesController(
if (issue is null) continue; if (issue is null) continue;
var closed = issue.State.Equals("closed", StringComparison.OrdinalIgnoreCase); var closed = issue.State.Equals("closed", StringComparison.OrdinalIgnoreCase);
// Only ever move a case between the two states this reconciler // Closed means fixed, so a case failing on that bug now passes.
// itself owns. Reopening undoes the close transition and nothing // Only ever move between those two states: reopening undoes the
// else: "open issue and the case isn't Failed" would also catch a // close and nothing else. "Open issue and the case isn't Failed"
// case a tester has just Claimed, or one they marked Passed after // would also catch a case a tester has just Claimed, or one nobody
// verifying the fix, and drag it back to Failed underneath them. // has run yet, and mark it Failed underneath them.
int? next = (closed, tc.Status) switch int? next = (closed, tc.Status) switch
{ {
(true, StatusFailed) => StatusNotYetTested, (true, StatusFailed) => StatusPassed,
(false, StatusNotYetTested) => StatusFailed, (false, StatusPassed) => StatusFailed,
_ => null, _ => null,
}; };
if (next is not int status) continue; if (next is not int status) continue;
+7 -8
View File
@@ -222,15 +222,14 @@ issue number is only read from a URL matching the GitHub issue shape.
| Issue | Case status | Result | | Issue | Case status | Result |
| --- | --- | --- | | --- | --- | --- |
| Closed | Failed | → NotYetTested | | Closed | Failed | → Passed |
| Open | NotYetTested | → Failed | | Open | Passed | → Failed |
| anything | Claimed / Passed | untouched | | anything | Claimed / NotYetTested | untouched |
A closed issue does **not** mark the case Passed: closing is a developer's claim Closed means fixed: the case that was failing on that bug now passes, and
that the fix landed, not a tester's observation that the case passes, so it goes reopening the issue undoes it. Only those two states are the reconciler's to
back in the queue for another run. And only those two states are the move — a case a tester has Claimed, or one nobody has run yet, is never
reconciler's to move — a case a tester has Claimed, or marked Passed after rewritten underneath them.
verifying the fix, is never dragged back underneath them.
## Build And Deploy ## Build And Deploy