From 695dd954e7e35cb95384c4004c6f42370ee05bf7 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sat, 16 May 2026 14:28:27 +0200 Subject: [PATCH] test(setup): regression for telnet-only migration detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins the ordering invariant fixed in the preceding commit. Builds a fake-speaker scenario where: - SSH is unavailable (every SSH-driven axis stays false) - telnet getpdo reports the AfterTouch hostname Pre-fix, checkIsMigratedFromProbe ran before the telnet channel was drained, so summary.TelnetVerifiedConfig was empty when isTelnetMigrated read it — the telnet axis came back false and summary.IsMigrated followed. The CLI's `setup verify` exited non-zero, the web UI rendered "Not Migrated". Reproduced by foob61451 on #293. The test asserts: - summary.TelnetVerifiedConfig is populated (sanity guard — the downstream assertions are meaningless if the probe didn't run) - summary.TelnetMigrated == true - summary.IsMigrated == true Verified locally: the test PASSES with the ordering fix applied and FAILS without it. Failure messages name PR #294 by number so a future regression points at the same code path. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../setup/migration_summary_telnet_test.go | 53 +++++++++++++++++++ pkg/service/setup/setup.go | 8 +-- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/pkg/service/setup/migration_summary_telnet_test.go b/pkg/service/setup/migration_summary_telnet_test.go index 332ff62..454baff 100644 --- a/pkg/service/setup/migration_summary_telnet_test.go +++ b/pkg/service/setup/migration_summary_telnet_test.go @@ -185,3 +185,56 @@ func TestGetMigrationSummary_TelnetSucceedsSSHSucceeds(t *testing.T) { t.Errorf("TelnetVerifiedConfig = %q, want %q", summary.TelnetVerifiedConfig, target) } } + +// TestGetMigrationSummary_TelnetOnlyMigrationDetected pins the ordering +// bug fixed in PR #294 / issue #293. +// +// Before the fix, GetMigrationSummary called checkIsMigratedFromProbe +// before draining the telnet goroutine's result, so +// summary.TelnetVerifiedConfig was empty when isTelnetMigrated read it +// — and the telnet axis was always reported false. For speakers +// migrated *only* via telnet (envswitch flip; no SSH XML rewrite, +// no DNS hook, no CA install), this misclassification meant +// summary.IsMigrated was false despite the speaker actually pointing +// at AfterTouch. The CLI's `setup verify` exited non-zero, and the +// web UI rendered "Not Migrated". +// +// The fix moves m.checkIsMigratedFromProbe(summary, probe) to run +// *after* the <-telnetCh drain, so TelnetVerifiedConfig is populated +// when isTelnetMigrated inspects it. +// +// The scenario here matches foob61451's 2026-05-16 #293 reproducer: +// SSH unavailable / disabled (every axis false), telnet getpdo reports +// the AfterTouch host, no other migration path applied. +func TestGetMigrationSummary_TelnetOnlyMigrationDetected(t *testing.T) { + target := "http://example:8000" + ft := &fakeTelnet{ + banner: "BoseShell\n-> ", + responses: map[string]string{ + "getpdo CurrentSystemConfiguration": "margeServerUrl=" + target + "\n", + }, + } + + m, host, cleanup := telnetSummaryEnv(t, nil, ft) + defer cleanup() + + summary, err := m.GetMigrationSummary(host, "", "", nil) + if err != nil { + t.Fatalf("GetMigrationSummary: %v", err) + } + + // Pre-condition for the test to be meaningful: the telnet probe + // must have populated TelnetVerifiedConfig. Without this, the + // downstream assertions could pass trivially. + if !strings.Contains(summary.TelnetVerifiedConfig, target) { + t.Fatalf("setup: TelnetVerifiedConfig = %q, want it to contain %q", summary.TelnetVerifiedConfig, target) + } + + if !summary.TelnetMigrated { + t.Errorf("TelnetMigrated = false, want true — telnet getpdo reports %q which matches Manager.ServerURL host. Likely regression of PR #294 ordering fix in GetMigrationSummary.", target) + } + + if !summary.IsMigrated { + t.Errorf("IsMigrated = false, want true — telnet axis should carry IsMigrated when SSH-driven axes are false. Likely regression of PR #294 ordering fix.") + } +} diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 603b913..c065107 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -329,7 +329,7 @@ func (m *Manager) GetMigrationSummary(deviceIP, targetURL, proxyURL string, opti // 3. Provide HTTPS URL for testing (consumed by the migration UI) summary.ServerHTTPSURL = m.buildServerHTTPSURL(targetURL) - // 6. Mirroring settings + // 4. Mirroring settings if m.DataStore != nil { settings, err := m.DataStore.GetSettings() if err == nil { @@ -340,19 +340,19 @@ func (m *Manager) GetMigrationSummary(deviceIP, targetURL, proxyURL string, opti } } - // 7. Merge telnet preflight results (started in parallel at the top). + // 5. Merge telnet preflight results (started in parallel at the top). telnetResult := <-telnetCh summary.TelnetReachable = telnetResult.TelnetReachable summary.TelnetBanner = telnetResult.TelnetBanner summary.TelnetVerifiedConfig = telnetResult.TelnetVerifiedConfig summary.TelnetProbeError = telnetResult.TelnetProbeError - // 8. Check if migrated (must run after telnet results are merged so + // 6. Check if migrated (must run after telnet results are merged so // TelnetVerifiedConfig is populated). XML/hosts/resolv axes use the // probe data already gathered above. m.checkIsMigratedFromProbe(summary, probe) - // 9. Cross-check SSH-XML and telnet-getpdo readings; surface any + // 7. Cross-check SSH-XML and telnet-getpdo readings; surface any // divergence as a non-fatal warning. m.crossCheckPreflights(summary)