diff --git a/DorkNet.Server.Tests/TestCaseGitHubIssueTests.cs b/DorkNet.Server.Tests/TestCaseGitHubIssueTests.cs new file mode 100644 index 0000000..eddcdb5 --- /dev/null +++ b/DorkNet.Server.Tests/TestCaseGitHubIssueTests.cs @@ -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; + +/// +/// QA test cases linked to GitHub issues. +/// +/// The link lives in — 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. +/// +public sealed class TestCaseGitHubIssueTests : IClassFixture +{ + 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(); + 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(); + 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(); + 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(); + (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(); + 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 WithGitHub(IGitHubIssues github) => + _factory.WithWebHostBuilder(builder => + builder.ConfigureServices(services => + { + services.RemoveAll(); + services.AddSingleton(github); + })); + + private async Task AdminClientAsync(WebApplicationFactory 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(); + (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 SeedCaseAsync(WebApplicationFactory factory, int status) + { + var id = $"tc-{Guid.NewGuid():N}"; + await using var scope = factory.Services.CreateAsyncScope(); + var db = scope.ServiceProvider.GetRequiredService(); + 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; + } + + /// An in-memory GitHub. Records what was filed so the tests can + /// assert we did not file twice, and lets them flip issue state. + private sealed class FakeGitHub : IGitHubIssues + { + private readonly Dictionary _issues = []; + private int _next = 1; + + public bool IsConfigured => true; + + public string Repository => "DorkSquadRR/DorkNet"; + + public int Created { get; private set; } + + public Task CreateAsync( + string title, string body, IEnumerable 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(issue); + } + + public Task GetAsync(int number, CancellationToken ct = default) => + Task.FromResult(_issues.TryGetValue(number, out var issue) ? issue : null); + + public Task 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 }; + } + } +} diff --git a/DorkNet.Server/Controllers/API/TestCaseManagement/TestCaseIssuesController.cs b/DorkNet.Server/Controllers/API/TestCaseManagement/TestCaseIssuesController.cs new file mode 100644 index 0000000..86dd060 --- /dev/null +++ b/DorkNet.Server/Controllers/API/TestCaseManagement/TestCaseIssuesController.cs @@ -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; + +/// +/// Links QA test cases to GitHub issues. +/// +/// 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 +/// which are deliberately open +/// (the watch gates that tab client-side on the developer role). +/// +/// The issue URL is stored in — +/// 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. +/// +[ApiController] +[AdminOnly] +public class TestCaseIssuesController( + DorkNetDbContext db, + IGitHubIssues github, + ILogger log) : ControllerBase +{ + /// Test-case status values, matching the client's + /// TestCaseStatus enum. + private const int StatusNotYetTested = 0; + private const int StatusFailed = 2; + + /// POST api/testcasemanagement/v1/testcase/{id}/issue — + /// 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. + [HttpPost("api/testcasemanagement/v1/testcase/{id}/issue")] + public async Task 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)); + } + + /// DELETE api/testcasemanagement/v1/testcase/{id}/issue — + /// unlink, leaving the issue itself alone. Closing someone's issue as a + /// side effect of tidying a QA link would be the wrong call. + [HttpDelete("api/testcasemanagement/v1/testcase/{id}/issue")] + public async Task 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 }); + } + + /// POST api/testcasemanagement/v1/issues/sync — reconcile + /// every linked case against its issue. + /// + /// The state a test case should be in follows the issue, in one direction + /// only: + /// + /// + /// 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. + /// Issue REOPENED and the case is NotYetTested → back to Failed. + /// The bug came back, so the close transition above is undone. + /// + /// + /// 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. + [HttpPost("api/testcasemanagement/v1/issues/sync")] + public async Task 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 }); + } + + /// Shared by the endpoint and the background reconciler so the two + /// can never drift apart. + internal static async Task> 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(); + 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}"; + + /// The issue body. Written for whoever picks the bug up, so it + /// leads with what to do rather than with metadata. + private static string BodyFor(TestCaseEntity tc) + { + var lines = new List(); + 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("Filed from DorkNet QA test-case management."); + return string.Join("\n", lines); + } + + private static IEnumerable LabelsFor(TestCaseEntity tc) + { + yield return "qa"; + foreach (var tag in ParseComments(tc.TagsJson)) yield return tag; + } + + /// Both CommentsJson and TagsJson are JSON string arrays. A + /// comment is stored as playerId|timestamp|text; only the text is + /// worth putting in an issue. + private static List ParseComments(string json) + { + try + { + var items = System.Text.Json.JsonSerializer.Deserialize>(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 []; + } + } +} diff --git a/DorkNet.Server/Services/GitHubIssueService.cs b/DorkNet.Server/Services/GitHubIssueService.cs new file mode 100644 index 0000000..e09ef93 --- /dev/null +++ b/DorkNet.Server/Services/GitHubIssueService.cs @@ -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; + +/// One GitHub issue, reduced to what QA linking needs. +public sealed record GitHubIssue(int Number, string Url, string State, string Title); + +/// +/// 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. +/// +public interface IGitHubIssues +{ + /// 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. + bool IsConfigured { get; } + + /// owner/name, or empty when unconfigured. + string Repository { get; } + + Task CreateAsync(string title, string body, IEnumerable labels, CancellationToken ct = default); + + Task GetAsync(int number, CancellationToken ct = default); + + Task CommentAsync(int number, string body, CancellationToken ct = default); +} + +/// +/// REST implementation over api.github.com. +/// +/// Configured with GitHub:Token (a PAT with issues:write on the +/// target repo) and GitHub:Repository (owner/name). Both are +/// read once at construction; absent either, is +/// false and every call returns null rather than throwing, so QA endpoints +/// answer "not configured" instead of 500ing. +/// +public sealed class GitHubIssueService : IGitHubIssues +{ + private const string ApiRoot = "https://api.github.com"; + + private readonly IHttpClientFactory _httpClientFactory; + private readonly ILogger _log; + private readonly string _token; + + public GitHubIssueService( + IConfiguration config, + IHttpClientFactory httpClientFactory, + ILogger 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 CreateAsync( + string title, string body, IEnumerable 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 GetAsync(int number, CancellationToken ct = default) => + IsConfigured + ? SendAsync(HttpMethod.Get, $"/repos/{Repository}/issues/{number}", null, ct) + : Task.FromResult(null); + + public async Task 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 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 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; + } + } + + /// Pull the issue number out of a stored issue URL. + /// + /// The link lives in TestCaseEntity.JiraBugUrl — 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. + public static int? IssueNumberFromUrl(string? url) + { + if (string.IsNullOrWhiteSpace(url)) return null; + var match = Regex.Match(url, @"github\.com/[^/]+/[^/]+/issues/(?\d+)", + RegexOptions.IgnoreCase); + return match.Success && int.TryParse(match.Groups["number"].Value, out var number) + ? number + : null; + } +} diff --git a/DorkNet.Server/Services/TestCaseIssueReconciler.cs b/DorkNet.Server/Services/TestCaseIssueReconciler.cs new file mode 100644 index 0000000..50002e7 --- /dev/null +++ b/DorkNet.Server/Services/TestCaseIssueReconciler.cs @@ -0,0 +1,89 @@ +using Microsoft.Extensions.Options; +using DorkNet.Server.Controllers.API.TestCaseManagement; +using DorkNet.Server.Data; + +namespace DorkNet.Server.Services; + +/// +/// Keeps linked test cases in step with their GitHub issues without anyone +/// having to press anything. +/// +/// 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 +/// and is shared with the +/// manual endpoint, so the automatic and manual paths cannot drift. +/// +/// 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. +/// +public sealed class TestCaseIssueReconciler( + IServiceScopeFactory scopeFactory, + IGitHubIssues github, + IConfiguration config, + ILogger log) : BackgroundService +{ + /// 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 + /// GitHub:ReconcileIntervalMinutes; 0 or less disables the sweep + /// while leaving the manual endpoint working. + 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("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(); + 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 SafeWaitAsync(PeriodicTimer timer, CancellationToken ct) + { + try { return await timer.WaitForNextTickAsync(ct); } + catch (OperationCanceledException) { return false; } + } +} diff --git a/DorkNet.Server/Startup/ServiceCollectionExtensions.cs b/DorkNet.Server/Startup/ServiceCollectionExtensions.cs index 793d230..04e66ca 100644 --- a/DorkNet.Server/Startup/ServiceCollectionExtensions.cs +++ b/DorkNet.Server/Startup/ServiceCollectionExtensions.cs @@ -210,6 +210,13 @@ public static class ServiceCollectionExtensions services.AddSingleton(); services.AddHttpClient(); services.AddSingleton(); + // 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(); + services.AddHostedService(); // 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 diff --git a/docs/admin.md b/docs/admin.md index 22fc381..5ad6fd7 100644 --- a/docs/admin.md +++ b/docs/admin.md @@ -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