From 37aece9791458a7a6b3a88a51756340066ea5d97 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Fri, 11 Sep 2026 08:21:35 +0800 Subject: [PATCH] =?UTF-8?q?test=E2=9C=85:=20share=20one=20sys=5Fapp=20fixt?= =?UTF-8?q?ure,=20and=20count=20through=20the=20helper=20that=20checks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two tests in this package each wrote out what an installed sys_app row looks like, field for field, and a third inlined the same Create with a different status. One appRow(t, db, code, status) now covers all three, so a new NOT NULL column on SysApp is one edit rather than three. One assertion counted with a bare db.Model(...).Count(&n) and dropped the error that call returns. A failing query leaves n at zero, which is exactly what that assertion wanted to see - so the test would have passed on a broken query. The package already had a count helper that fails on the error, and this now uses it. Also a cycle reached from outside itself. The existing case walks straight into its own cycle from the first code, so the path trimming had nothing to do and replacing it with the untrimmed path left the test green - the trimming was never covered. With a requiring b, b requiring c and c requiring b, the untrimmed report names a as part of a cycle it is not in, and the test goes red. --- cmd/migrate/install_test.go | 47 ++++++++++++++++++++++++----------- cmd/migrate/uninstall_test.go | 6 +---- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/cmd/migrate/install_test.go b/cmd/migrate/install_test.go index 12a6fd0f..44ff8dbd 100644 --- a/cmd/migrate/install_test.go +++ b/cmd/migrate/install_test.go @@ -75,13 +75,14 @@ func orderManifest(version string) app.Manifest { } } -// installedApp writes the sys_app row a satisfied dependency looks like. -func installedApp(t *testing.T, db *gorm.DB, code string) { +// appRow writes one sys_app row: what another application looks like to the +// installer, in whichever state the caller is testing against. +func appRow(t *testing.T, db *gorm.DB, code string, status int) { t.Helper() if err := db.Create(&adminmodels.SysApp{ - AppCode: code, Name: code, Version: "1.0.0", Status: adminmodels.AppInstalled, + AppCode: code, Name: code, Version: "1.0.0", Status: status, }).Error; err != nil { - t.Fatalf("seeding %q as installed: %v", code, err) + t.Fatalf("seeding %q with status %d: %v", code, status, err) } } @@ -481,8 +482,8 @@ func TestInstallNormalizesTheAppCode(t *testing.T) { // shape sys_app.requires carries. func TestInstallStoresTheDeclaredRequires(t *testing.T) { db := newInstallDB(t) - installedApp(t, db, "crm") - installedApp(t, db, "billing") + appRow(t, db, "crm", adminmodels.AppInstalled) + appRow(t, db, "billing", adminmodels.AppInstalled) eng := &fakeEngine{entries: []migration.StatusEntry{ {Version: "order-1786800001000", AppCode: "order", Registered: true}, }} @@ -519,9 +520,7 @@ func TestInstallRefusesWhenADependencyIsNotInstalled(t *testing.T) { t.Errorf("the engine ran anyway: %v", eng.calls) } // Refused before phase A, so a refusal leaves nothing behind. - var n int64 - db.Model(&adminmodels.SysApp{}).Where("app_code = ?", "order").Count(&n) - if n != 0 { + if n := count(t, db, "sys_app", "app_code = ?", "order"); n != 0 { t.Errorf("a refused install wrote %d sys_app row(s)", n) } } @@ -540,11 +539,7 @@ func TestInstallRefusesWhenADependencyIsNotFinished(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { db := newInstallDB(t) - if err := db.Create(&adminmodels.SysApp{ - AppCode: "crm", Name: "crm", Version: "1.0.0", Status: tc.status, - }).Error; err != nil { - t.Fatal(err) - } + appRow(t, db, "crm", tc.status) m := orderManifest("1.0.0") m.Requires = []string{"crm"} _, err := install(db, &fakeEngine{}, m) @@ -560,7 +555,7 @@ func TestInstallRefusesWhenADependencyIsNotFinished(t *testing.T) { func TestInstallAcceptsASatisfiedDependency(t *testing.T) { db := newInstallDB(t) - installedApp(t, db, "crm") + appRow(t, db, "crm", adminmodels.AppInstalled) eng := &fakeEngine{entries: []migration.StatusEntry{ {Version: "order-1786800001000", AppCode: "order", Registered: true}, }} @@ -630,3 +625,25 @@ func TestDependencyCycleOfOne(t *testing.T) { t.Errorf("error = %q", err) } } + +// The cycle reached from outside it. a is not part of anything circular; b +// and c are. Reporting the walk instead of the cycle would name a as well, +// and sending somebody to look at an application that is not involved is +// the whole reason the path is trimmed. +func TestDependencyCycleReportsOnlyTheCycleItReached(t *testing.T) { + manifests := map[string]app.Manifest{ + "a": {Code: "a", Requires: []string{"b"}}, + "b": {Code: "b", Requires: []string{"c"}}, + "c": {Code: "c", Requires: []string{"b"}}, + } + err := refuseOnDependencyCycle(manifests) + if err == nil { + t.Fatal("a cycle was accepted") + } + if !strings.Contains(err.Error(), "b -> c -> b") { + t.Errorf("error = %q, want just the cycle", err) + } + if strings.Contains(err.Error(), "a ->") { + t.Errorf("the walk that reached the cycle was reported as part of it: %q", err) + } +} diff --git a/cmd/migrate/uninstall_test.go b/cmd/migrate/uninstall_test.go index 2ae79df4..76694a3a 100644 --- a/cmd/migrate/uninstall_test.go +++ b/cmd/migrate/uninstall_test.go @@ -87,11 +87,7 @@ func seedApp(t *testing.T, db *gorm.DB, code string) { }).Error; err != nil { t.Fatalf("recording the migration for %q: %v", code, err) } - if err := db.Create(&adminmodels.SysApp{ - AppCode: code, Name: code, Version: "1.0.0", Status: adminmodels.AppInstalled, - }).Error; err != nil { - t.Fatalf("recording sys_app for %q: %v", code, err) - } + appRow(t, db, code, adminmodels.AppInstalled) } func count(t *testing.T, db *gorm.DB, table, where string, args ...any) int64 {