From 75a35764cf63e01e5286671e277c5d7fd4bf3659 Mon Sep 17 00:00:00 2001 From: Kevin Schneider Date: Sat, 2 May 2026 08:47:20 -0700 Subject: [PATCH] Tests Phase 3: TUI flow tests + remove ActiveHubProjects dead branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final phase of the test-suite buildout. Phase 3 covers the L3 layer per the test plan: bubbletea state-machine transitions driven by direct Update(msg) calls, plus one cross-layer test that drives the full UI -> tea.Cmd -> api package -> mocked APS server chain. Coverage: ui 24.5% -> 32.5% (+8pp) total 38.5% -> 43.1% (+4.6pp) Coverage by package across all three phases: config 90.6% (Phase 1) fusion 84.2% (Phase 1+2, +2 from dead-code removal here) auth 73.9% (Phase 1+2) api 69.7% (Phase 1+2) ui 32.5% (Phase 1+3 — view/render code intentionally untested) total 43.1% ui/app_test.go adds 7 tests: - TestUpdate_TokenReadyMsg_TransitionsToLoading: empty/non-empty token paths - TestUpdate_TokenReadyMsg_EmptyTokenGoesAuthNeeded - TestUpdate_KeyQuit: 'q' in stateBrowsing returns tea.QuitMsg - TestUpdate_NavigateRight_LoadsContents (cross-layer L2/L3): drives a project NavigateRight through the ui state machine into the fan-out cmd that calls api.GetFolders + api.GetProjectItems concurrently against an httptest GraphQL fake; asserts the contentsLoadedMsg has folders before items, decoded into NavItems with the right Kind/IsContainer - TestVerifySameHub: 4 sub-cases (id match, name fallback, no match, empty skips check) against a fake MCP server - TestRecoverFromError_AuthError_DeletesTokens: t.Setenv-redirected HOME; saves a token, drives recovery from a "401 unauthorized" error, asserts state reset + tokens.json deleted from disk - TestBreadcrumb_HitDetection: pure unit on buildBreadcrumb; asserts 4 hits (hub + project + 2 folders) with right kinds, indices, and monotonically increasing x-bounds Production change: api.SetGraphqlEndpointForTesting added so ui flow tests can swap the GraphQL endpoint without exporting the var. Function doc explicitly says "production code MUST NOT call this". Cleanup: removed the success:false guard in fusion.ActiveHubProjects (was unreachable — parseToolErrorText in callTool catches that case upstream and surfaces it via the err returned by c.invoke). Tightened TestActiveHubProjects_SuccessFalse to assert the actual upstream error message ("tool reported failure") instead of accepting either branch. All tests run in under 5s with -race -count=1. Co-Authored-By: Claude Opus 4.7 (1M context) --- api/client.go | 11 ++ fusion/mcp.go | 8 +- fusion/mcp_test.go | 13 +- ui/app_test.go | 380 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 400 insertions(+), 12 deletions(-) create mode 100644 ui/app_test.go diff --git a/api/client.go b/api/client.go index 678f589..4ac7cb2 100644 --- a/api/client.go +++ b/api/client.go @@ -49,6 +49,17 @@ func SetRegion(r string) { region = r } +// SetGraphqlEndpointForTesting overrides the GraphQL endpoint URL and +// returns a function that restores the prior value. Intended only for +// tests that need to point the api package at an httptest.Server from a +// different test package (notably ui flow tests that drive a tea.Cmd which +// internally calls into api). Production code MUST NOT call this. +func SetGraphqlEndpointForTesting(url string) (restore func()) { + prev := graphqlEndpoint + graphqlEndpoint = url + return func() { graphqlEndpoint = prev } +} + // NavItem is a navigable node in the APS Manufacturing Data Model hierarchy. type NavItem struct { ID string diff --git a/fusion/mcp.go b/fusion/mcp.go index 3438061..d4405ba 100644 --- a/fusion/mcp.go +++ b/fusion/mcp.go @@ -158,16 +158,16 @@ func (c *Client) ActiveHubProjects(ctx context.Context) ([]HubProject, error) { if tr == nil || len(tr.Content) == 0 { return nil, errors.New("fusion MCP: empty projects response") } + // success:false payloads (with or without an error string) are caught + // upstream by parseToolErrorText in callTool and surfaced as an error + // from c.invoke above, so we only reach this point on a success path — + // no need for a per-method success guard here. var payload struct { - Success *bool `json:"success"` Projects []HubProject `json:"projects"` } if err := json.Unmarshal([]byte(tr.Content[0].Text), &payload); err != nil { return nil, fmt.Errorf("fusion MCP: decoding projects: %w", err) } - if payload.Success != nil && !*payload.Success { - return nil, errors.New("fusion MCP: projects query failed") - } return payload.Projects, nil } diff --git a/fusion/mcp_test.go b/fusion/mcp_test.go index 36d89d9..73f5b7e 100644 --- a/fusion/mcp_test.go +++ b/fusion/mcp_test.go @@ -346,14 +346,11 @@ func TestActiveHubProjects_SuccessFalse(t *testing.T) { if err == nil { t.Fatal("ActiveHubProjects: expected error for success:false, got nil") } - // Subtlety: callTool's parseToolErrorText branch fires first on success:false - // payloads, surfacing them as "tool reported failure" before the per-method - // "projects query failed" branch in ActiveHubProjects ever runs. Either - // message indicates the failure was caught — assert on either to remain - // resilient to whichever guard short-circuits first. - msg := err.Error() - if !strings.Contains(msg, "projects query failed") && !strings.Contains(msg, "tool reported failure") { - t.Errorf("ActiveHubProjects: error %q does not contain \"projects query failed\" or \"tool reported failure\"", msg) + // callTool's parseToolErrorText catches {success:false} (no error field) + // and surfaces it as "tool reported failure". The per-method success + // guard in ActiveHubProjects was removed because it was unreachable. + if msg := err.Error(); !strings.Contains(msg, "tool reported failure") { + t.Errorf("ActiveHubProjects: error %q does not contain %q", msg, "tool reported failure") } } diff --git a/ui/app_test.go b/ui/app_test.go new file mode 100644 index 0000000..4aba34c --- /dev/null +++ b/ui/app_test.go @@ -0,0 +1,380 @@ +package ui + +import ( + "context" + "encoding/base64" + "errors" + "strings" + "testing" + "time" + + "github.com/charmbracelet/bubbles/spinner" + tea "github.com/charmbracelet/bubbletea" + + "github.com/schneik80/FusionDataCLI/api" + "github.com/schneik80/FusionDataCLI/auth" + "github.com/schneik80/FusionDataCLI/fusion" + "github.com/schneik80/FusionDataCLI/internal/testutil" +) + +// TestUpdate_TokenReadyMsg_TransitionsToLoading drives the model from the +// pre-auth waiting state into the hub-loading state by feeding it a +// successful tokenReadyMsg. It locks down the contract that a non-empty +// token transitions to stateLoading + hubLoading and dispatches a hub-load +// command. +func TestUpdate_TokenReadyMsg_TransitionsToLoading(t *testing.T) { + m := Model{ + state: stateLoading, + spinner: spinner.New(), + styleCache: &styleCache{}, + detailsCache: map[string]*api.ItemDetails{}, + } + updated, cmd := m.Update(tokenReadyMsg{token: "abc"}) + um, ok := updated.(Model) + if !ok { + t.Fatalf("Update did not return Model, got %T", updated) + } + if um.token != "abc" { + t.Errorf("token = %q, want %q", um.token, "abc") + } + if um.state != stateLoading { + t.Errorf("state = %d, want stateLoading (%d)", um.state, stateLoading) + } + if !um.hubLoading { + t.Errorf("hubLoading = false, want true") + } + if cmd == nil { + t.Errorf("cmd = nil, want non-nil load-hubs cmd") + } +} + +// TestUpdate_TokenReadyMsg_EmptyTokenGoesAuthNeeded confirms the auth +// fall-through: when checkTokensCmd reports no token, the UI moves to +// stateAuthNeeded and emits no command. +func TestUpdate_TokenReadyMsg_EmptyTokenGoesAuthNeeded(t *testing.T) { + m := Model{ + state: stateLoading, + spinner: spinner.New(), + styleCache: &styleCache{}, + detailsCache: map[string]*api.ItemDetails{}, + } + updated, cmd := m.Update(tokenReadyMsg{token: ""}) + um, ok := updated.(Model) + if !ok { + t.Fatalf("Update did not return Model, got %T", updated) + } + if um.state != stateAuthNeeded { + t.Errorf("state = %d, want stateAuthNeeded (%d)", um.state, stateAuthNeeded) + } + if cmd != nil { + t.Errorf("cmd = %v, want nil", cmd) + } +} + +// TestUpdate_KeyQuit asserts that pressing q in the browsing state +// returns tea.Quit, which evaluates to a tea.QuitMsg{} when invoked. +func TestUpdate_KeyQuit(t *testing.T) { + m := sampleBrowsingModel(120, 40) + _, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'q'}}) + if cmd == nil { + t.Fatalf("cmd = nil, want tea.Quit") + } + msg := cmd() + if _, ok := msg.(tea.QuitMsg); !ok { + t.Errorf("cmd() returned %T, want tea.QuitMsg", msg) + } +} + +// TestUpdate_NavigateRight_LoadsContents is the marquee Phase 3 test. It +// drives a right-arrow press from stateBrowsing/colProjects through to +// the contentsLoadedMsg returned by loadProjectContentsCmd's fan-in, +// exercising the full chain: state transition → fan-out tea.Cmd → +// concurrent api.GetFolders + api.GetProjectItems → merge into a +// single contentsLoadedMsg. +func TestUpdate_NavigateRight_LoadsContents(t *testing.T) { + handler := func(req testutil.GraphQLRequest) testutil.GraphQLResponse { + if req.AuthHeader != "Bearer test-token" { + t.Errorf("AuthHeader = %q, want %q", req.AuthHeader, "Bearer test-token") + } + if got := req.Variables["projectId"]; got != "proj-1" { + t.Errorf("projectId = %v, want \"proj-1\"", got) + } + if strings.Contains(req.Query, "foldersByProject") { + return testutil.GraphQLResponse{Data: map[string]any{ + "foldersByProject": map[string]any{ + "pagination": map[string]any{"cursor": ""}, + "results": []map[string]any{ + {"id": "folder-1", "name": "Drawings"}, + }, + }, + }} + } + return testutil.GraphQLResponse{Data: map[string]any{ + "itemsByProject": map[string]any{ + "pagination": map[string]any{"cursor": ""}, + "results": []map[string]any{ + {"__typename": "DesignItem", "id": "design-1", "name": "Widget"}, + }, + }, + }} + } + srv := testutil.GraphQLServer(t, handler) + t.Cleanup(api.SetGraphqlEndpointForTesting(srv.URL)) + + m := Model{ + state: stateBrowsing, + width: 120, + height: 40, + activeCol: colProjects, + token: "test-token", + selectedHubID: "hub-1", + cols: [numCols][]api.NavItem{ + {{ID: "proj-1", Name: "Project A", Kind: "project", AltID: "alt-1", IsContainer: true}}, + nil, + }, + cursors: [numCols]int{0, 0}, + spinner: spinner.New(), + styleCache: &styleCache{}, + detailsCache: map[string]*api.ItemDetails{}, + } + + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRight}) + um, ok := updated.(Model) + if !ok { + t.Fatalf("Update did not return Model, got %T", updated) + } + if um.activeCol != colContents { + t.Errorf("activeCol = %d, want colContents (%d)", um.activeCol, colContents) + } + if !um.loading[colContents] { + t.Errorf("loading[colContents] = false, want true") + } + if um.selectedProjectAltID != "alt-1" { + t.Errorf("selectedProjectAltID = %q, want %q", um.selectedProjectAltID, "alt-1") + } + if cmd == nil { + t.Fatalf("cmd = nil, want load-contents cmd") + } + + msg := cmd() + loaded, ok := msg.(contentsLoadedMsg) + if !ok { + t.Fatalf("cmd() returned %T, want contentsLoadedMsg", msg) + } + if got, want := len(loaded.items), 2; got != want { + t.Fatalf("len(items) = %d, want %d (got items=%+v)", got, want, loaded.items) + } + // Folders come before items per loadProjectContentsCmd's append order. + if loaded.items[0].Kind != "folder" || !loaded.items[0].IsContainer { + t.Errorf("items[0] = %+v, want kind=folder & IsContainer=true", loaded.items[0]) + } + if loaded.items[0].Name != "Drawings" { + t.Errorf("items[0].Name = %q, want %q", loaded.items[0].Name, "Drawings") + } + if loaded.items[1].Kind != "design" { + t.Errorf("items[1].Kind = %q, want \"design\"", loaded.items[1].Kind) + } + if loaded.items[1].Name != "Widget" { + t.Errorf("items[1].Name = %q, want %q", loaded.items[1].Name, "Widget") + } +} + +// TestVerifySameHub covers the four code paths in verifySameHub: +// - exact ID match (after NormalizeProjectID) +// - case-insensitive name fallback when the ID can't be parsed +// - no match → returns the "different hub" error naming the expected hub +// - empty inputs short-circuit before the MCP call +func TestVerifySameHub(t *testing.T) { + srv := testutil.NewMCPServer(t, testutil.MCPScenario{ + SessionID: "verify-sid", + Tools: map[string]testutil.MCPHandler{ + "fusion_mcp_read": func(args map[string]any) testutil.MCPResponse { + return testutil.MCPResponse{ + ContentText: `{"success": true, "projects": [` + + `{"id": "20250213876602531", "name": "Buggy"},` + + `{"id": "98765432101234567", "name": "Robot"}` + + `]}`, + } + }, + }, + }) + client := &fusion.Client{Endpoint: srv.URL, HTTP: srv.Client()} + + encode := func(plain string) string { + return "a." + base64.RawURLEncoding.EncodeToString([]byte(plain)) + } + + cases := []struct { + name string + altID string + projName string + hubName string + wantErr bool + errSubstr []string + }{ + { + name: "exact_id_match", + altID: encode("business:autodesk#20250213876602531"), + hubName: "MyHub", + wantErr: false, + }, + { + name: "name_match_when_id_unparseable", + altID: "garbage", + projName: "Robot", + hubName: "MyHub", + wantErr: false, + }, + { + name: "no_match", + altID: encode("business:autodesk#9999"), + projName: "Nope", + hubName: "MyHub", + wantErr: true, + errSubstr: []string{"different hub", "MyHub"}, + }, + { + name: "empty_skips_check", + altID: "", + wantErr: false, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + err := verifySameHub(ctx, client, tc.altID, tc.projName, tc.hubName) + if tc.wantErr { + if err == nil { + t.Fatalf("verifySameHub: expected error, got nil") + } + for _, sub := range tc.errSubstr { + if !strings.Contains(err.Error(), sub) { + t.Errorf("verifySameHub: error %q missing substring %q", err.Error(), sub) + } + } + return + } + if err != nil { + t.Errorf("verifySameHub: unexpected error: %v", err) + } + }) + } +} + +// TestRecoverFromError_AuthError_DeletesTokens locks down the auth-recovery +// behaviour: when the model is in stateError with an auth-flavored error, +// recoverFromError must remove the on-disk token file so the next +// checkTokensCmd run prompts for fresh login. Non-auth recovery paths skip +// the token deletion (covered implicitly by the explicit auth-error +// trigger here). +func TestRecoverFromError_AuthError_DeletesTokens(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + td := &auth.TokenData{ + AccessToken: "x", + RefreshToken: "y", + ExpiresAt: time.Now().Add(time.Hour), + } + if err := auth.SaveTokens(td); err != nil { + t.Fatalf("SaveTokens: unexpected error: %v", err) + } + // Sanity-check that the file is on disk before we ask recoverFromError + // to delete it — otherwise a missing-file false-positive would mask a + // regression where the function silently does nothing. + if got, err := auth.LoadTokens(); err != nil || got == nil { + t.Fatalf("LoadTokens before recover: got=%v err=%v, want non-nil token", got, err) + } + + m := Model{ + state: stateError, + err: errors.New("401 unauthorized"), + spinner: spinner.New(), + styleCache: &styleCache{}, + detailsCache: map[string]*api.ItemDetails{}, + } + if !isAuthError(m.err) { + t.Fatalf("test setup: isAuthError(%v) = false; pick an auth-flavored message", m.err) + } + updated, cmd := m.recoverFromError() + if updated.state != stateLoading { + t.Errorf("state after recover = %d, want stateLoading (%d)", updated.state, stateLoading) + } + if updated.err != nil { + t.Errorf("err after recover = %v, want nil", updated.err) + } + if cmd == nil { + t.Errorf("cmd after recover = nil, want non-nil (spinner.Tick + checkTokensCmd batch)") + } + + got, err := auth.LoadTokens() + if err != nil { + t.Fatalf("LoadTokens after recover: unexpected error: %v", err) + } + if got != nil { + t.Errorf("LoadTokens after recover: got %+v, want nil (file should be deleted)", got) + } +} + +// TestBreadcrumb_HitDetection exercises the pure buildBreadcrumb helper +// against a model with hub + project + a 2-deep folder stack, verifying +// the rendered text, the number of clickable hits, their kinds, and +// folder-stack indices. +func TestBreadcrumb_HitDetection(t *testing.T) { + m := Model{ + width: 120, + height: 40, + selectedHubNameCache: "Hub", + cols: [numCols][]api.NavItem{ + {{ID: "p1", Name: "Project", Kind: "project"}}, + {{ID: "f2", Name: "Subfolder", Kind: "folder", IsContainer: true}}, + }, + cursors: [numCols]int{0, 0}, + folderStack: []breadcrumbEntry{ + {id: "f1", name: "Outer"}, + {id: "f2", name: "Inner"}, + }, + activeCol: colContents, + spinner: spinner.New(), + styleCache: &styleCache{}, + detailsCache: map[string]*api.ItemDetails{}, + } + + text, hits := m.buildBreadcrumb(breadcrumbXOffset()) + + for _, want := range []string{"Hub", "Project", "Outer", "Inner"} { + if !strings.Contains(text, want) { + t.Errorf("breadcrumb text %q missing segment %q", text, want) + } + } + if !strings.Contains(text, " › ") { + t.Errorf("breadcrumb text %q missing separator %q", text, " › ") + } + + if len(hits) != 4 { + t.Fatalf("len(hits) = %d, want 4 (hub + project + 2 folders); hits=%+v", len(hits), hits) + } + + wantKinds := []string{"hub", "project", "folder", "folder"} + off := breadcrumbXOffset() + for i, h := range hits { + if h.kind != wantKinds[i] { + t.Errorf("hits[%d].kind = %q, want %q", i, h.kind, wantKinds[i]) + } + if h.xStart < off { + t.Errorf("hits[%d].xStart = %d, want >= %d", i, h.xStart, off) + } + if h.xEnd <= h.xStart { + t.Errorf("hits[%d]: xEnd %d <= xStart %d", i, h.xEnd, h.xStart) + } + } + + if hits[2].index != 0 { + t.Errorf("hits[2].index = %d, want 0 (Outer folder)", hits[2].index) + } + if hits[3].index != 1 { + t.Errorf("hits[3].index = %d, want 1 (Inner folder)", hits[3].index) + } +}