From 2b93eaf55b73cd36f0e90ea730164a394943934c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 8 May 2026 08:49:40 +0800 Subject: [PATCH] refactor: only reset a database table when the table's data was changed (#37573) Reduce CI time Saves about 3 minutes for each test suit test-unit: 13min -> 10min (-race) test-pgsql: 24min -> 20min (-race) test-mysql: 15min -> 12min test-mssql: 16min -> 12min --------- Co-authored-by: silverwind Co-authored-by: Claude (Opus 4.7) --- models/db/context.go | 13 ++-- models/db/engine_hook.go | 6 ++ models/migrations/migrationtest/tests.go | 3 +- models/unittest/fixtures.go | 95 +++++++++++++++++++++++- models/unittest/fixtures_loader.go | 32 +++++--- modules/setting/database.go | 5 +- tests/test_utils.go | 1 + 7 files changed, 136 insertions(+), 19 deletions(-) diff --git a/models/db/context.go b/models/db/context.go index 8bb14f1389..5e74f458ec 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -17,12 +17,15 @@ import ( "xorm.io/xorm" ) -type engineContextKeyType struct{} +type contextKey struct{ key string } -var engineContextKey = engineContextKeyType{} +var ( + contextKeyEngine = contextKey{"engine"} + ContextKeyTestFixtures = contextKey{"test-fixtures"} +) func withContextEngine(ctx context.Context, e Engine) context.Context { - return context.WithValue(ctx, engineContextKey, e) + return context.WithValue(ctx, contextKeyEngine, e) } var ( @@ -68,7 +71,7 @@ func contextSafetyCheck(e Engine) { // GetEngine gets an existing db Engine/Statement or creates a new Session func GetEngine(ctx context.Context) Engine { - if engine, ok := ctx.Value(engineContextKey).(Engine); ok { + if engine, ok := ctx.Value(contextKeyEngine).(Engine); ok { // if reusing the existing session, need to do "contextSafetyCheck" because the Iterate creates a "autoResetStatement=false" session contextSafetyCheck(engine) return engine @@ -309,7 +312,7 @@ func InTransaction(ctx context.Context) bool { } func getTransactionSession(ctx context.Context) *xorm.Session { - e, _ := ctx.Value(engineContextKey).(Engine) + e, _ := ctx.Value(contextKeyEngine).(Engine) if sess, ok := e.(*xorm.Session); ok && sess.IsInTx() { return sess } diff --git a/models/db/engine_hook.go b/models/db/engine_hook.go index 8709a2c2a1..8d8ed3992c 100644 --- a/models/db/engine_hook.go +++ b/models/db/engine_hook.go @@ -22,11 +22,17 @@ type EngineHook struct { var _ contexts.Hook = (*EngineHook)(nil) func (*EngineHook) BeforeProcess(c *contexts.ContextHook) (context.Context, error) { + if c.Ctx.Value(ContextKeyTestFixtures) != nil { + return c.Ctx, nil + } ctx, _ := gtprof.GetTracer().Start(c.Ctx, gtprof.TraceSpanDatabase) return ctx, nil } func (h *EngineHook) AfterProcess(c *contexts.ContextHook) error { + if c.Ctx.Value(ContextKeyTestFixtures) != nil { + return nil + } span := gtprof.GetContextSpan(c.Ctx) if span != nil { // Do not record SQL parameters here: diff --git a/models/migrations/migrationtest/tests.go b/models/migrations/migrationtest/tests.go index ed8bb16ef1..e0f7d04bb0 100644 --- a/models/migrations/migrationtest/tests.go +++ b/models/migrations/migrationtest/tests.go @@ -72,7 +72,7 @@ func PrepareTestEnv(t *testing.T, skip int, syncModels ...any) (*xorm.Engine, fu if err := unittest.InitFixtures( unittest.FixturesOptions{ Dir: fixturesDir, - }, x); err != nil { + }); err != nil { t.Errorf("error whilst initializing fixtures from %s: %v", fixturesDir, err) return x, deferFn } @@ -110,6 +110,7 @@ func mainTest(m *testing.M) int { if err = git.InitFull(); err != nil { return testlogger.MainErrorf("Unable to InitFull: %v", err) } + setting.Database.SlowQueryThreshold = 0 setting.LoadDBSetting() setting.InitLoggersForTest() return m.Run() diff --git a/models/unittest/fixtures.go b/models/unittest/fixtures.go index a9a01a3227..872bdffc6d 100644 --- a/models/unittest/fixtures.go +++ b/models/unittest/fixtures.go @@ -4,7 +4,10 @@ package unittest import ( + "context" "fmt" + "strings" + "unicode" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/auth/password/hash" @@ -12,11 +15,13 @@ import ( "code.gitea.io/gitea/modules/util" "xorm.io/xorm" + "xorm.io/xorm/contexts" "xorm.io/xorm/schemas" ) type FixturesLoader interface { Load() error + MarkTableChanged(tableName string) } var fixturesLoader FixturesLoader @@ -57,15 +62,101 @@ func loadFixtureResetSeqPgsql(e *xorm.Engine) error { return nil } +type fixturesHookStruct struct{} + +func cutSpaceForSQL(s string) (string, string, bool) { + s = strings.TrimSpace(s) + pos := strings.IndexFunc(s, unicode.IsSpace) + if pos == -1 { + return s, "", false + } + return s[:pos], strings.TrimSpace(s[pos+1:]), true +} + +func trimTableNameQuotes(s string) string { + pos := strings.IndexByte(s, '.') + if pos != -1 { + s = s[pos+1:] + } + return strings.Trim(s, "\"`[]") +} + +func (f fixturesHookStruct) BeforeProcess(c *contexts.ContextHook) (context.Context, error) { + if c.Ctx.Value(db.ContextKeyTestFixtures) != nil { + return c.Ctx, nil + } + ctx, sql := c.Ctx, c.SQL + cmdPart, cmdRemaining, ok := cutSpaceForSQL(sql) + if !ok { + return ctx, nil + } + + // ignore the SQLs which don't change data + if util.AsciiEqualFold(cmdPart, "SELECT") || + util.AsciiEqualFold(cmdPart, "SHOW") || + util.AsciiEqualFold(cmdPart, "PRAGMA") || + util.AsciiEqualFold(cmdPart, "ALTER") || + util.AsciiEqualFold(cmdPart, "CREATE") || + util.AsciiEqualFold(cmdPart, "DROP") || + util.AsciiEqualFold(cmdPart, "IF") || + util.AsciiEqualFold(cmdPart, "SET") || + util.AsciiEqualFold(cmdPart, "sp_rename") || + util.AsciiEqualFold(cmdPart, "BEGIN") || + util.AsciiEqualFold(cmdPart, "ROLLBACK") || + util.AsciiEqualFold(cmdPart, "COMMIT") { + return ctx, nil + } + + switch { + case util.AsciiEqualFold(cmdPart, "INSERT"): + cmdPart, cmdRemaining, _ = cutSpaceForSQL(cmdRemaining) + if util.AsciiEqualFold(cmdPart, "INTO") { + cmdPart, cmdRemaining, _ = cutSpaceForSQL(cmdRemaining) + } + fixturesLoader.MarkTableChanged(trimTableNameQuotes(cmdPart)) + case util.AsciiEqualFold(cmdPart, "MERGE"): + cmdPart, cmdRemaining, _ = cutSpaceForSQL(cmdRemaining) + if util.AsciiEqualFold(cmdPart, "INTO") { + cmdPart, cmdRemaining, _ = cutSpaceForSQL(cmdRemaining) + } + fixturesLoader.MarkTableChanged(trimTableNameQuotes(cmdPart)) + case util.AsciiEqualFold(cmdPart, "UPDATE"): + cmdPart, cmdRemaining, _ = cutSpaceForSQL(cmdRemaining) + fixturesLoader.MarkTableChanged(trimTableNameQuotes(cmdPart)) + case util.AsciiEqualFold(cmdPart, "DELETE"): + cmdPart, cmdRemaining, _ = cutSpaceForSQL(cmdRemaining) + if util.AsciiEqualFold(cmdPart, "FROM") { + cmdPart, cmdRemaining, _ = cutSpaceForSQL(cmdRemaining) + } + fixturesLoader.MarkTableChanged(trimTableNameQuotes(cmdPart)) + case util.AsciiEqualFold(cmdPart, "TRUNCATE"): + cmdPart, cmdRemaining, _ = cutSpaceForSQL(cmdRemaining) + if util.AsciiEqualFold(cmdPart, "TABLE") { + cmdPart, cmdRemaining, _ = cutSpaceForSQL(cmdRemaining) + } + fixturesLoader.MarkTableChanged(trimTableNameQuotes(cmdPart)) + default: + // should either parse the table name if it changes data, or ignore it + panic("unrecognized sql: " + sql) + } + _ = cmdRemaining + return ctx, nil +} + +func (f fixturesHookStruct) AfterProcess(c *contexts.ContextHook) error { + return nil +} + // InitFixtures initialize test fixtures for a test database -func InitFixtures(opts FixturesOptions, engine ...*xorm.Engine) (err error) { - xormEngine := util.IfZero(util.OptionalArg(engine), GetXORMEngine()) +func InitFixtures(opts FixturesOptions) (err error) { + xormEngine := GetXORMEngine() fixturesLoader, err = NewFixturesLoader(xormEngine, opts) // fixturesLoader = NewFixturesLoaderVendor(xormEngine, opts) // register the dummy hash algorithm function used in the test fixtures _ = hash.Register("dummy", hash.NewDummyHasher) setting.PasswordHashAlgo, _ = hash.SetDefaultPasswordHashAlgorithm("dummy") + xormEngine.AddHook(&fixturesHookStruct{}) return err } diff --git a/models/unittest/fixtures_loader.go b/models/unittest/fixtures_loader.go index 5b79cb5643..e2f2f5846f 100644 --- a/models/unittest/fixtures_loader.go +++ b/models/unittest/fixtures_loader.go @@ -4,6 +4,7 @@ package unittest import ( + "context" "database/sql" "encoding/hex" "fmt" @@ -11,6 +12,7 @@ import ( "path/filepath" "slices" "strings" + "sync" "code.gitea.io/gitea/models/db" @@ -32,7 +34,7 @@ type FixtureItem struct { type fixturesLoaderInternal struct { xormEngine *xorm.Engine - xormTableNames map[string]bool + tableSyncMap sync.Map db *sql.DB dbType schemas.DBType fixtures map[string]*FixtureItem @@ -148,25 +150,36 @@ func (f *fixturesLoaderInternal) Load() error { } defer func() { _ = tx.Rollback() }() + ctx := context.WithValue(context.Background(), db.ContextKeyTestFixtures, true) + for _, fixture := range f.fixtures { - if !f.xormTableNames[fixture.tableName] { + synced, existing := f.tableSyncMap.Load(fixture.tableName) + if synced == true || !existing { continue } if err := f.loadFixtures(tx, fixture); err != nil { return fmt.Errorf("failed to load fixtures from %s: %w", fixture.fileFullPath, err) } + f.tableSyncMap.Store(fixture.tableName, true) } if err = tx.Commit(); err != nil { return err } - for xormTableName := range f.xormTableNames { - if f.fixtures[xormTableName] == nil { - _, _ = f.xormEngine.Exec("DELETE FROM `" + xormTableName + "`") + f.tableSyncMap.Range(func(k, v any) bool { + tableName, synced := k.(string), v.(bool) + if !synced && f.fixtures[tableName] == nil { + _, _ = f.xormEngine.Context(ctx).Exec("DELETE FROM `" + tableName + "`") } - } + f.tableSyncMap.Store(tableName, true) + return true + }) return nil } +func (f *fixturesLoaderInternal) MarkTableChanged(tableName string) { + f.tableSyncMap.Store(tableName, false) +} + func FixturesFileFullPaths(dir string, files []string) (map[string]*FixtureItem, error) { if files != nil && len(files) == 0 { return nil, nil //nolint:nilnil // load nothing @@ -215,11 +228,12 @@ func NewFixturesLoader(x *xorm.Engine, opts FixturesOptions) (FixturesLoader, er f.paramPlaceholder = func(idx int) string { return "?" } } + // If a model is not imported in a package (no bean is registered), the table won't exist in database. + // So only use tables of registered models (beans). xormBeans, _ := db.NamesToBean() - f.xormTableNames = map[string]bool{} for _, bean := range xormBeans { - f.xormTableNames[x.TableName(bean)] = true + beanTableName := x.TableName(bean) + f.tableSyncMap.Store(trimTableNameQuotes(beanTableName), false) } - return f, nil } diff --git a/modules/setting/database.go b/modules/setting/database.go index a65c8e9495..edaaf32c14 100644 --- a/modules/setting/database.go +++ b/modules/setting/database.go @@ -41,7 +41,8 @@ var ( AutoMigration bool SlowQueryThreshold time.Duration }{ - IterateBufferSize: 50, + IterateBufferSize: 50, + SlowQueryThreshold: 5 * time.Second, } ) @@ -86,7 +87,7 @@ func loadDBSetting(rootCfg ConfigProvider) { Database.DBConnectRetries = sec.Key("DB_RETRIES").MustInt(10) Database.DBConnectBackoff = sec.Key("DB_RETRY_BACKOFF").MustDuration(3 * time.Second) Database.AutoMigration = sec.Key("AUTO_MIGRATION").MustBool(true) - Database.SlowQueryThreshold = sec.Key("SLOW_QUERY_THRESHOLD").MustDuration(5 * time.Second) + Database.SlowQueryThreshold = sec.Key("SLOW_QUERY_THRESHOLD").MustDuration(Database.SlowQueryThreshold) } // DatabaseType FIXME: it is also used directly with "schemas.DBType", so the names must be consistent diff --git a/tests/test_utils.go b/tests/test_utils.go index 11e5ac2434..1b7987527a 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -36,6 +36,7 @@ func InitIntegrationTest() error { return err } + setting.Database.SlowQueryThreshold = 0 setting.LoadDBSetting() cleanupDb, err := unittest.ResetTestDatabase() if err != nil {