increase test coverage (#18)

* add circular dependency test

* a bit more coverage

* adjust how coverage is calculated to include top level

* OpenAnyDB() can now handle "postgresql" DSNs

* add description and test coverage for SkipRemainingIf

* coverage tweaks in Makefile

* breaking change: fix spelling of Asynchronous

* bugfix: async migrations were not unlocking the migrations when finished

.github/workflows/codecov.yml

@@ -13,7 +13,7 @@ jobs:
         with:
           go-version: '1.17'
       - name: Run coverage
-        run: go test ./... -race -coverprofile=coverage.txt -covermode=atomic 
+        run: make calculate_coverage
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v2

.github/workflows/mysql.yml

@@ -39,7 +39,7 @@ jobs:
       - name: Run Coverage
         env:
           LIBSCHEMA_MYSQL_TEST_DSN: "root:mysql@tcp(127.0.0.1:3306)/libschematest?tls=false"
-        run: go test -coverprofile=coverage.txt -covermode=atomic ./lsmysql/... 
+        run: go test -coverprofile=coverage.txt -covermode=atomic -coverpkg=github.com/muir/libschema/... ./lsmysql/... 
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v2

.github/workflows/pg.yml

@@ -38,7 +38,7 @@ jobs:
       - name: Run Coverage
         env:
           LIBSCHEMA_POSTGRES_TEST_DSN: "postgres://postgres:postgres@localhost?sslmode=disable"
-        run: go test -coverprofile=coverage.txt -covermode=atomic ./lspostgres/... 
+        run: go test -coverprofile=coverage.txt -covermode=atomic -coverpkg=github.com/muir/libschema/... ./lspostgres/... 
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v2

Makefile

@@ -0,0 +1,30 @@
+
+
+all:
+	go install golang.org/x/tools/...@latest
+	go generate
+	go test
+	golangci-lint run
+
+coverageO:
+	go install github.com/eltorocorp/drygopher/drygopher@latest
+	-drygopher -s 80
+	go tool cover -html=coverage.out
+
+calculate_coverage:
+	echo "mode: atomic" > coverage.txt
+	for d in $$(go list ./...); do \
+	  go test -race -covermode=atomic -coverprofile=profile.out -coverpkg=github.com/muir/libschema/... $$d; \
+	  if [ -f profile.out ]; then \
+	    grep -v ^mode profile.out >> coverage.txt; \
+	    rm profile.out; \
+	  fi; \
+	done
+
+coverage: calculate_coverage
+	go tool cover -html=coverage.txt
+
+golanglint:
+	# binary will be $(go env GOPATH)/bin/golangci-lint
+	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $$(go env GOPATH)/bin v1.45.2
+	golangci-lint --version

api.go

@@ -177,10 +177,10 @@ func (s *Schema) NewDatabase(log MyLogger, name string, db *sql.DB, driver Drive
 	return database, nil
 }
 
-// Asyncrhronous marks a migration is okay to run asynchronously.  If all of the
+// Asynchronous marks a migration is okay to run asynchronously.  If all of the
 // remaining migrations can be asynchronous, then schema.Migrate() will return
 // while the remaining migrations run.
-func Asyncrhronous() MigrationOption {
+func Asynchronous() MigrationOption {
 	return func(m Migration) {
 		m.Base().async = true
 	}
@@ -203,12 +203,24 @@ func After(lib, migration string) MigrationOption {
 	}
 }
 
+// SkipIf is checked before the migration is run.  If the function returns true
+// then this migration is skipped.  For MySQL, this allows migrations
+// that are not idempotent to be checked before they're run and skipped
+// if they have already been applied.
 func SkipIf(pred func() (bool, error)) MigrationOption {
 	return func(m Migration) {
 		m.Base().skipIf = pred
 	}
 }
 
+// SkipRemainingIf is checked before the migration is run.  If the function
+// returns true then this migration and all following it are not run at this
+// time.  One use for this to hold back migrations that have not been released
+// yet.  For example, in a blue-green deploy organization, you could first
+// do a migration that creates another column, then later do a migration that
+// removes the old column.  The migration to remove the old column can be defined
+// and tested in advance but held back by SkipRemainingIf until it's time to
+// deploy it.
 func SkipRemainingIf(pred func() (bool, error)) MigrationOption {
 	return func(m Migration) {
 		m.Base().skipRemainingIf = pred

apply.go

@@ -277,6 +277,7 @@ func (d *Database) lastUnfinishedSynchrnous() int {
 	return -1
 }
 
+// allDone reports status
 func (d *Database) allDone(m Migration, err error) {
 	if err != nil && m != nil {
 		err = errors.Wrapf(err, "Migration %s", m.Base().Name)
@@ -299,8 +300,15 @@ func (d *Database) allDone(m Migration, err error) {
 func (d *Database) asyncMigrate(ctx context.Context) {
 	var err error
 	var m Migration
+	d.log.Info("Starting async migrations")
 	defer func() {
+		d.asyncInProgress = false
+		e := d.unlock()
+		if err == nil {
+			err = e
+		}
 		d.allDone(m, err)
+		d.log.Info("Done with async migrations")
 	}()
 	for _, m = range d.sequence {
 		if m.Base().Status().Done {

dgorder/dependencies_test.go

@@ -1,7 +1,6 @@
 package dgorder
 
 import (
-	"fmt"
 	"strconv"
 	"testing"
 
@@ -33,11 +32,34 @@ func TestDependenciesOkay(t *testing.T) {
 			},
 			want: []int{1, 0, 3, 4, 2, 5},
 		},
+		{
+			nodes: []Node{
+				{},
+				{
+					Blocking: []int{2},
+				},
+				{
+					Blocking: []int{1},
+				},
+				{},
+			},
+			want: nil,
+			err:  "Circular dependency found that includes 1 depending upon 2",
+		},
+		{
+			nodes: []Node{
+				{
+					Blocking: []int{22},
+				},
+			},
+			want: nil,
+			err:  "Invalid dependency from 0 to 22",
+		},
 	}
 	for _, tc := range cases {
 		got, err := Order(tc.nodes, strconv.Itoa)
 		if tc.err != "" {
-			assert.Equal(t, fmt.Errorf(tc.err), err)
+			assert.Equal(t, tc.err, err.Error())
 		} else {
 			assert.NoError(t, err)
 			assert.Equal(t, tc.want, got)

driver.go

@@ -6,14 +6,23 @@ import (
 	"strings"
 )
 
+// maps from DSN to driver name
+var driverAliases = map[string]string{
+	"postgresql": "postgres",
+}
+
 func OpenAnyDB(dsn string) (*sql.DB, error) {
-	for _, driver := range sql.Drivers() {
-		if strings.HasPrefix(dsn, driver+"://") {
-			return sql.Open(driver, dsn)
-		}
-	}
 	if i := strings.Index(dsn, "://"); i != -1 {
-		return nil, fmt.Errorf("Could not find database driver matching %s", dsn[:i])
+		wanted := dsn[0:i]
+		if alias, ok := driverAliases[wanted]; ok {
+			wanted = alias
+		}
+		for _, driver := range sql.Drivers() {
+			if driver == wanted {
+				return sql.Open(driver, dsn)
+			}
+		}
+		return nil, fmt.Errorf("Could not find database driver matching %s", wanted)
 	}
 	return nil, fmt.Errorf("Could not find appropriate database driver for DSN")
 }

fake_test.go

@@ -0,0 +1,11 @@
+package libschema_test
+
+import(
+	"testing"
+)
+
+// TestNothing exists so that there are at least some tests at the
+// libschema level. Without any tests, test coverage reports are not
+// generated.
+func TestNothing(t *testing.T) {
+}

go.mod

@@ -3,7 +3,7 @@ module github.com/muir/libschema
 go 1.16
 
 require (
-	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/eltorocorp/drygopher v1.1.2
 	github.com/go-sql-driver/mysql v1.6.0
 	github.com/lib/pq v1.10.5
 	github.com/muir/sqltoken v0.0.4