From 793b13fe012a98f3b14c129bf6b3a47a2a800413 Mon Sep 17 00:00:00 2001 From: Wessie Date: Mon, 3 Jun 2024 03:06:17 +0100 Subject: [PATCH] storage/mariadb: make tests use one database and truncate tables between tests this speeds up these tests by about half the runtime, slightly less clean but might be worth keeping anyway. storage/test: adjust to the different method of running the tests --- storage/mariadb/mariadb_test.go | 107 ++++++++++++++++++++------------ storage/mariadb/search_test.go | 3 +- storage/mariadb/track.go | 27 ++++++-- storage/test/storage.go | 37 ++++------- storage/test/track.go | 33 ++++++++++ 5 files changed, 133 insertions(+), 74 deletions(-) diff --git a/storage/mariadb/mariadb_test.go b/storage/mariadb/mariadb_test.go index 5857a989..5e300419 100644 --- a/storage/mariadb/mariadb_test.go +++ b/storage/mariadb/mariadb_test.go @@ -18,16 +18,22 @@ import ( type MariaDBSetup struct { container *mariadb.MariaDBContainer + dbname string db *sqlx.DB + + truncateQuery string + storage radio.StorageService } func (setup *MariaDBSetup) Setup(ctx context.Context) error { - testcontainers.Logger = testcontainers.TestLogger(storagetest.CtxT(ctx)) + t := storagetest.CtxT(ctx) + testcontainers.Logger = testcontainers.TestLogger(t) + setup.dbname = "go-test" // setup a container to test in container, err := mariadb.RunContainer(ctx, testcontainers.WithImage("mariadb:latest"), - //mariadb.WithDatabase("test"), + mariadb.WithDatabase(setup.dbname), mariadb.WithUsername("root"), mariadb.WithPassword(""), ) @@ -36,76 +42,97 @@ func (setup *MariaDBSetup) Setup(ctx context.Context) error { } setup.container = container + // setup the DSN dsn, err := container.ConnectionString(ctx) if err != nil { return err } dsn = fixdsn(dsn) + mycfg, err := mysql.ParseDSN(dsn) + if err != nil { + return err + } + mycfg.MultiStatements = true + dsn = mycfg.FormatDSN() + // setup the config + cfg := config.TestConfig() + bare := cfg.Conf() + bare.Database.DSN = dsn + cfg.StoreConf(bare) + + // connect setup.db, err = sqlx.ConnectContext(ctx, "mysql", dsn) if err != nil { - container.Terminate(ctx) + return err } - return err -} -func (setup *MariaDBSetup) RunMigrations(ctx context.Context, cfg config.Config) error { - migr, err := mysqlmigrations.New(ctx, cfg) + err = setup.RunMigrations(ctx, cfg) if err != nil { return err } - err = migr.Up() + var names []string + err = sqlx.Select(setup.db, &names, "SELECT table_name FROM information_schema.tables WHERE table_schema = ?", setup.dbname) if err != nil { return err } - return nil -} -func (setup *MariaDBSetup) TearDown(ctx context.Context) error { - err := setup.container.Terminate(ctx) + setup.truncateQuery = "SET FOREIGN_KEY_CHECKS=0;" + for _, name := range names { + if name == "schema_migrations" { + // this one is where the migrations package stores its stuff, don't truncate + // that one + continue + } + if name == "permission_kinds" { + // permission_kinds are constant values too, don't delete these between tests + continue + } + + setup.truncateQuery += "TRUNCATE TABLE " + name + "; " + } + setup.truncateQuery += "SET FOREIGN_KEY_CHECKS=1;" + + setup.storage, err = storage.Open(ctx, cfg) if err != nil { return err } + return nil } -func (setup *MariaDBSetup) CreateStorage(ctx context.Context, name string) (radio.StorageService, error) { - // test names have a / prefixed sometimes - name = strings.ReplaceAll(name, "/", "") - // create the database - setup.db.MustExecContext(ctx, "CREATE DATABASE "+name+";") - // update our config to connect to the container - cfg := config.TestConfig() - - dsn, err := setup.container.ConnectionString(ctx) +func (setup *MariaDBSetup) RunMigrations(ctx context.Context, cfg config.Config) error { + migr, err := mysqlmigrations.New(ctx, cfg) if err != nil { - return nil, err + return err } - dsn = fixdsn(dsn) + defer migr.Close() - mycfg, err := mysql.ParseDSN(dsn) + err = migr.Up() if err != nil { - return nil, err + return err } + return nil +} - mycfg.DBName = name - bare := cfg.Conf() - bare.Database.DSN = mycfg.FormatDSN() - cfg.StoreConf(bare) - - // run migrations - err = setup.RunMigrations(ctx, cfg) - if err != nil { - return nil, err +func (setup *MariaDBSetup) TearDown(ctx context.Context) error { + if setup.container != nil { + setup.container.Terminate(ctx) } - - // then open a storage instance - s, err := storage.Open(ctx, cfg) - if err != nil { - return nil, err + if setup.db != nil { + setup.db.Close() + } + if setup.storage != nil { + setup.storage.Close() } - return s, nil + return nil +} + +func (setup *MariaDBSetup) CreateStorage(ctx context.Context) radio.StorageService { + // truncate all tables in the database + sqlx.MustExecContext(ctx, setup.db, setup.truncateQuery) + return setup.storage } func TestMariaDBStorage(t *testing.T) { diff --git a/storage/mariadb/search_test.go b/storage/mariadb/search_test.go index 60577ef6..011cd713 100644 --- a/storage/mariadb/search_test.go +++ b/storage/mariadb/search_test.go @@ -89,8 +89,7 @@ func FuzzProcessQuery(f *testing.F) { require.NoError(f, err) defer setup.TearDown(ctx) - s, err := setup.CreateStorage(ctx, "FuzzProcessQuery") - require.NoError(f, err) + s := setup.CreateStorage(ctx) ss := s.(interface { Search() radio.SearchService diff --git a/storage/mariadb/track.go b/storage/mariadb/track.go index e451b22f..59d1e41d 100644 --- a/storage/mariadb/track.go +++ b/storage/mariadb/track.go @@ -11,9 +11,17 @@ import ( "github.com/jmoiron/sqlx" ) -// FavePriorityIncrement is the amount we increase/decrease priority by -// on a track when it gets favorited/unfavorited -const FavePriorityIncrement = 1 +const ( + // FavePriorityIncrement is the amount we increase/decrease priority by + // on a track when it gets favorited/unfavorited + FavePriorityIncrement = 1 + // RequestPriorityIncremenet is the amount we increase priority by when + // a track gets requested + RequestPriorityIncrement = 1 + // RequestCountIncrement is the amount we increase requestcount by when + // a track gets requested + RequestCountIncrement = 2 +) func expand(query string) string { var orig = query @@ -314,6 +322,10 @@ func (ss SongStorage) FavoriteCount(song radio.Song) (int64, error) { handle, deferFn := ss.handle.span(op) defer deferFn() + if song.HashLink.IsZero() { + return 0, errors.E(op, errors.InvalidArgument) + } + var query = ` SELECT count(*) @@ -339,6 +351,10 @@ func (ss SongStorage) Favorites(song radio.Song) ([]string, error) { handle, deferFn := ss.handle.span(op) defer deferFn() + if song.HashLink.IsZero() { + return nil, errors.E(op, errors.InvalidArgument) + } + var query = ` SELECT DISTINCT enick.nick @@ -960,11 +976,10 @@ func (ts TrackStorage) UpdateRequestInfo(id radio.TrackID) error { handle, deferFn := ts.handle.span(op) defer deferFn() - // TODO(wessie): don't hardcode requestcount and priority var query = `UPDATE tracks SET lastrequested=NOW(), - requestcount=requestcount+2, priority=priority+1 WHERE id=?;` + requestcount=requestcount+?, priority=priority+? WHERE id=?;` - res, err := handle.Exec(query, id) + res, err := handle.Exec(query, RequestCountIncrement, RequestPriorityIncrement, id) if err != nil { return errors.E(op, err) } diff --git a/storage/test/storage.go b/storage/test/storage.go index ab809210..86130cc4 100644 --- a/storage/test/storage.go +++ b/storage/test/storage.go @@ -13,19 +13,15 @@ import ( type TestSetup interface { Setup(context.Context) error - CreateStorage(ctx context.Context, name string) (radio.StorageService, error) + CreateStorage(ctx context.Context) radio.StorageService TearDown(context.Context) error } func RunTests(t *testing.T, s TestSetup) { ctx := zerolog.New(os.Stdout).Level(zerolog.ErrorLevel).WithContext(context.Background()) ctx = PutT(ctx, t) - // do test setup - err := s.Setup(ctx) - if err != nil { - t.Fatal(err) - } + // make sure TearDown is always called defer func() { err := s.TearDown(ctx) if err != nil { @@ -33,6 +29,12 @@ func RunTests(t *testing.T, s TestSetup) { } }() + // do test Setup + err := s.Setup(ctx) + if err != nil { + t.Fatal(err) + } + suite := NewSuite(ctx, s) tests := gatherAllTests(suite) @@ -40,9 +42,8 @@ func RunTests(t *testing.T, s TestSetup) { t.Run("Storage", func(t *testing.T) { for name, fn := range tests { fn := fn - t.Run(name, func(t *testing.T) { - t.Parallel() + t.Run(name, func(t *testing.T) { err := suite.BeforeTest(t.Name()) if err != nil { t.Error("failed test setup:", err) @@ -65,7 +66,7 @@ func gatherAllTests(suite *Suite) map[string]testFn { mv := rv.Method(i) mt := mv.Type() - if mt.NumIn() != 1 && mt.NumOut() != 0 { + if mt.NumIn() != 1 || mt.NumOut() != 0 { continue } @@ -97,31 +98,15 @@ type Suite struct { } func (suite *Suite) BeforeTest(testName string) error { - s, err := suite.ToBeTested.CreateStorage(suite.ctx, testName) - if err != nil { - return err - } - - suite.storageMu.Lock() - suite.storageMap[testName] = s - suite.storageMu.Unlock() return nil } func (suite *Suite) AfterTest(testName string) error { - suite.storageMu.Lock() - defer suite.storageMu.Unlock() - if s, ok := suite.storageMap[testName]; ok { - return s.Close() - } return nil } func (suite *Suite) Storage(t *testing.T) radio.StorageService { - suite.storageMu.Lock() - defer suite.storageMu.Unlock() - - return suite.storageMap[t.Name()] + return suite.ToBeTested.CreateStorage(suite.ctx) } type testingKey struct{} diff --git a/storage/test/track.go b/storage/test/track.go index 3b87567b..54ce02ab 100644 --- a/storage/test/track.go +++ b/storage/test/track.go @@ -34,6 +34,39 @@ func (suite *Suite) TestSongPlayedCount(t *testing.T) { require.Zero(t, res) } +func (suite *Suite) TestSongFavoriteCount(t *testing.T) { + s := suite.Storage(t) + ss := s.Song(suite.ctx) + + // invalid argument + res, err := ss.FavoriteCount(radio.Song{}) + Require(t, errors.InvalidArgument, err) + require.Zero(t, res) + + // non-existant argument + res, err = ss.FavoriteCount(radio.NewSong("favorite-count-test")) + // TODO: fix this, we currently can't tell the difference in the + // current mariadb implementation + // Require(t, errors.SongUnknown, err) + require.NoError(t, err) + require.Zero(t, res) +} + +func (suite *Suite) TestSongFavorites(t *testing.T) { + s := suite.Storage(t) + ss := s.Song(suite.ctx) + + // invalid argument + res, err := ss.Favorites(radio.Song{}) + Require(t, errors.InvalidArgument, err) + require.Zero(t, res) + + // non-existant argument + res, err = ss.Favorites(radio.NewSong("favorites-test")) + require.NoError(t, err) + require.Len(t, res, 0) +} + func (suite *Suite) TestSongUpdateLength(t *testing.T) { s := suite.Storage(t) ss := s.Song(suite.ctx)