From 6dd0259029426790a33c3f370f5fef295030d4d0 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Thu, 28 Mar 2024 15:21:57 +0000 Subject: [PATCH] Include sql in verbose mode for tests Whilst debugging a test failure, including the SQL being executed helped diagnose the problem. Ideally, we would add a new flag for everyone to check against, but that requires a lot of reworking of the tests. Instead, for now we can just set an envvar whilst running the tests and it spew out what it ran and how it got stuck. --- core/database/schema/query.go | 2 +- core/database/schema/query_test.go | 2 +- core/database/schema/schema.go | 6 ++++-- domain/schema/schema_test.go | 10 ++++++---- domain/schema/testing/controllermodelsuite.go | 3 ++- domain/schema/testing/controllersuite.go | 6 ++++-- domain/schema/testing/modelsuite.go | 3 ++- domain/schema/testing/schema.go | 14 +++++++++----- internal/changestream/stream/package_test.go | 3 ++- internal/database/testing/dqlitesuite.go | 11 +++++++++++ internal/worker/changestreampruner/package_test.go | 6 ++++-- 11 files changed, 46 insertions(+), 20 deletions(-) diff --git a/core/database/schema/query.go b/core/database/schema/query.go index 890f9c071a5..278424633c3 100644 --- a/core/database/schema/query.go +++ b/core/database/schema/query.go @@ -133,7 +133,7 @@ func ensurePatchesAreApplied(ctx context.Context, tx *sql.Tx, current int, patch return errors.Trace(err) } - if err := hook(current); err != nil { + if err := hook(current, patch.stmt); err != nil { return errors.Annotatef(err, "failed to execute hook (version %d)", current) } diff --git a/core/database/schema/query_test.go b/core/database/schema/query_test.go index 71462dd2fe1..31c58c31e7a 100644 --- a/core/database/schema/query_test.go +++ b/core/database/schema/query_test.go @@ -108,7 +108,7 @@ func (s *querySuite) TestEnsurePatches(c *gc.C) { var called bool err := s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { - return ensurePatchesAreApplied(context.Background(), tx, 0, patches, func(i int) error { + return ensurePatchesAreApplied(context.Background(), tx, 0, patches, func(i int, statement string) error { called = true c.Check(i, gc.Equals, 0) return nil diff --git a/core/database/schema/schema.go b/core/database/schema/schema.go index e6b1d859c71..91bf4d501f7 100644 --- a/core/database/schema/schema.go +++ b/core/database/schema/schema.go @@ -29,6 +29,7 @@ type Schema struct { type Patch struct { run func(context.Context, Tx) error hash string + stmt string } // MakePatch returns a patch that applies the given SQL statement with the given @@ -40,11 +41,12 @@ func MakePatch(statement string, args ...any) Patch { return errors.Trace(err) }, hash: computeHash(statement), + stmt: statement, } } // Hook is a callback that gets fired when a update gets applied. -type Hook func(int) error +type Hook func(int, string) error // New creates a new schema Schema with the given patches. func New(patches ...Patch) *Schema { @@ -122,4 +124,4 @@ func (s *Schema) Ensure(ctx context.Context, runner database.TxnRunner) (ChangeS } // omitHook always returns a nil, omitting the error. -func omitHook(int) error { return nil } +func omitHook(int, string) error { return nil } diff --git a/domain/schema/schema_test.go b/domain/schema/schema_test.go index 1bce8a6da0a..beec56f020d 100644 --- a/domain/schema/schema_test.go +++ b/domain/schema/schema_test.go @@ -71,10 +71,12 @@ func readEntityNames(c *gc.C, db *sql.DB, entity_type string) []string { } func (s *schemaSuite) applyDDL(c *gc.C, ddl *schema.Schema) { - ddl.Hook(func(i int) error { - c.Log("Applying schema change", i) - return nil - }) + if s.Verbose { + ddl.Hook(func(i int, statement string) error { + c.Logf("-- Applying schema change %d\n%s\n", i, statement) + return nil + }) + } changeSet, err := ddl.Ensure(context.Background(), s.TxnRunner()) c.Assert(err, jc.ErrorIsNil) c.Check(changeSet.Current, gc.Equals, 0) diff --git a/domain/schema/testing/controllermodelsuite.go b/domain/schema/testing/controllermodelsuite.go index 155a6cafaa1..f790c58b105 100644 --- a/domain/schema/testing/controllermodelsuite.go +++ b/domain/schema/testing/controllermodelsuite.go @@ -22,7 +22,8 @@ type ControllerModelSuite struct { func (s *ControllerModelSuite) ModelTxnRunner(c *gc.C, modelUUID string) coredatabase.TxnRunner { txnRunner, _ := s.DqliteSuite.OpenDBForNamespace(c, modelUUID) s.DqliteSuite.ApplyDDLForRunner(c, &SchemaApplier{ - Schema: schema.ModelDDL(), + Schema: schema.ModelDDL(), + Verbose: s.Verbose, }, txnRunner) return txnRunner } diff --git a/domain/schema/testing/controllersuite.go b/domain/schema/testing/controllersuite.go index 053c047f9f1..29e705152a1 100644 --- a/domain/schema/testing/controllersuite.go +++ b/domain/schema/testing/controllersuite.go @@ -31,7 +31,8 @@ func (s *ControllerSuite) ControllerTxnRunner() coredatabase.TxnRunner { func (s *ControllerSuite) SetUpTest(c *gc.C) { s.DqliteSuite.SetUpTest(c) s.DqliteSuite.ApplyDDL(c, &SchemaApplier{ - Schema: schema.ControllerDDL(), + Schema: schema.ControllerDDL(), + Verbose: s.Verbose, }) err := database.InsertControllerNodeID(context.Background(), s.DqliteSuite.TxnRunner(), 0x2dc171858c3155be) c.Assert(err, jc.ErrorIsNil) @@ -41,7 +42,8 @@ func (s *ControllerSuite) SetUpTest(c *gc.C) { // given database. func (s *ControllerSuite) ApplyDDLForRunner(c *gc.C, runner coredatabase.TxnRunner) { s.DqliteSuite.ApplyDDLForRunner(c, &SchemaApplier{ - Schema: schema.ControllerDDL(), + Schema: schema.ControllerDDL(), + Verbose: s.Verbose, }, runner) err := database.InsertControllerNodeID(context.Background(), runner, 0x2dc171858c3155be) c.Assert(err, jc.ErrorIsNil) diff --git a/domain/schema/testing/modelsuite.go b/domain/schema/testing/modelsuite.go index 3b54fe421cb..cd9a9fd1f30 100644 --- a/domain/schema/testing/modelsuite.go +++ b/domain/schema/testing/modelsuite.go @@ -32,7 +32,8 @@ func (s *ModelSuite) SetUpTest(c *gc.C) { s.DqliteSuite.SetUpTest(c) s.DqliteSuite.ApplyDDL(c, &SchemaApplier{ - Schema: schema.ModelDDL(), + Schema: schema.ModelDDL(), + Verbose: s.Verbose, }) } diff --git a/domain/schema/testing/schema.go b/domain/schema/testing/schema.go index 99f8cbc0290..83d7b73cdcf 100644 --- a/domain/schema/testing/schema.go +++ b/domain/schema/testing/schema.go @@ -13,14 +13,18 @@ import ( ) type SchemaApplier struct { - Schema *schema.Schema + Schema *schema.Schema + Verbose bool } func (s *SchemaApplier) Apply(c *gc.C, ctx context.Context, runner database.TxnRunner) { - s.Schema.Hook(func(i int) error { - //c.Log("Applying schema change", i) - return nil - }) + if s.Verbose { + s.Schema.Hook(func(i int, statement string) error { + c.Logf("-- Applying schema change %d\n%s\n", i, statement) + return nil + }) + } + changeSet, err := s.Schema.Ensure(ctx, runner) c.Assert(err, gc.IsNil) c.Check(changeSet.Post, gc.Equals, s.Schema.Len()) diff --git a/internal/changestream/stream/package_test.go b/internal/changestream/stream/package_test.go index 1a1dddf6bee..ef1cb77d5d3 100644 --- a/internal/changestream/stream/package_test.go +++ b/internal/changestream/stream/package_test.go @@ -42,7 +42,8 @@ type baseSuite struct { func (s *baseSuite) SetUpTest(c *gc.C) { s.DqliteSuite.SetUpTest(c) s.DqliteSuite.ApplyDDL(c, &domaintesting.SchemaApplier{ - Schema: schema.ControllerDDL(), + Schema: schema.ControllerDDL(), + Verbose: s.Verbose, }) } diff --git a/internal/database/testing/dqlitesuite.go b/internal/database/testing/dqlitesuite.go index 889efabec92..4e30d05925b 100644 --- a/internal/database/testing/dqlitesuite.go +++ b/internal/database/testing/dqlitesuite.go @@ -29,6 +29,10 @@ import ( "github.com/juju/juju/internal/database/pragma" ) +// includeSQLOutput is used to enable the output of all SQL queries hitting the +// database. +var includeSQLOutput = os.Getenv("INCLUDE_SQL_OUTPUT") + // SchemaApplier is an interface that can be used to apply a schema to a // database. type SchemaApplier interface { @@ -41,6 +45,10 @@ type SchemaApplier interface { type DqliteSuite struct { testing.IsolationSuite + // Verbose indicates whether the suite should print all the sql + // hitting the db. + Verbose bool + dbPath string rootPath string uniqueID int64 @@ -97,6 +105,9 @@ func (s *DqliteSuite) SetUpTest(c *gc.C) { ) c.Assert(err, jc.ErrorIsNil) + // Enable super verbose mode. + s.Verbose = verbose && includeSQLOutput != "" + err = s.dqlite.Ready(context.Background()) c.Assert(err, jc.ErrorIsNil) diff --git a/internal/worker/changestreampruner/package_test.go b/internal/worker/changestreampruner/package_test.go index 4db23324628..0bc00883fab 100644 --- a/internal/worker/changestreampruner/package_test.go +++ b/internal/worker/changestreampruner/package_test.go @@ -41,7 +41,8 @@ type baseSuite struct { func (s *baseSuite) SetUpTest(c *gc.C) { s.DqliteSuite.SetUpTest(c) s.DqliteSuite.ApplyDDL(c, &domaintesting.SchemaApplier{ - Schema: schema.ControllerDDL(), + Schema: schema.ControllerDDL(), + Verbose: s.Verbose, }) } @@ -49,7 +50,8 @@ func (s *baseSuite) SetUpTest(c *gc.C) { // given database. func (s *baseSuite) ApplyDDLForRunner(c *gc.C, runner coredatabase.TxnRunner) { s.DqliteSuite.ApplyDDLForRunner(c, &domaintesting.SchemaApplier{ - Schema: schema.ControllerDDL(), + Schema: schema.ControllerDDL(), + Verbose: s.Verbose, }, runner) }