mirror of
https://github.com/DorkSquadRR/DorkNet.git
synced 2026-10-26 07:28:45 -07:00
feat: link QA test cases to GitHub issues
File a GitHub issue from a failing test case, and let the case follow the
issue afterwards.
POST api/testcasemanagement/v1/testcase/{id}/issue file + link
DELETE api/testcasemanagement/v1/testcase/{id}/issue unlink
POST api/testcasemanagement/v1/issues/sync reconcile now
plus TestCaseIssueReconciler, a background sweep on the same code path as the
manual endpoint so the two cannot drift. Admin-gated: these are not part of the
2023 client's surface, unlike the existing test-case routes which are open
because the watch gates that tab client-side.
The link lives in TestCaseEntity.JiraBugUrl — the field Rec Room's own tooling
used for the bug filed against a failing case. That avoids a migration, which
matters more here than the name: the Postgres path is EnsureCreated-only and
never replays migrations, so a new column has to be added twice or it is missing
in production. A leftover genuine Jira link is ignored rather than misparsed,
since the issue number is only read from a GitHub-shaped URL.
Filing is idempotent — a case already carrying a live issue returns it instead
of filing again, so sweeping a failing pass repeatedly does not spam duplicates.
Unlinking leaves the issue alone; closing someone's issue as a side effect of
tidying a QA link would be the wrong call.
Reconcile moves a case only between the two states it owns: closed+Failed goes
to NotYetTested, open+NotYetTested goes back to Failed. A closed issue is NOT
marked Passed — closing is a developer's claim that the fix landed, not a
tester's observation that the case passes, so the case goes back in the queue.
The first draft flipped any non-Failed case to Failed on an open issue, which
contradicted its own doc comment and yanked a case out from under a tester
mid-claim; the test for that caught it.
Everything degrades rather than fails when unconfigured: IsConfigured is false
without a token, endpoints answer 503 with the reason, and the reconciler logs
once and idles. GitHub is faked in the tests, which cover idempotency, both
state transitions, the mid-claim case, and the unconfigured path.
This commit is contained in:
Alexa
@@ -0,0 +1,250 @@
|
||||
using System.Net;
|
||||
using System.Net.Http.Headers;
|
||||
using System.Text.Json;
|
||||
using DorkNet.Server.Data;
|
||||
using DorkNet.Server.Data.Entities;
|
||||
using DorkNet.Server.Services;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.DependencyInjection.Extensions;
|
||||
using Microsoft.AspNetCore.Mvc.Testing;
|
||||
|
||||
namespace DorkNet.Server.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// QA test cases linked to GitHub issues.
|
||||
///
|
||||
/// The link lives in <see cref="TestCaseEntity.JiraBugUrl"/> — the field Rec
|
||||
/// Room's own tooling used for the bug filed against a failing case — so no
|
||||
/// column is added and the Postgres "migrations never replay" trap is avoided
|
||||
/// entirely.
|
||||
///
|
||||
/// GitHub itself is faked. These tests are about our behaviour around it:
|
||||
/// filing once rather than per sweep, following issue state in the one
|
||||
/// direction that is safe, and degrading rather than 500ing when no token is
|
||||
/// configured.
|
||||
/// </summary>
|
||||
public sealed class TestCaseGitHubIssueTests : IClassFixture<DorkNetServerFactory>
|
||||
{
|
||||
private readonly DorkNetServerFactory _factory;
|
||||
|
||||
public TestCaseGitHubIssueTests(DorkNetServerFactory factory) => _factory = factory;
|
||||
|
||||
private const int StatusNotYetTested = 0;
|
||||
private const int StatusClaimed = 1;
|
||||
private const int StatusFailed = 2;
|
||||
|
||||
[Fact]
|
||||
public async Task Filing_is_idempotent_and_stores_the_issue_url()
|
||||
{
|
||||
var github = new FakeGitHub();
|
||||
using var factory = WithGitHub(github);
|
||||
using var client = await AdminClientAsync(factory);
|
||||
var caseId = await SeedCaseAsync(factory, StatusFailed);
|
||||
|
||||
using var first = await client.PostAsync($"/api/testcasemanagement/v1/testcase/{caseId}/issue", null);
|
||||
var firstBody = await first.Content.ReadAsStringAsync();
|
||||
Assert.True(first.IsSuccessStatusCode, $"first file -> {(int)first.StatusCode}: {firstBody}");
|
||||
Assert.True(JsonDocument.Parse(firstBody).RootElement.GetProperty("created").GetBoolean());
|
||||
|
||||
// The link must be persisted, and in the field the admin UI reads.
|
||||
await using (var scope = factory.Services.CreateAsyncScope())
|
||||
{
|
||||
var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();
|
||||
var tc = await db.TestCases.SingleAsync(c => c.Id == caseId);
|
||||
Assert.Contains("/issues/", tc.JiraBugUrl);
|
||||
}
|
||||
|
||||
// Re-running a sweep over a failing pass must not file the bug again.
|
||||
// Measured as a DELTA rather than an absolute count: the fake is shared
|
||||
// with whatever else the server does during the test, and the property
|
||||
// under test is "this call filed nothing", not "nothing was ever filed".
|
||||
var filedBefore = github.Created;
|
||||
using var second = await client.PostAsync($"/api/testcasemanagement/v1/testcase/{caseId}/issue", null);
|
||||
var secondBody = await second.Content.ReadAsStringAsync();
|
||||
Assert.True(second.IsSuccessStatusCode, $"second file -> {(int)second.StatusCode}: {secondBody}");
|
||||
Assert.False(JsonDocument.Parse(secondBody).RootElement.GetProperty("created").GetBoolean(),
|
||||
"filing twice created a duplicate issue");
|
||||
Assert.Equal(filedBefore, github.Created);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task A_closed_issue_sends_the_case_back_for_retesting_not_to_passed()
|
||||
{
|
||||
var github = new FakeGitHub();
|
||||
using var factory = WithGitHub(github);
|
||||
using var client = await AdminClientAsync(factory);
|
||||
var caseId = await SeedCaseAsync(factory, StatusFailed);
|
||||
|
||||
using var filed = await client.PostAsync($"/api/testcasemanagement/v1/testcase/{caseId}/issue", null);
|
||||
Assert.True(filed.IsSuccessStatusCode);
|
||||
|
||||
github.CloseAll();
|
||||
using var sync = await client.PostAsync("/api/testcasemanagement/v1/issues/sync", null);
|
||||
Assert.True(sync.IsSuccessStatusCode, $"sync -> {(int)sync.StatusCode}");
|
||||
|
||||
await using var scope = factory.Services.CreateAsyncScope();
|
||||
var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();
|
||||
var tc = await db.TestCases.SingleAsync(c => c.Id == caseId);
|
||||
|
||||
// Closing an issue is a developer saying the fix landed — not a tester
|
||||
// saying the case passes. It goes back in the queue, not to Passed.
|
||||
Assert.Equal(StatusNotYetTested, tc.Status);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task A_reopened_issue_marks_the_case_failed_again()
|
||||
{
|
||||
var github = new FakeGitHub();
|
||||
using var factory = WithGitHub(github);
|
||||
using var client = await AdminClientAsync(factory);
|
||||
var caseId = await SeedCaseAsync(factory, StatusFailed);
|
||||
|
||||
using var filed = await client.PostAsync($"/api/testcasemanagement/v1/testcase/{caseId}/issue", null);
|
||||
Assert.True(filed.IsSuccessStatusCode);
|
||||
|
||||
github.CloseAll();
|
||||
using var closeSync = await client.PostAsync("/api/testcasemanagement/v1/issues/sync", null);
|
||||
Assert.True(closeSync.IsSuccessStatusCode);
|
||||
|
||||
github.ReopenAll();
|
||||
using var reopenSync = await client.PostAsync("/api/testcasemanagement/v1/issues/sync", null);
|
||||
Assert.True(reopenSync.IsSuccessStatusCode);
|
||||
|
||||
await using var scope = factory.Services.CreateAsyncScope();
|
||||
var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();
|
||||
Assert.Equal(StatusFailed, (await db.TestCases.SingleAsync(c => c.Id == caseId)).Status);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task A_sweep_does_not_stomp_a_tester_mid_claim()
|
||||
{
|
||||
var github = new FakeGitHub();
|
||||
using var factory = WithGitHub(github);
|
||||
using var client = await AdminClientAsync(factory);
|
||||
var caseId = await SeedCaseAsync(factory, StatusFailed);
|
||||
|
||||
using var filed = await client.PostAsync($"/api/testcasemanagement/v1/testcase/{caseId}/issue", null);
|
||||
Assert.True(filed.IsSuccessStatusCode);
|
||||
|
||||
// Someone picks the case up while the issue is still open.
|
||||
await using (var scope = factory.Services.CreateAsyncScope())
|
||||
{
|
||||
var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();
|
||||
(await db.TestCases.SingleAsync(c => c.Id == caseId)).Status = StatusClaimed;
|
||||
await db.SaveChangesAsync();
|
||||
}
|
||||
|
||||
using var sync = await client.PostAsync("/api/testcasemanagement/v1/issues/sync", null);
|
||||
Assert.True(sync.IsSuccessStatusCode);
|
||||
|
||||
await using var check = factory.Services.CreateAsyncScope();
|
||||
var db2 = check.ServiceProvider.GetRequiredService<DorkNetDbContext>();
|
||||
var tc = await db2.TestCases.SingleAsync(c => c.Id == caseId);
|
||||
|
||||
// An open issue against a claimed case says nothing new; flipping it to
|
||||
// Failed would yank it out from under whoever is testing it.
|
||||
Assert.Equal(StatusClaimed, tc.Status);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Without_a_token_the_endpoint_says_so_instead_of_failing()
|
||||
{
|
||||
// The default fixture has no GitHub configuration, which is what an
|
||||
// ordinary deployment looks like.
|
||||
using var client = await AdminClientAsync(_factory);
|
||||
var caseId = await SeedCaseAsync(_factory, StatusFailed);
|
||||
|
||||
using var response = await client.PostAsync(
|
||||
$"/api/testcasemanagement/v1/testcase/{caseId}/issue", null);
|
||||
|
||||
Assert.Equal(HttpStatusCode.ServiceUnavailable, response.StatusCode);
|
||||
}
|
||||
|
||||
private WebApplicationFactory<Program> WithGitHub(IGitHubIssues github) =>
|
||||
_factory.WithWebHostBuilder(builder =>
|
||||
builder.ConfigureServices(services =>
|
||||
{
|
||||
services.RemoveAll<IGitHubIssues>();
|
||||
services.AddSingleton(github);
|
||||
}));
|
||||
|
||||
private async Task<HttpClient> AdminClientAsync(WebApplicationFactory<Program> factory)
|
||||
{
|
||||
var client = factory.CreateClient(new() { AllowAutoRedirect = false });
|
||||
client.BaseAddress = new Uri($"http://api.{_factory.ApexDomain}");
|
||||
var session = await GameClientSessionFactory.CreateAsync(client, _factory.ApexDomain);
|
||||
|
||||
await using (var scope = factory.Services.CreateAsyncScope())
|
||||
{
|
||||
var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();
|
||||
(await db.Players.FirstAsync(p => p.Id == session.PlayerId)).IsAdmin = true;
|
||||
await db.SaveChangesAsync();
|
||||
}
|
||||
|
||||
client.DefaultRequestHeaders.Authorization =
|
||||
new AuthenticationHeaderValue("Bearer", session.AccessToken);
|
||||
return client;
|
||||
}
|
||||
|
||||
private static async Task<string> SeedCaseAsync(WebApplicationFactory<Program> factory, int status)
|
||||
{
|
||||
var id = $"tc-{Guid.NewGuid():N}";
|
||||
await using var scope = factory.Services.CreateAsyncScope();
|
||||
var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();
|
||||
db.TestCases.Add(new TestCaseEntity
|
||||
{
|
||||
Id = id,
|
||||
Key = $"QA-{Random.Shared.Next(1000, 9999)}",
|
||||
Title = "Dorm fails to load after cloning a room",
|
||||
Description = "Clone RecCenter, then return to the dorm.",
|
||||
RoomName = "Dorm",
|
||||
Status = status,
|
||||
TagsJson = """["dorm","regression"]""",
|
||||
CommentsJson = """["1811750|2026-07-28T08:00:00Z|Reproduced twice"]""",
|
||||
});
|
||||
await db.SaveChangesAsync();
|
||||
return id;
|
||||
}
|
||||
|
||||
/// <summary>An in-memory GitHub. Records what was filed so the tests can
|
||||
/// assert we did not file twice, and lets them flip issue state.</summary>
|
||||
private sealed class FakeGitHub : IGitHubIssues
|
||||
{
|
||||
private readonly Dictionary<int, GitHubIssue> _issues = [];
|
||||
private int _next = 1;
|
||||
|
||||
public bool IsConfigured => true;
|
||||
|
||||
public string Repository => "DorkSquadRR/DorkNet";
|
||||
|
||||
public int Created { get; private set; }
|
||||
|
||||
public Task<GitHubIssue?> CreateAsync(
|
||||
string title, string body, IEnumerable<string> labels, CancellationToken ct = default)
|
||||
{
|
||||
var number = _next++;
|
||||
var issue = new GitHubIssue(
|
||||
number, $"https://github.com/{Repository}/issues/{number}", "open", title);
|
||||
_issues[number] = issue;
|
||||
Created++;
|
||||
return Task.FromResult<GitHubIssue?>(issue);
|
||||
}
|
||||
|
||||
public Task<GitHubIssue?> GetAsync(int number, CancellationToken ct = default) =>
|
||||
Task.FromResult(_issues.TryGetValue(number, out var issue) ? issue : null);
|
||||
|
||||
public Task<bool> CommentAsync(int number, string body, CancellationToken ct = default) =>
|
||||
Task.FromResult(_issues.ContainsKey(number));
|
||||
|
||||
public void CloseAll() => SetState("closed");
|
||||
|
||||
public void ReopenAll() => SetState("open");
|
||||
|
||||
private void SetState(string state)
|
||||
{
|
||||
foreach (var number in _issues.Keys.ToList())
|
||||
_issues[number] = _issues[number] with { State = state };
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,245 @@
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using DorkNet.Server.Auth;
|
||||
using DorkNet.Server.Data;
|
||||
using DorkNet.Server.Data.Entities;
|
||||
using DorkNet.Server.Services;
|
||||
|
||||
namespace DorkNet.Server.Controllers.API.TestCaseManagement;
|
||||
|
||||
/// <summary>
|
||||
/// Links QA test cases to GitHub issues.
|
||||
///
|
||||
/// <para>These routes are NOT part of the 2023 client's surface — the in-game
|
||||
/// QA tab never calls them. They exist for the admin tooling, so they are
|
||||
/// admin-gated, unlike the client-facing routes in
|
||||
/// <see cref="TestCaseManagementController"/> which are deliberately open
|
||||
/// (the watch gates that tab client-side on the developer role).</para>
|
||||
///
|
||||
/// <para>The issue URL is stored in <see cref="TestCaseEntity.JiraBugUrl"/> —
|
||||
/// the field Rec Room's own tooling used for exactly this, a link to the bug
|
||||
/// filed against a failing case. Reusing it means no schema change, and on
|
||||
/// this server that matters more than the name: the Postgres path is
|
||||
/// EnsureCreated-only and never replays migrations, so every new column has to
|
||||
/// be added twice or it is missing in production. A field that already exists
|
||||
/// everywhere has neither problem, and the admin UI already renders it.</para>
|
||||
/// </summary>
|
||||
[ApiController]
|
||||
[AdminOnly]
|
||||
public class TestCaseIssuesController(
|
||||
DorkNetDbContext db,
|
||||
IGitHubIssues github,
|
||||
ILogger<TestCaseIssuesController> log) : ControllerBase
|
||||
{
|
||||
/// <summary>Test-case status values, matching the client's
|
||||
/// <c>TestCaseStatus</c> enum.</summary>
|
||||
private const int StatusNotYetTested = 0;
|
||||
private const int StatusFailed = 2;
|
||||
|
||||
/// <summary>POST <c>api/testcasemanagement/v1/testcase/{id}/issue</c> —
|
||||
/// file a GitHub issue for a test case and link it.
|
||||
///
|
||||
/// Idempotent: a case that already carries a live issue link returns that
|
||||
/// issue rather than filing a duplicate. Re-running a sweep over a failing
|
||||
/// pass is therefore safe, which is the whole point — the alternative is a
|
||||
/// tester filing the same bug on every run.</summary>
|
||||
[HttpPost("api/testcasemanagement/v1/testcase/{id}/issue")]
|
||||
public async Task<IActionResult> CreateIssue(string id, CancellationToken ct)
|
||||
{
|
||||
if (!github.IsConfigured)
|
||||
return Problem(
|
||||
"GitHub issue linking is not configured. Set GitHub:Token and GitHub:Repository.",
|
||||
statusCode: StatusCodes.Status503ServiceUnavailable);
|
||||
|
||||
var tc = await db.TestCases.FirstOrDefaultAsync(c => c.Id == id, ct);
|
||||
if (tc is null) return NotFound();
|
||||
|
||||
if (GitHubIssueService.IssueNumberFromUrl(tc.JiraBugUrl) is int existingNumber)
|
||||
{
|
||||
var existing = await github.GetAsync(existingNumber, ct);
|
||||
// A link to an issue that no longer exists (deleted, or the repo
|
||||
// changed) must not block filing a new one.
|
||||
if (existing is not null)
|
||||
return Ok(Describe(tc, existing, created: false));
|
||||
}
|
||||
|
||||
var issue = await github.CreateAsync(TitleFor(tc), BodyFor(tc), LabelsFor(tc), ct);
|
||||
if (issue is null)
|
||||
return Problem("GitHub rejected the issue.", statusCode: StatusCodes.Status502BadGateway);
|
||||
|
||||
tc.JiraBugUrl = issue.Url;
|
||||
tc.UpdatedAt = DateTime.UtcNow;
|
||||
await db.SaveChangesAsync(ct);
|
||||
|
||||
log.LogInformation(
|
||||
"[testcase-issue] filed {Repo}#{Number} for case {Key} ({Id})",
|
||||
github.Repository, issue.Number, tc.Key, tc.Id);
|
||||
return Ok(Describe(tc, issue, created: true));
|
||||
}
|
||||
|
||||
/// <summary>DELETE <c>api/testcasemanagement/v1/testcase/{id}/issue</c> —
|
||||
/// unlink, leaving the issue itself alone. Closing someone's issue as a
|
||||
/// side effect of tidying a QA link would be the wrong call.</summary>
|
||||
[HttpDelete("api/testcasemanagement/v1/testcase/{id}/issue")]
|
||||
public async Task<IActionResult> UnlinkIssue(string id, CancellationToken ct)
|
||||
{
|
||||
var tc = await db.TestCases.FirstOrDefaultAsync(c => c.Id == id, ct);
|
||||
if (tc is null) return NotFound();
|
||||
|
||||
var previous = tc.JiraBugUrl;
|
||||
tc.JiraBugUrl = string.Empty;
|
||||
tc.UpdatedAt = DateTime.UtcNow;
|
||||
await db.SaveChangesAsync(ct);
|
||||
|
||||
log.LogInformation("[testcase-issue] unlinked {Url} from case {Key}", previous, tc.Key);
|
||||
return Ok(new { tc.Id, tc.Key, unlinked = previous });
|
||||
}
|
||||
|
||||
/// <summary>POST <c>api/testcasemanagement/v1/issues/sync</c> — reconcile
|
||||
/// every linked case against its issue.
|
||||
///
|
||||
/// The state a test case should be in follows the issue, in one direction
|
||||
/// only:
|
||||
///
|
||||
/// <list type="bullet">
|
||||
/// <item>Issue CLOSED and the case is still Failed → back to NotYetTested.
|
||||
/// The bug is fixed, so the case is due for another run. It is deliberately
|
||||
/// NOT marked Passed: closing an issue is a developer's claim that the fix
|
||||
/// landed, not a tester's observation that the case passes.</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>
|
||||
///
|
||||
/// Nothing else moves. Only these two states are the reconciler's to
|
||||
/// change, so a case a tester has Claimed — or marked Passed after
|
||||
/// verifying the fix — is never dragged back underneath them.</summary>
|
||||
[HttpPost("api/testcasemanagement/v1/issues/sync")]
|
||||
public async Task<IActionResult> Sync(CancellationToken ct)
|
||||
{
|
||||
if (!github.IsConfigured)
|
||||
return Problem(
|
||||
"GitHub issue linking is not configured. Set GitHub:Token and GitHub:Repository.",
|
||||
statusCode: StatusCodes.Status503ServiceUnavailable);
|
||||
|
||||
var changed = await ReconcileAsync(db, github, log, ct);
|
||||
return Ok(new { reconciled = changed.Count, changes = changed });
|
||||
}
|
||||
|
||||
/// <summary>Shared by the endpoint and the background reconciler so the two
|
||||
/// can never drift apart.</summary>
|
||||
internal static async Task<List<object>> ReconcileAsync(
|
||||
DorkNetDbContext db, IGitHubIssues github, ILogger log, CancellationToken ct)
|
||||
{
|
||||
var linked = await db.TestCases
|
||||
.Where(c => c.JiraBugUrl != string.Empty)
|
||||
.ToListAsync(ct);
|
||||
|
||||
var changes = new List<object>();
|
||||
foreach (var tc in linked)
|
||||
{
|
||||
if (ct.IsCancellationRequested) break;
|
||||
if (GitHubIssueService.IssueNumberFromUrl(tc.JiraBugUrl) is not int number) continue;
|
||||
|
||||
var issue = await github.GetAsync(number, ct);
|
||||
if (issue is null) continue;
|
||||
|
||||
var closed = issue.State.Equals("closed", StringComparison.OrdinalIgnoreCase);
|
||||
// Only ever move a case between the two states this reconciler
|
||||
// itself owns. Reopening undoes the close transition and nothing
|
||||
// else: "open issue and the case isn't Failed" would also catch a
|
||||
// case a tester has just Claimed, or one they marked Passed after
|
||||
// verifying the fix, and drag it back to Failed underneath them.
|
||||
int? next = (closed, tc.Status) switch
|
||||
{
|
||||
(true, StatusFailed) => StatusNotYetTested,
|
||||
(false, StatusNotYetTested) => StatusFailed,
|
||||
_ => null,
|
||||
};
|
||||
if (next is not int status) continue;
|
||||
|
||||
var from = tc.Status;
|
||||
tc.Status = status;
|
||||
tc.UpdatedAt = DateTime.UtcNow;
|
||||
changes.Add(new { tc.Id, tc.Key, issue = number, from, to = status });
|
||||
log.LogInformation(
|
||||
"[testcase-issue] case {Key} status {From} -> {To} (issue #{Number} is {State})",
|
||||
tc.Key, from, status, number, issue.State);
|
||||
}
|
||||
|
||||
if (changes.Count > 0) await db.SaveChangesAsync(ct);
|
||||
return changes;
|
||||
}
|
||||
|
||||
private object Describe(TestCaseEntity tc, GitHubIssue issue, bool created) => new
|
||||
{
|
||||
tc.Id,
|
||||
tc.Key,
|
||||
tc.Title,
|
||||
tc.Status,
|
||||
issue = new { issue.Number, issue.Url, issue.State },
|
||||
created,
|
||||
};
|
||||
|
||||
private static string TitleFor(TestCaseEntity tc) =>
|
||||
string.IsNullOrWhiteSpace(tc.Key) ? tc.Title : $"[{tc.Key}] {tc.Title}";
|
||||
|
||||
/// <summary>The issue body. Written for whoever picks the bug up, so it
|
||||
/// leads with what to do rather than with metadata.</summary>
|
||||
private static string BodyFor(TestCaseEntity tc)
|
||||
{
|
||||
var lines = new List<string>();
|
||||
if (!string.IsNullOrWhiteSpace(tc.Description))
|
||||
{
|
||||
lines.Add(tc.Description.Trim());
|
||||
lines.Add("");
|
||||
}
|
||||
|
||||
lines.Add("| | |");
|
||||
lines.Add("| --- | --- |");
|
||||
lines.Add($"| Test case | `{tc.Key}` |");
|
||||
if (!string.IsNullOrWhiteSpace(tc.RoomName)) lines.Add($"| Room | {tc.RoomName} |");
|
||||
if (tc.TestPassId is uint pass) lines.Add($"| Test pass | {pass} |");
|
||||
if (!string.IsNullOrWhiteSpace(tc.JiraUrl)) lines.Add($"| Test case link | {tc.JiraUrl} |");
|
||||
|
||||
var comments = ParseComments(tc.CommentsJson);
|
||||
if (comments.Count > 0)
|
||||
{
|
||||
lines.Add("");
|
||||
lines.Add("### Tester notes");
|
||||
foreach (var comment in comments) lines.Add($"- {comment}");
|
||||
}
|
||||
|
||||
lines.Add("");
|
||||
lines.Add("<sub>Filed from DorkNet QA test-case management.</sub>");
|
||||
return string.Join("\n", lines);
|
||||
}
|
||||
|
||||
private static IEnumerable<string> LabelsFor(TestCaseEntity tc)
|
||||
{
|
||||
yield return "qa";
|
||||
foreach (var tag in ParseComments(tc.TagsJson)) yield return tag;
|
||||
}
|
||||
|
||||
/// <summary>Both CommentsJson and TagsJson are JSON string arrays. A
|
||||
/// comment is stored as <c>playerId|timestamp|text</c>; only the text is
|
||||
/// worth putting in an issue.</summary>
|
||||
private static List<string> ParseComments(string json)
|
||||
{
|
||||
try
|
||||
{
|
||||
var items = System.Text.Json.JsonSerializer.Deserialize<List<string>>(json) ?? [];
|
||||
return items
|
||||
.Select(item =>
|
||||
{
|
||||
var parts = item.Split('|', 3);
|
||||
return (parts.Length == 3 ? parts[2] : item).Trim();
|
||||
})
|
||||
.Where(text => text.Length > 0)
|
||||
.ToList();
|
||||
}
|
||||
catch (System.Text.Json.JsonException)
|
||||
{
|
||||
return [];
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,182 @@
|
||||
using System.Net;
|
||||
using System.Net.Http.Headers;
|
||||
using System.Text;
|
||||
using System.Text.Json;
|
||||
using System.Text.RegularExpressions;
|
||||
|
||||
namespace DorkNet.Server.Services;
|
||||
|
||||
/// <summary>One GitHub issue, reduced to what QA linking needs.</summary>
|
||||
public sealed record GitHubIssue(int Number, string Url, string State, string Title);
|
||||
|
||||
/// <summary>
|
||||
/// Files and reads GitHub issues for the QA test-case tooling.
|
||||
///
|
||||
/// Deliberately narrow: create, read state, comment. Everything else a QA
|
||||
/// workflow might want (labels beyond the fixed set, milestones, assignees) is
|
||||
/// out of scope until something asks for it.
|
||||
/// </summary>
|
||||
public interface IGitHubIssues
|
||||
{
|
||||
/// <summary>False when no token/repo is configured. Callers must check this
|
||||
/// and degrade rather than throw — a server without GitHub credentials is a
|
||||
/// normal deployment, not a broken one.</summary>
|
||||
bool IsConfigured { get; }
|
||||
|
||||
/// <summary>owner/name, or empty when unconfigured.</summary>
|
||||
string Repository { get; }
|
||||
|
||||
Task<GitHubIssue?> CreateAsync(string title, string body, IEnumerable<string> labels, CancellationToken ct = default);
|
||||
|
||||
Task<GitHubIssue?> GetAsync(int number, CancellationToken ct = default);
|
||||
|
||||
Task<bool> CommentAsync(int number, string body, CancellationToken ct = default);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// REST implementation over api.github.com.
|
||||
///
|
||||
/// Configured with <c>GitHub:Token</c> (a PAT with <c>issues:write</c> on the
|
||||
/// target repo) and <c>GitHub:Repository</c> (<c>owner/name</c>). Both are
|
||||
/// read once at construction; absent either, <see cref="IsConfigured"/> is
|
||||
/// false and every call returns null rather than throwing, so QA endpoints
|
||||
/// answer "not configured" instead of 500ing.
|
||||
/// </summary>
|
||||
public sealed class GitHubIssueService : IGitHubIssues
|
||||
{
|
||||
private const string ApiRoot = "https://api.github.com";
|
||||
|
||||
private readonly IHttpClientFactory _httpClientFactory;
|
||||
private readonly ILogger<GitHubIssueService> _log;
|
||||
private readonly string _token;
|
||||
|
||||
public GitHubIssueService(
|
||||
IConfiguration config,
|
||||
IHttpClientFactory httpClientFactory,
|
||||
ILogger<GitHubIssueService> log)
|
||||
{
|
||||
_httpClientFactory = httpClientFactory;
|
||||
_log = log;
|
||||
_token = config["GitHub:Token"] ?? string.Empty;
|
||||
Repository = (config["GitHub:Repository"] ?? string.Empty).Trim().Trim('/');
|
||||
|
||||
if (!IsConfigured)
|
||||
{
|
||||
_log.LogInformation(
|
||||
"[github] no token/repository configured; issue linking is disabled");
|
||||
}
|
||||
else
|
||||
{
|
||||
_log.LogInformation("[github] issue linking enabled for {Repository}", Repository);
|
||||
}
|
||||
}
|
||||
|
||||
public bool IsConfigured => _token.Length > 0 && Repository.Contains('/');
|
||||
|
||||
public string Repository { get; }
|
||||
|
||||
public async Task<GitHubIssue?> CreateAsync(
|
||||
string title, string body, IEnumerable<string> labels, CancellationToken ct = default)
|
||||
{
|
||||
if (!IsConfigured) return null;
|
||||
|
||||
var payload = JsonSerializer.Serialize(new
|
||||
{
|
||||
title,
|
||||
body,
|
||||
labels = labels.Where(l => !string.IsNullOrWhiteSpace(l)).Distinct().ToArray(),
|
||||
});
|
||||
|
||||
return await SendAsync(HttpMethod.Post, $"/repos/{Repository}/issues", payload, ct);
|
||||
}
|
||||
|
||||
public Task<GitHubIssue?> GetAsync(int number, CancellationToken ct = default) =>
|
||||
IsConfigured
|
||||
? SendAsync(HttpMethod.Get, $"/repos/{Repository}/issues/{number}", null, ct)
|
||||
: Task.FromResult<GitHubIssue?>(null);
|
||||
|
||||
public async Task<bool> CommentAsync(int number, string body, CancellationToken ct = default)
|
||||
{
|
||||
if (!IsConfigured) return false;
|
||||
|
||||
var payload = JsonSerializer.Serialize(new { body });
|
||||
using var response = await SendRawAsync(
|
||||
HttpMethod.Post, $"/repos/{Repository}/issues/{number}/comments", payload, ct);
|
||||
return response is { IsSuccessStatusCode: true };
|
||||
}
|
||||
|
||||
private async Task<GitHubIssue?> SendAsync(
|
||||
HttpMethod method, string path, string? payload, CancellationToken ct)
|
||||
{
|
||||
using var response = await SendRawAsync(method, path, payload, ct);
|
||||
if (response is null || !response.IsSuccessStatusCode) return null;
|
||||
|
||||
try
|
||||
{
|
||||
var json = await response.Content.ReadAsStringAsync(ct);
|
||||
using var doc = JsonDocument.Parse(json);
|
||||
var root = doc.RootElement;
|
||||
return new GitHubIssue(
|
||||
root.GetProperty("number").GetInt32(),
|
||||
root.GetProperty("html_url").GetString() ?? string.Empty,
|
||||
root.GetProperty("state").GetString() ?? "open",
|
||||
root.TryGetProperty("title", out var t) ? t.GetString() ?? string.Empty : string.Empty);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_log.LogWarning(ex, "[github] could not read the issue response from {Method} {Path}", method, path);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
private async Task<HttpResponseMessage?> SendRawAsync(
|
||||
HttpMethod method, string path, string? payload, CancellationToken ct)
|
||||
{
|
||||
try
|
||||
{
|
||||
var client = _httpClientFactory.CreateClient();
|
||||
using var request = new HttpRequestMessage(method, ApiRoot + path);
|
||||
request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", _token);
|
||||
request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/vnd.github+json"));
|
||||
// GitHub rejects requests without one.
|
||||
request.Headers.UserAgent.ParseAdd("DorkNet-QA");
|
||||
request.Headers.Add("X-GitHub-Api-Version", "2022-11-28");
|
||||
if (payload is not null)
|
||||
request.Content = new StringContent(payload, Encoding.UTF8, "application/json");
|
||||
|
||||
var response = await client.SendAsync(request, ct);
|
||||
if (!response.IsSuccessStatusCode)
|
||||
{
|
||||
// A 404 on a GET is ordinary (deleted issue); anything else is
|
||||
// worth seeing, but never fatal to the caller.
|
||||
var level = method == HttpMethod.Get && response.StatusCode == HttpStatusCode.NotFound
|
||||
? LogLevel.Debug
|
||||
: LogLevel.Warning;
|
||||
_log.Log(level, "[github] {Method} {Path} -> {Status}", method, path, (int)response.StatusCode);
|
||||
}
|
||||
return response;
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_log.LogWarning(ex, "[github] {Method} {Path} failed", method, path);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Pull the issue number out of a stored issue URL.
|
||||
///
|
||||
/// The link lives in <c>TestCaseEntity.JiraBugUrl</c> — the field Rec
|
||||
/// Room's own QA tooling used for the same purpose — so the only thing
|
||||
/// identifying it as a GitHub issue is the URL shape. Returns null for
|
||||
/// anything that isn't one, which is how a genuine Jira link left over
|
||||
/// from the original data is ignored rather than misparsed.</summary>
|
||||
public static int? IssueNumberFromUrl(string? url)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(url)) return null;
|
||||
var match = Regex.Match(url, @"github\.com/[^/]+/[^/]+/issues/(?<number>\d+)",
|
||||
RegexOptions.IgnoreCase);
|
||||
return match.Success && int.TryParse(match.Groups["number"].Value, out var number)
|
||||
? number
|
||||
: null;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,89 @@
|
||||
using Microsoft.Extensions.Options;
|
||||
using DorkNet.Server.Controllers.API.TestCaseManagement;
|
||||
using DorkNet.Server.Data;
|
||||
|
||||
namespace DorkNet.Server.Services;
|
||||
|
||||
/// <summary>
|
||||
/// Keeps linked test cases in step with their GitHub issues without anyone
|
||||
/// having to press anything.
|
||||
///
|
||||
/// <para>A closed issue means the bug is fixed, so its test case is due for
|
||||
/// another run; a reopened one means it came back. Waiting for a human to
|
||||
/// notice that defeats the point of linking them, so this sweeps on an
|
||||
/// interval. The reconcile logic itself lives in
|
||||
/// <see cref="TestCaseIssuesController.ReconcileAsync"/> and is shared with the
|
||||
/// manual endpoint, so the automatic and manual paths cannot drift.</para>
|
||||
///
|
||||
/// <para>Does nothing at all when GitHub is unconfigured — a server without a
|
||||
/// token is a normal deployment, not a broken one, and it should not log a
|
||||
/// failure every interval forever.</para>
|
||||
/// </summary>
|
||||
public sealed class TestCaseIssueReconciler(
|
||||
IServiceScopeFactory scopeFactory,
|
||||
IGitHubIssues github,
|
||||
IConfiguration config,
|
||||
ILogger<TestCaseIssueReconciler> log) : BackgroundService
|
||||
{
|
||||
/// <summary>How often to sweep. Issue state changes are not urgent — a QA
|
||||
/// pass is a human-scale activity — and each sweep costs one API call per
|
||||
/// linked case against a rate limit shared with everything else using the
|
||||
/// token, so this is deliberately slow. Override with
|
||||
/// <c>GitHub:ReconcileIntervalMinutes</c>; 0 or less disables the sweep
|
||||
/// while leaving the manual endpoint working.</summary>
|
||||
private const int DefaultIntervalMinutes = 15;
|
||||
|
||||
protected override async Task ExecuteAsync(CancellationToken stoppingToken)
|
||||
{
|
||||
if (!github.IsConfigured)
|
||||
{
|
||||
log.LogInformation(
|
||||
"[testcase-issue] GitHub not configured; the reconciler is idle");
|
||||
return;
|
||||
}
|
||||
|
||||
var minutes = config.GetValue<int?>("GitHub:ReconcileIntervalMinutes")
|
||||
?? DefaultIntervalMinutes;
|
||||
if (minutes <= 0)
|
||||
{
|
||||
log.LogInformation(
|
||||
"[testcase-issue] reconcile interval is {Minutes}; sweeps disabled", minutes);
|
||||
return;
|
||||
}
|
||||
|
||||
log.LogInformation(
|
||||
"[testcase-issue] reconciling against {Repo} every {Minutes} minute(s)",
|
||||
github.Repository, minutes);
|
||||
|
||||
using var timer = new PeriodicTimer(TimeSpan.FromMinutes(minutes));
|
||||
while (await SafeWaitAsync(timer, stoppingToken))
|
||||
{
|
||||
try
|
||||
{
|
||||
await using var scope = scopeFactory.CreateAsyncScope();
|
||||
var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();
|
||||
var changes = await TestCaseIssuesController.ReconcileAsync(
|
||||
db, github, log, stoppingToken);
|
||||
if (changes.Count > 0)
|
||||
log.LogInformation("[testcase-issue] sweep updated {Count} case(s)", changes.Count);
|
||||
}
|
||||
catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
|
||||
{
|
||||
return;
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
// One bad sweep must not take the loop down — GitHub being
|
||||
// unreachable for a minute is not a reason to stop
|
||||
// reconciling for the lifetime of the process.
|
||||
log.LogWarning(ex, "[testcase-issue] reconcile sweep failed; retrying next interval");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static async Task<bool> SafeWaitAsync(PeriodicTimer timer, CancellationToken ct)
|
||||
{
|
||||
try { return await timer.WaitForNextTickAsync(ct); }
|
||||
catch (OperationCanceledException) { return false; }
|
||||
}
|
||||
}
|
||||
@@ -210,6 +210,13 @@ public static class ServiceCollectionExtensions
|
||||
services.AddSingleton<RoomBlobNormalizerService>();
|
||||
services.AddHttpClient();
|
||||
services.AddSingleton<HtrAssetMirrorService>();
|
||||
// QA test cases ↔ GitHub issues. Singleton because it holds only
|
||||
// configuration; the reconciler makes its own scope for the DbContext.
|
||||
// Both are registered unconditionally — the service reports
|
||||
// IsConfigured=false without a token and the reconciler then idles,
|
||||
// so a deployment with no GitHub credentials behaves normally.
|
||||
services.AddSingleton<IGitHubIssues, GitHubIssueService>();
|
||||
services.AddHostedService<TestCaseIssueReconciler>();
|
||||
// Scoped (per-request) — these services hold DbContext references and
|
||||
// must share the request's scope to participate in the request's
|
||||
// transaction lifecycle. Pre-PR-3 they were Singleton with in-process
|
||||
|
||||
@@ -179,6 +179,59 @@ Enforcement caveat carried over from `RoomEntity.MaxCapacity`: this is the
|
||||
advertised cap. The client never sets Photon `RoomOptions.MaxPlayers`, so
|
||||
hard enforcement still needs a ClientMod Photon patch.
|
||||
|
||||
## Test cases ↔ GitHub issues
|
||||
|
||||
QA test cases can file and track GitHub issues. Configure with:
|
||||
|
||||
| Key | Meaning |
|
||||
| --- | --- |
|
||||
| `GitHub:Token` | PAT with `issues:write` on the target repo |
|
||||
| `GitHub:Repository` | `owner/name`, e.g. `DorkSquadRR/DorkNet` |
|
||||
| `GitHub:ReconcileIntervalMinutes` | Sweep interval, default 15; `0` disables sweeps but leaves the manual endpoint working |
|
||||
|
||||
**Without a token nothing breaks.** `IGitHubIssues.IsConfigured` is false, the
|
||||
endpoints answer `503` with the reason, and the background reconciler logs once
|
||||
and idles. A server with no GitHub credentials is a normal deployment.
|
||||
|
||||
Endpoints (admin-gated — these are not part of the 2023 client's surface):
|
||||
|
||||
- `POST api/testcasemanagement/v1/testcase/{id}/issue` — file and link.
|
||||
**Idempotent**: a case already carrying a live issue returns that issue
|
||||
instead of filing a duplicate, so sweeping a failing pass repeatedly is safe.
|
||||
The issue body carries the description, key, room, test pass and tester
|
||||
comments; labels are `qa` plus the case's own tags.
|
||||
- `DELETE api/testcasemanagement/v1/testcase/{id}/issue` — unlink only. The
|
||||
issue itself is left alone; closing someone's issue as a side effect of
|
||||
tidying a QA link would be the wrong call.
|
||||
- `POST api/testcasemanagement/v1/issues/sync` — reconcile now. The background
|
||||
service runs the same code path on its interval, so manual and automatic
|
||||
cannot drift.
|
||||
|
||||
### Where the link is stored
|
||||
|
||||
In `TestCaseEntity.JiraBugUrl` — the field Rec Room's own QA tooling used for
|
||||
the bug filed against a failing case. Reusing it means **no migration**, which
|
||||
matters more here than the name: the Postgres path is `EnsureCreated`-only and
|
||||
never replays migrations, so a new column has to be added twice (migration *and*
|
||||
an idempotent `Ensure*` patch) or it is simply missing in production. A field
|
||||
that already exists everywhere has neither problem, and the admin UI renders it
|
||||
already. A leftover genuine Jira link is ignored rather than misparsed — the
|
||||
issue number is only read from a URL matching the GitHub issue shape.
|
||||
|
||||
### What the reconciler will and won't change
|
||||
|
||||
| Issue | Case status | Result |
|
||||
| --- | --- | --- |
|
||||
| Closed | Failed | → NotYetTested |
|
||||
| Open | NotYetTested | → Failed |
|
||||
| anything | Claimed / Passed | untouched |
|
||||
|
||||
A closed issue does **not** mark the case Passed: closing is a developer's claim
|
||||
that the fix landed, not a tester's observation that the case passes, so it goes
|
||||
back in the queue for another run. And only those two states are the
|
||||
reconciler's to move — a case a tester has Claimed, or marked Passed after
|
||||
verifying the fix, is never dragged back underneath them.
|
||||
|
||||
## Build And Deploy
|
||||
|
||||
The service Dockerfile builds the admin SPA for service images that need
|
||||
|
||||
Reference in New Issue
Block a user