From 22c0487c7bc2400d3ae46f89587a774d07a35ded Mon Sep 17 00:00:00 2001 From: hackerman <3372410+aeneasr@users.noreply.github.com> Date: Tue, 2 Apr 2019 13:50:44 +0200 Subject: [PATCH] Resolve sql testing race issues (#1332) Signed-off-by: aeneasr --- Makefile | 4 ++-- consent/manager_test.go | 25 ++++++------------------- driver/registry_sql.go | 17 +++++++++-------- oauth2/fosite_store_test.go | 22 ++++++---------------- oauth2/oauth2_auth_code_test.go | 29 ++++++++++++++--------------- 5 files changed, 37 insertions(+), 60 deletions(-) diff --git a/Makefile b/Makefile index 75debb215d5..4602f6a4c57 100644 --- a/Makefile +++ b/Makefile @@ -27,8 +27,8 @@ test-resetdb: docker run --rm --name hydra_test_database_postgres -p 3445:5432 -e POSTGRES_PASSWORD=secret -e POSTGRES_DB=hydra -d postgres:9.6 # Runs tests in short mode, without database adapters -.PHONY: test-short -test-short: +.PHONY: quicktest +quicktest: go test -failfast -short ./... # Formats the code diff --git a/consent/manager_test.go b/consent/manager_test.go index ee60a03a164..8e9fd11f178 100644 --- a/consent/manager_test.go +++ b/consent/manager_test.go @@ -26,9 +26,6 @@ import ( "testing" "time" - "github.com/ory/hydra/client" - "github.com/ory/hydra/oauth2" - "github.com/jmoiron/sqlx" "github.com/spf13/viper" @@ -74,16 +71,7 @@ func TestMain(m *testing.M) { func createSQL(db *sqlx.DB) driver.Registry { conf := internal.NewConfigurationWithDefaults() reg := internal.NewRegistrySQL(conf, db) - - if _, err := reg.ClientManager().(*client.SQLManager).CreateSchemas(); err != nil { - panic(err) - } - - if _, err := reg.ConsentManager().(*SQLManager).CreateSchemas(); err != nil { - panic(err) - } - - if _, err := reg.OAuth2Storage().(*oauth2.FositeSQLStore).CreateSchemas(); err != nil { + if _, err := reg.CreateSchemas(); err != nil { panic(err) } @@ -96,17 +84,16 @@ func TestManagers(t *testing.T) { regs["memory"] = internal.NewRegistry(conf) if !testing.Short() { + var p, m *sqlx.DB dockertest.Parallel([]func(){ func() { - m.Lock() - regs["postgres"] = createSQL(connectToPostgres(t)) - m.Unlock() + p = connectToPostgres(t) }, func() { - m.Lock() - regs["mysql"] = createSQL(connectToMySQL(t)) - m.Unlock() + m = connectToMySQL(t) }, }) + regs["postgres"] = createSQL(p) + regs["mysql"] = createSQL(m) } for k, m := range regs { diff --git a/driver/registry_sql.go b/driver/registry_sql.go index 58a54fe007d..9f27e778006 100644 --- a/driver/registry_sql.go +++ b/driver/registry_sql.go @@ -81,21 +81,22 @@ func (m *RegistrySQL) DB() *sqlx.DB { func (m *RegistrySQL) CreateSchemas() (int, error) { var total int - // Ensure dependencies exist - _, _, _, _ = m.ClientManager(), m.ConsentManager(), m.KeyManager(), m.OAuth2Storage() - - for _, s := range []schemaCreator{ - m.km.(schemaCreator), - m.cm.(schemaCreator), - m.com.(schemaCreator), - m.fs.(schemaCreator), + m.Logger().Debugf("Applying %s SQL migrations...", m.db.DriverName()) + for k, s := range []schemaCreator{ + m.KeyManager().(schemaCreator), + m.ClientManager().(schemaCreator), + m.ConsentManager().(schemaCreator), + m.OAuth2Storage().(schemaCreator), } { + m.Logger().Debugf("Applying %s SQL migrations for manager: %T (%d)", m.db.DriverName(), s, k) if c, err := s.CreateSchemas(); err != nil { return c, err } else { + m.Logger().Debugf("Successfully applied %d %s SQL migrations from manager: %T (%d)", c, m.db.DriverName(), s, k) total += c } } + m.Logger().Debugf("Applied %d %s SQL migrations", total, m.db.DriverName()) return total, nil } diff --git a/oauth2/fosite_store_test.go b/oauth2/fosite_store_test.go index dbe47ef554c..c3dd573f7b2 100644 --- a/oauth2/fosite_store_test.go +++ b/oauth2/fosite_store_test.go @@ -23,7 +23,6 @@ package oauth2_test import ( "context" "flag" - "sync" "testing" "github.com/ory/hydra/x" @@ -34,7 +33,6 @@ import ( "github.com/stretchr/testify/require" "github.com/ory/hydra/client" - "github.com/ory/hydra/consent" "github.com/ory/hydra/driver/configuration" "github.com/ory/hydra/driver" @@ -46,8 +44,6 @@ import ( var registries = make(map[string]driver.Registry) -var m sync.Mutex - func TestMain(m *testing.M) { flag.Parse() runner := dockertest.Register() @@ -70,11 +66,7 @@ func connectToMySQL(t *testing.T) *sqlx.DB { func connectSQL(t *testing.T, conf *configuration.ViperProvider, db *sqlx.DB) driver.Registry { reg := internal.NewRegistrySQL(conf, db) - _, err := reg.ClientManager().(*client.SQLManager).CreateSchemas() - require.NoError(t, err) - _, err = reg.ConsentManager().(*consent.SQLManager).CreateSchemas() - require.NoError(t, err) - _, err = reg.OAuth2Storage().(*FositeSQLStore).CreateSchemas() + _, err := reg.CreateSchemas() require.NoError(t, err) return reg } @@ -87,19 +79,17 @@ func TestManagers(t *testing.T) { registries["memory"] = reg if !testing.Short() { + var p, m *sqlx.DB dockertest.Parallel([]func(){ func() { - m.Lock() - - registries["postgres"] = connectSQL(t, conf, connectToPG(t)) - m.Unlock() + p = connectToPG(t) }, func() { - m.Lock() - registries["mysql"] = connectSQL(t, conf, connectToMySQL(t)) - m.Unlock() + m = connectToMySQL(t) }, }) + registries["postgres"] = connectSQL(t, conf, p) + registries["mysql"] = connectSQL(t, conf, m) } for k, store := range registries { diff --git a/oauth2/oauth2_auth_code_test.go b/oauth2/oauth2_auth_code_test.go index 2beed6ef1fd..4756356dd1c 100644 --- a/oauth2/oauth2_auth_code_test.go +++ b/oauth2/oauth2_auth_code_test.go @@ -35,6 +35,8 @@ import ( "testing" "time" + "github.com/jmoiron/sqlx" + "github.com/ory/hydra/x" "github.com/ory/x/sqlcon/dockertest" @@ -96,33 +98,30 @@ type clientCreator interface { // - [x] If `id_token_hint` is handled properly // - [x] What happens if `id_token_hint` does not match the value from the handled authentication request ("accept login") func TestAuthCodeWithDefaultStrategy(t *testing.T) { - var mutex sync.Mutex conf := internal.NewConfigurationWithDefaults() regs := map[string]driver.Registry{ "memory": internal.NewRegistry(conf), } if !testing.Short() { + var p, m *sqlx.DB dockertest.Parallel([]func(){ func() { - r := internal.NewRegistrySQL(conf, connectToPG(t)) - _, err := r.CreateSchemas() - require.NoError(t, err) - - mutex.Lock() - regs["postgres"] = r - mutex.Unlock() + p = connectToPG(t) }, func() { - r := internal.NewRegistrySQL(conf, connectToMySQL(t)) - _, err := r.CreateSchemas() - require.NoError(t, err) - - mutex.Lock() - regs["mysql"] = r - mutex.Unlock() + m = connectToMySQL(t) }, }) + pr := internal.NewRegistrySQL(conf, p) + _, err := pr.CreateSchemas() + require.NoError(t, err) + regs["postgres"] = pr + + mr := internal.NewRegistrySQL(conf, m) + _, err = mr.CreateSchemas() + require.NoError(t, err) + regs["mysql"] = mr } for km, reg := range regs {