diff --git a/internal/application/application.go b/internal/application/application.go index 505320a..ff4646d 100644 --- a/internal/application/application.go +++ b/internal/application/application.go @@ -274,8 +274,19 @@ func (app *Application) checkState() { return } + oldState := app.state app.state = newState + // Both DOWNLOADING_ASSETS and READY map to the HOME page. + // Skip re-navigation when transitioning between them to avoid + // destroying and recreating the Home page (which would close + // the already-closed DownloadDone channel). + oldIsHome := oldState == DOWNLOADING_ASSETS || oldState == READY + newIsHome := newState == DOWNLOADING_ASSETS || newState == READY + if oldIsHome && newIsHome { + return + } + switch newState { case NEEDS_INTERNET: app.PageRouter.NavigateTo(page.WIFI) diff --git a/internal/application/application_test.go b/internal/application/application_test.go index 6f8504d..a467f61 100644 --- a/internal/application/application_test.go +++ b/internal/application/application_test.go @@ -2,6 +2,7 @@ package application import ( "testing" + "time" "gorm.io/gorm" "rhemawavetech.com/rhemawave/internal/db" @@ -245,6 +246,205 @@ func TestDownloadDoneChannelSetsDownloadsComplete(t *testing.T) { } } +// TestPrerequisitePriorityEmailBeforeLicense verifies that missing email +// is reported before missing license key, even when both are absent. +func TestPrerequisitePriorityEmailBeforeLicense(t *testing.T) { + app := &Application{ + state: FIRST_RUN, + DB: setupTestDB(t), + } + + networkIsUp = true + config, err := db.GetConfig(app.DB) + if err != nil { + t.Fatal(err) + } + config.EmailAddress = "" + config.LicenseKey = "" + app.DB.Save(config) + + app.downloadsComplete = true + + downloadDone := make(chan struct{}) + app.PageRouter = &page.Router{ + Context: page.PageCtx{ + DbPtr: app.DB, + DownloadDone: downloadDone, + }, + } + + result := app.computeState() + + if result != NEEDS_EMAIL_ADDRESS { + t.Errorf("state = %v, want NEEDS_EMAIL_ADDRESS (email should be checked before license)", result) + } +} + +// TestPrerequisitePriorityLicenseAfterEmail verifies that when email is +// present but license is missing, the state is NEEDS_LICENSE_KEY. +func TestPrerequisitePriorityLicenseAfterEmail(t *testing.T) { + app := &Application{ + state: FIRST_RUN, + DB: setupTestDB(t), + } + + networkIsUp = true + config, err := db.GetConfig(app.DB) + if err != nil { + t.Fatal(err) + } + config.EmailAddress = "test@example.com" + config.LicenseKey = "" + app.DB.Save(config) + + app.downloadsComplete = true + + downloadDone := make(chan struct{}) + app.PageRouter = &page.Router{ + Context: page.PageCtx{ + DbPtr: app.DB, + DownloadDone: downloadDone, + }, + } + + result := app.computeState() + + if result != NEEDS_LICENSE_KEY { + t.Errorf("state = %v, want NEEDS_LICENSE_KEY", result) + } +} + +// TestStateBlocksOnDownloadUntilChannelClosed verifies that computeState +// returns DOWNLOADING_ASSETS repeatedly until DownloadDone is closed. +func TestStateBlocksOnDownloadUntilChannelClosed(t *testing.T) { + app := &Application{ + state: FIRST_RUN, + DB: setupTestDB(t), + } + + networkIsUp = true + config, err := db.GetConfig(app.DB) + if err != nil { + t.Fatal(err) + } + config.EmailAddress = "test@example.com" + config.LicenseKey = "test-key-123" + app.DB.Save(config) + + app.downloadsComplete = false + + downloadDone := make(chan struct{}) + app.PageRouter = &page.Router{ + Context: page.PageCtx{ + DbPtr: app.DB, + DownloadDone: downloadDone, + }, + } + + // Should return DOWNLOADING_ASSETS multiple times before channel closes. + for i := 0; i < 5; i++ { + result := app.computeState() + if result != DOWNLOADING_ASSETS { + t.Fatalf("iteration %d: state = %v, want DOWNLOADING_ASSETS", i, result) + } + } + + // Now close the channel. + close(downloadDone) + + result := app.computeState() + if result != READY { + t.Errorf("after close: state = %v, want READY", result) + } +} + +// TestDownloadDoneNonBlocking verifies that computeState does not block +// when DownloadDone channel is open (non-blocking select). +func TestDownloadDoneNonBlocking(t *testing.T) { + app := &Application{ + state: FIRST_RUN, + DB: setupTestDB(t), + } + + networkIsUp = true + config, err := db.GetConfig(app.DB) + if err != nil { + t.Fatal(err) + } + config.EmailAddress = "test@example.com" + config.LicenseKey = "test-key-123" + app.DB.Save(config) + + app.downloadsComplete = false + + downloadDone := make(chan struct{}) + app.PageRouter = &page.Router{ + Context: page.PageCtx{ + DbPtr: app.DB, + DownloadDone: downloadDone, + }, + } + + // computeState must return promptly even though DownloadDone is not closed. + done := make(chan struct{}) + go func() { + result := app.computeState() + if result != DOWNLOADING_ASSETS { + t.Errorf("state = %v, want DOWNLOADING_ASSETS", result) + } + close(done) + }() + + select { + case <-done: + // Good: returned promptly. + case <-time.After(2 * time.Second): + t.Fatal("computeState blocked on DownloadDone channel (should be non-blocking)") + } +} + +// TestMultipleComputeStateCallsAfterDownloadComplete verifies that calling +// computeState repeatedly after downloads finish does not panic or change state. +func TestMultipleComputeStateCallsAfterDownloadComplete(t *testing.T) { + app := &Application{ + state: FIRST_RUN, + DB: setupTestDB(t), + } + + networkIsUp = true + config, err := db.GetConfig(app.DB) + if err != nil { + t.Fatal(err) + } + config.EmailAddress = "test@example.com" + config.LicenseKey = "test-key-123" + app.DB.Save(config) + + downloadDone := make(chan struct{}) + close(downloadDone) + app.PageRouter = &page.Router{ + Context: page.PageCtx{ + DbPtr: app.DB, + DownloadDone: downloadDone, + }, + } + + // First call transitions to READY. + result := app.computeState() + if result != READY { + t.Fatalf("first call: state = %v, want READY", result) + } + app.state = result + + // Subsequent calls should return READY without panic. + for i := 0; i < 10; i++ { + result = app.computeState() + if result != READY { + t.Fatalf("iteration %d: state = %v, want READY", i, result) + } + } +} + func setupTestDB(t *testing.T) *gorm.DB { t.Helper() d, err := db.Connect(":memory:") diff --git a/internal/page/home_test.go b/internal/page/home_test.go new file mode 100644 index 0000000..02d59e4 --- /dev/null +++ b/internal/page/home_test.go @@ -0,0 +1,107 @@ +package page + +import ( + "testing" + + "rhemawavetech.com/rhemawave/internal/download" +) + +// Regression: DownloadDone was closed twice when checkState() re-navigated +// to HOME on the DOWNLOADING_ASSETS -> READY transition. The new Home page +// saw the already-closed MessageChannel and tried to close DownloadDone +// again, causing panic: close of closed channel. +// +// Primary fix: checkState() now skips navigation when transitioning between +// states that both map to the HOME page. The downloadDone flag on Home is +// defense-in-depth. + +// TestHomeDownloadDoneGuardPreventsDoubleClose verifies that the per-instance +// downloadDone flag prevents closing DownloadDone more than once, even if a +// new Home page somehow reaches the close path. +func TestHomeDownloadDoneGuardPreventsDoubleClose(t *testing.T) { + downloadDone := make(chan struct{}) + + // First Home closes DownloadDone. + h1 := &Home{downloadDone: false} + h1.Context = &PageCtx{DownloadDone: downloadDone} + if !h1.downloadDone { + h1.downloadDone = true + close(h1.Context.DownloadDone) + } + + // Second Home with guard already set (simulates defense-in-depth). + h2 := &Home{downloadDone: true} + h2.Context = &PageCtx{DownloadDone: downloadDone} + if !h2.downloadDone { + h2.downloadDone = true + close(h2.Context.DownloadDone) // would panic without guard + } +} + +// TestHomeDownloadDoneGuardUnsetWouldPanic proves that without the +// downloadDone guard, closing DownloadDone a second time panics. +func TestHomeDownloadDoneGuardUnsetWouldPanic(t *testing.T) { + downloadDone := make(chan struct{}) + close(downloadDone) // pre-close + + defer func() { + if r := recover(); r == nil { + t.Error("expected panic when closing already-closed channel") + } + }() + + // New Home with downloadDone=false tries to close an already-closed channel. + h := &Home{downloadDone: false} + h.Context = &PageCtx{DownloadDone: downloadDone} + if !h.downloadDone { + h.downloadDone = true + close(h.Context.DownloadDone) // panics + } +} + +// TestHomeDownloadFailedDoesNotClose verifies that when a download fails, +// DownloadDone is NOT closed (the error path should not signal success). +func TestHomeDownloadFailedDoesNotClose(t *testing.T) { + downloadDone := make(chan struct{}) + + h := &Home{ + downloadFailed: true, + downloadDone: false, + } + h.Context = &PageCtx{ + MessageChannel: make(chan download.DownloadStatus), + DownloadDone: downloadDone, + } + + // Simulate the Update logic: on failure, downloadDone should not be set + // and DownloadDone should not be closed. + if h.downloadFailed { + // Error path: do not close DownloadDone. + } else if !h.downloadDone { + h.downloadDone = true + close(h.Context.DownloadDone) + } + + select { + case <-downloadDone: + t.Error("DownloadDone should not be closed on download failure") + default: + // Correct: channel is still open. + } +} + +// TestHomeDownloadDoneAlreadyClosed verifies that a Home page with +// downloadDone=true never attempts to close the channel. +func TestHomeDownloadDoneAlreadyClosed(t *testing.T) { + downloadDone := make(chan struct{}) + close(downloadDone) + + h := &Home{downloadDone: true} + h.Context = &PageCtx{DownloadDone: downloadDone} + + // Guard should prevent any close attempt. + if !h.downloadDone { + h.downloadDone = true + close(h.Context.DownloadDone) + } +}