From a884e7e9c510a5c312c05c325c5519f293f9cae3 Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 8 Jun 2026 22:59:25 +0200 Subject: [PATCH] test(bdd): add scenario name-coverage gate (no godog) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close the gap where docs/use-cases/*.feature claimed to be the behavior spec but nothing executed them — so scenarios drifted (the stale "auto summarizes every new video" and "manual is the default" were proof). Decision (per issue #5 BDD-runner fork): no godog — keep .feature as design records, add a cheap name-coverage gate instead. TestScenarioCoverage parses every scenario and asserts each non-@pending one maps to an existing Go test in the scenarioCoverage manifest; it flags unmapped scenarios, missing/renamed tests, and stale entries. It checks the link, not that the test exercises the scenario (the deliberate trade for skipping godog). Also: - Fix the stale ADR-018 drift: "Manual is the default" -> auto is the default for new users; added an explicit default scenario + a plain manual scenario. - Tag 4 documented-but-unbuilt/untested scenarios @pending with reasons (Vimeo connect, BYO config flow, logout->welcome, re-register-after-delete) so they are tracked without a false coverage claim. - CLAUDE.md BDD section now describes the real setup (design records + the gate + @pending convention) instead of claiming an executable spec. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 10 +- docs/use-cases/connect_account.feature | 5 + docs/use-cases/landing_page.feature | 3 + docs/use-cases/registration.feature | 3 + docs/use-cases/summarize_mode.feature | 9 +- test/acceptance/scenario_coverage_test.go | 211 ++++++++++++++++++++++ 6 files changed, 236 insertions(+), 5 deletions(-) create mode 100644 test/acceptance/scenario_coverage_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 5ac78e4..2b8c188 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,8 +60,14 @@ These caused real mistakes that were caught and corrected; the corrections are l stores, and sinks are adapters. Adding a video provider or a sink = a new adapter implementing the interface, nothing in the engine changes. This is what keeps "standalone vs homelab" a wiring choice (ADR-003). -- **BDD.** The `docs/use-cases/*.feature` files are the behavior spec. New behavior gets a - scenario; the use-case core is tested through fake adapters, not live YouTube/brain. +- **BDD.** The `docs/use-cases/*.feature` files are the behavior spec (design records — there is + no godog runner). New behavior gets a scenario; the use-case core is tested through fake + adapters, not live YouTube/brain. A name-coverage gate (`test/acceptance/scenario_coverage_test.go`, + `TestScenarioCoverage`) keeps the two from drifting: every non-`@pending` scenario must be + mapped to an existing Go test in `scenarioCoverage`. When you add a scenario, either map it to + its covering test or tag it `@pending` in the `.feature` with a one-line reason. It checks the + *link*, not that the test exercises the scenario — that's the deliberate trade for not running + godog (see issue #5 / the BDD-runner decision). ## Skills (engineering discipline) diff --git a/docs/use-cases/connect_account.feature b/docs/use-cases/connect_account.feature index 95df65b..31af737 100644 --- a/docs/use-cases/connect_account.feature +++ b/docs/use-cases/connect_account.feature @@ -10,6 +10,8 @@ Feature: Connect and manage video accounts And my refresh token is stored only as a secret reference And my subscriptions are synced + @pending + # Vimeo connect is not built yet (provider label exists; no connect flow or test). Scenario: Connect a Vimeo account Given I have no connected video accounts When I connect my Vimeo account @@ -28,6 +30,9 @@ Feature: Connect and manage video accounts And no new videos are watched for that connection And my existing summaries remain readable + @pending + # Per-provider BYO credential config is not built as a web flow yet (the summarizer + # supports a fallback endpoint, but there is no user-facing BYO setup + its test). Scenario Outline: BYO AI credential is optional and per-provider When I configure a BYO provider "" Then the credential is stored only as a secret reference diff --git a/docs/use-cases/landing_page.feature b/docs/use-cases/landing_page.feature index 31d75d4..2172b6c 100644 --- a/docs/use-cases/landing_page.feature +++ b/docs/use-cases/landing_page.feature @@ -19,6 +19,9 @@ Feature: Public landing page Then I see a link to my summaries And I see a way to log out + @pending + # Behaviour ships (logout redirects to /welcome) but is not unit-tested: logout lives in + # the OIDC Auth impl and StubAuth has no routes to exercise it cheaply. Scenario: Logging out returns to the welcome page Given I am logged in When I log out diff --git a/docs/use-cases/registration.feature b/docs/use-cases/registration.feature index 16d67da..e5da769 100644 --- a/docs/use-cases/registration.feature +++ b/docs/use-cases/registration.feature @@ -34,6 +34,9 @@ Feature: Register and manage a multi-user account And the other user's data remains intact And my Dex identity is left intact + @pending + # Re-registration after delete is supported by design (delete leaves the Dex identity, + # ADR-013) but has no dedicated end-to-end test yet. Scenario: A deleted user can register again as a fresh account Given I deleted my Tapir account but my Dex identity still exists When I sign in again diff --git a/docs/use-cases/summarize_mode.feature b/docs/use-cases/summarize_mode.feature index 535dd6a..55b7fcc 100644 --- a/docs/use-cases/summarize_mode.feature +++ b/docs/use-cases/summarize_mode.feature @@ -19,9 +19,12 @@ Feature: Choose how new videos get summarized And it is not summarized automatically And I can still summarize it on demand with "Summarize" - Scenario: Manual mode is the default and leaves new videos unsummarized - Given I have not changed my summarization mode - Then my mode is "manual" + Scenario: Automatic is the default for a new user + Given I have just registered + Then my summarization mode is "auto" + + Scenario: Manual mode leaves new videos unsummarized + Given my summarization mode is "manual" When a subscribed channel posts a new video with captions Then the video appears in my list with no summary And nothing is summarized until I request it diff --git a/test/acceptance/scenario_coverage_test.go b/test/acceptance/scenario_coverage_test.go new file mode 100644 index 0000000..d60146b --- /dev/null +++ b/test/acceptance/scenario_coverage_test.go @@ -0,0 +1,211 @@ +package acceptance + +// This is the name-coverage gate for the BDD spec (see docs/use-cases/*.feature). +// There is no godog runner — the .feature files are design records, and the real +// behaviour is covered by the hand-written Go tests across the module. This test +// keeps the two from drifting in the cheapest honest way: every non-@pending +// Scenario must have an entry in scenarioCoverage pointing at a Go test that +// actually exists. It does NOT prove the test exercises the scenario (only godog +// could); it catches the common drift — "added a scenario, forgot the test", a +// renamed/deleted covering test, or a scenario removed without cleaning the map. +// +// When you add a Scenario: either map it here to its covering test, or tag it +// @pending in the .feature with a one-line reason for why it has no test yet. + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// scenarioCoverage maps each non-@pending Scenario name to the Go test that +// covers it. Keep it in sync with docs/use-cases/*.feature — the test below +// fails if a scenario is unmapped, a mapped test is missing, or an entry no +// longer matches a real non-pending scenario. +var scenarioCoverage = map[string]string{ + // ai_routing.feature + "Local AI produces the summary": "TestSummarize_LocalSucceeds", + "Local AI fails and the user has a BYO provider configured": "TestSummarize_FallsBackToBYO", + "Local AI fails and the user has no BYO provider": "TestSummarize_LocalFailsNoBYO_NoExternalSend", + "A user without BYO never has content sent externally": "TestSummarize_NoBYO_ContentOnlyLocal", + + // landing_page.feature + "An unauthenticated visit to the root is sent to the welcome page": "TestUnauthenticatedRootRedirectsToWelcome", + "The welcome page invites an unauthenticated visitor to start": "TestWelcomeLoggedOut", + "An authenticated user on the welcome page sees their way in and out": "TestWelcomeLoggedIn", + + // connect_account.feature + "Connect a YouTube account": "TestCallbackExchangesAndRecordsConnection", + "Tokens are never stored in the clear": "TestCallbackExchangesAndRecordsConnection", + "Revoking a connection stops watching but keeps history": "TestDisconnectRemovesTokenAndConnectionKeepsAccount", + + // summarize_mode.feature + "Auto mode summarizes recent new videos automatically": "TestRunOnce_AutoMode_SkipsOldVideos", + "Auto mode lists older videos without summarizing them": "TestRunOnce_AutoMode_SkipsOldVideos", + "Automatic is the default for a new user": "TestRegisteredUserDefaultsAutoSummarizeOn", + "Manual mode leaves new videos unsummarized": "TestRunOnce_ManualMode_SkipsUnrequested", + "Requesting a summary in manual mode queues it for the next run": "TestRunOnce_ManualMode_ProcessesRequested", + + // registration.feature + "A new Dex subject is routed to registration": "TestUnregisteredSubjectRedirectedToRegister", + "Registering creates the account and its identity mapping": "TestRegisterCreatesExactlyOneUserAndIdentity", + "A returning subject passes straight through": "TestRegisteredSubjectPassesThrough", + "Deleting an account removes only my data and leaves other users untouched": "TestDeleteAccountWipesDataAndSecretsAndLogsOut", + + // summarize_new_video.feature + "A subscribed channel posts a video that has captions": "TestSubscribedVideoWithCaptionsIsSummarizedAndDelivered", + "A subscribed channel posts a video with no usable transcript": "TestVideoWithNoTranscriptIsSkipped", + "A channel I am not subscribed to posts a video": "TestUnsubscribedChannelVideoIsNotProcessed", + "The same video is not summarized twice": "TestAlreadySummarizedVideoIsNotReprocessed", +} + +var ( + scenarioRe = regexp.MustCompile(`^\s*Scenario(?: Outline)?:\s*(.+?)\s*$`) + testFuncRe = regexp.MustCompile(`^func (Test\w+)\(`) +) + +// scenario is one parsed Gherkin scenario and whether it is @pending. +type scenario struct { + name string + pending bool +} + +func TestScenarioCoverage(t *testing.T) { + root := moduleRoot(t) + + scenarios := parseScenarios(t, filepath.Join(root, "docs", "use-cases")) + if len(scenarios) == 0 { + t.Fatal("no scenarios parsed from docs/use-cases — wrong path?") + } + tests := allTestFuncNames(t, root) + + // Index scenario names for the reverse (stale-entry) check. + active := map[string]bool{} // non-pending scenario names + var pending []string + for _, s := range scenarios { + if s.pending { + pending = append(pending, s.name) + continue + } + active[s.name] = true + + // 1. Every non-pending scenario must be mapped. + fn, ok := scenarioCoverage[s.name] + if !ok { + t.Errorf("scenario %q has no coverage entry — map it in scenarioCoverage to a covering test, or tag it @pending in the .feature", s.name) + continue + } + // 2. The mapped test must actually exist. + if !tests[fn] { + t.Errorf("scenario %q maps to %q, which is not a Test function anywhere in the module", s.name, fn) + } + } + + // 3. No stale entries: every map key must be a real, non-pending scenario. + for name := range scenarioCoverage { + if !active[name] { + t.Errorf("scenarioCoverage has entry %q, which is not a current non-pending scenario (renamed, removed, or now @pending?)", name) + } + } + + if len(pending) > 0 { + t.Logf("%d @pending scenario(s) without a test (tracked, not required): %s", + len(pending), strings.Join(pending, "; ")) + } +} + +// parseScenarios reads every *.feature under dir and returns its scenarios with +// their @pending status. A scenario is @pending when a `@pending` tag line +// precedes it (tags survive intervening comment lines, the layout these files +// use); the flag is consumed at the Scenario line and reset afterward. +func parseScenarios(t *testing.T, dir string) []scenario { + t.Helper() + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read use-cases dir: %v", err) + } + var out []scenario + for _, e := range entries { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".feature") { + continue + } + b, err := os.ReadFile(filepath.Join(dir, e.Name())) + if err != nil { + t.Fatalf("read %s: %v", e.Name(), err) + } + pending := false + for _, line := range strings.Split(string(b), "\n") { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "@") { + if strings.Contains(trimmed, "@pending") { + pending = true + } + continue + } + if m := scenarioRe.FindStringSubmatch(line); m != nil { + out = append(out, scenario{name: m[1], pending: pending}) + pending = false + } + // comment (#) and step lines leave a set @pending intact until the + // scenario consumes it; a blank line between scenarios is harmless. + } + } + return out +} + +// allTestFuncNames walks the module for `func TestXxx(` declarations, excluding +// this file (whose regex literal would otherwise look like a definition). +func allTestFuncNames(t *testing.T, root string) map[string]bool { + t.Helper() + self := "scenario_coverage_test.go" + names := map[string]bool{} + err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + if d.Name() == ".git" { + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(path, "_test.go") || filepath.Base(path) == self { + return nil + } + b, err := os.ReadFile(path) + if err != nil { + return err + } + for _, line := range strings.Split(string(b), "\n") { + if m := testFuncRe.FindStringSubmatch(line); m != nil { + names[m[1]] = true + } + } + return nil + }) + if err != nil { + t.Fatalf("walk module: %v", err) + } + return names +} + +// moduleRoot walks up from the working directory to the dir containing go.mod. +func moduleRoot(t *testing.T) string { + t.Helper() + dir, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + t.Fatal("go.mod not found walking up from cwd") + } + dir = parent + } +}