Merge pull request juju#17124 from SimonRichardson/include-sql-statem…

…ents

juju#17124

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 spews out what it ran and how it got stuck.

This only works with super verbose mode i.e. check.vv.

It will also dump out valid SQL now, so you can dump it into a db without modifications.

## Checklist

<!-- If an item is not applicable, use `~strikethrough~`. -->

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing

## QA steps

```sh
$ INCLUDE_SQL_OUTPUT=1 go test -v ./domain/schema -check.vv
```

## Links

**Jira card:** JUJU-

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)
 		}
 

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

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 }

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)

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
 }

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)

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,
 	})
 }
 

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())

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,
 	})
 }
 

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)
 

internal/worker/changestreampruner/package_test.go

@@ -41,15 +41,17 @@ 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,
 	})
 }
 
 // ApplyDDLForRunner is responsible for applying the controller schema to the
 // 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)
 }