fix test race condition and remove verbose global in parser (#457)

pull/462/head
Michael Fridman 2023-01-28 10:41:44 -05:00 committed by GitHub
parent c2f9bcbc80
commit b4af752f57
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 97 additions and 73 deletions

View File

@ -11,6 +11,20 @@ import (
"sync"
)
type Direction string
const (
DirectionUp Direction = "up"
DirectionDown Direction = "down"
)
func FromBool(b bool) Direction {
if b {
return DirectionUp
}
return DirectionDown
}
type parserState int
const (
@ -23,15 +37,37 @@ const (
gooseStatementEndDown // 6
)
type stateMachine parserState
func (s *stateMachine) Get() parserState {
return parserState(*s)
type stateMachine struct {
state parserState
verbose bool
}
func (s *stateMachine) Set(new parserState) {
verboseInfo("StateMachine: %v => %v", *s, new)
*s = stateMachine(new)
func newStateMachine(begin parserState, verbose bool) *stateMachine {
return &stateMachine{
state: begin,
verbose: verbose,
}
}
func (s *stateMachine) get() parserState {
return s.state
}
func (s *stateMachine) set(new parserState) {
s.print("set %d => %d", s.state, new)
s.state = new
}
const (
grayColor = "\033[90m"
resetColor = "\033[00m"
)
func (s *stateMachine) print(msg string, args ...interface{}) {
msg = "StateMachine: " + msg
if s.verbose {
log.Printf(grayColor+msg+resetColor, args...)
}
}
const scanBufSize = 4 * 1024 * 1024
@ -53,7 +89,7 @@ var bufferPool = sync.Pool{
// within a statement. For these cases, we provide the explicit annotations
// 'StatementBegin' and 'StatementEnd' to allow the script to
// tell us to ignore semicolons.
func ParseSQLMigration(r io.Reader, direction bool) (stmts []string, useTx bool, err error) {
func ParseSQLMigration(r io.Reader, direction Direction, verbose bool) (stmts []string, useTx bool, err error) {
scanBufPtr := bufferPool.Get().(*[]byte)
scanBuf := *scanBufPtr
defer bufferPool.Put(scanBufPtr)
@ -61,7 +97,7 @@ func ParseSQLMigration(r io.Reader, direction bool) (stmts []string, useTx bool,
scanner := bufio.NewScanner(r)
scanner.Buffer(scanBuf, scanBufSize)
stateMachine := stateMachine(start)
stateMachine := newStateMachine(start, verbose)
useTx = true
var buf bytes.Buffer
@ -70,7 +106,7 @@ func ParseSQLMigration(r io.Reader, direction bool) (stmts []string, useTx bool,
if verbose {
log.Println(line)
}
if stateMachine.Get() == start && strings.TrimSpace(line) == "" {
if stateMachine.get() == start && strings.TrimSpace(line) == "" {
continue
}
// TODO(mf): validate annotations to avoid common user errors:
@ -80,40 +116,40 @@ func ParseSQLMigration(r io.Reader, direction bool) (stmts []string, useTx bool,
switch cmd {
case "+goose Up":
switch stateMachine.Get() {
switch stateMachine.get() {
case start:
stateMachine.Set(gooseUp)
stateMachine.set(gooseUp)
default:
return nil, false, fmt.Errorf("duplicate '-- +goose Up' annotations; stateMachine=%v, see https://github.com/pressly/goose#sql-migrations", stateMachine)
return nil, false, fmt.Errorf("duplicate '-- +goose Up' annotations; stateMachine=%d, see https://github.com/pressly/goose#sql-migrations", stateMachine.state)
}
continue
case "+goose Down":
switch stateMachine.Get() {
switch stateMachine.get() {
case gooseUp, gooseStatementEndUp:
stateMachine.Set(gooseDown)
stateMachine.set(gooseDown)
default:
return nil, false, fmt.Errorf("must start with '-- +goose Up' annotation, stateMachine=%v, see https://github.com/pressly/goose#sql-migrations", stateMachine)
return nil, false, fmt.Errorf("must start with '-- +goose Up' annotation, stateMachine=%d, see https://github.com/pressly/goose#sql-migrations", stateMachine.state)
}
continue
case "+goose StatementBegin":
switch stateMachine.Get() {
switch stateMachine.get() {
case gooseUp, gooseStatementEndUp:
stateMachine.Set(gooseStatementBeginUp)
stateMachine.set(gooseStatementBeginUp)
case gooseDown, gooseStatementEndDown:
stateMachine.Set(gooseStatementBeginDown)
stateMachine.set(gooseStatementBeginDown)
default:
return nil, false, fmt.Errorf("'-- +goose StatementBegin' must be defined after '-- +goose Up' or '-- +goose Down' annotation, stateMachine=%v, see https://github.com/pressly/goose#sql-migrations", stateMachine)
return nil, false, fmt.Errorf("'-- +goose StatementBegin' must be defined after '-- +goose Up' or '-- +goose Down' annotation, stateMachine=%d, see https://github.com/pressly/goose#sql-migrations", stateMachine.state)
}
continue
case "+goose StatementEnd":
switch stateMachine.Get() {
switch stateMachine.get() {
case gooseStatementBeginUp:
stateMachine.Set(gooseStatementEndUp)
stateMachine.set(gooseStatementEndUp)
case gooseStatementBeginDown:
stateMachine.Set(gooseStatementEndDown)
stateMachine.set(gooseStatementEndDown)
default:
return nil, false, errors.New("'-- +goose StatementEnd' must be defined after '-- +goose StatementBegin', see https://github.com/pressly/goose#sql-migrations")
}
@ -129,11 +165,11 @@ func ParseSQLMigration(r io.Reader, direction bool) (stmts []string, useTx bool,
if buf.Len() == 0 {
// This check ensures leading comments and empty lines prior to a statement are ignored.
if strings.HasPrefix(strings.TrimSpace(line), "--") || line == "" {
verboseInfo("StateMachine: ignore comment")
stateMachine.print("ignore comment")
continue
}
}
switch stateMachine.Get() {
switch stateMachine.get() {
case gooseStatementEndDown, gooseStatementEndUp:
// Do not include the "+goose StatementEnd" annotation in the final statement.
default:
@ -147,46 +183,46 @@ func ParseSQLMigration(r io.Reader, direction bool) (stmts []string, useTx bool,
// 1) basic query with semicolon; 2) psql statement
//
// Export statement once we hit end of statement.
switch stateMachine.Get() {
switch stateMachine.get() {
case gooseUp, gooseStatementBeginUp, gooseStatementEndUp:
if !direction /*down*/ {
if direction == DirectionDown {
buf.Reset()
verboseInfo("StateMachine: ignore down")
stateMachine.print("ignore down")
continue
}
case gooseDown, gooseStatementBeginDown, gooseStatementEndDown:
if direction /*up*/ {
if direction == DirectionUp {
buf.Reset()
verboseInfo("StateMachine: ignore up")
stateMachine.print("ignore up")
continue
}
default:
return nil, false, fmt.Errorf("failed to parse migration: unexpected state %d on line %q, see https://github.com/pressly/goose#sql-migrations", stateMachine, line)
return nil, false, fmt.Errorf("failed to parse migration: unexpected state %d on line %q, see https://github.com/pressly/goose#sql-migrations", stateMachine.state, line)
}
switch stateMachine.Get() {
switch stateMachine.get() {
case gooseUp:
if endsWithSemicolon(line) {
stmts = append(stmts, cleanupStatement(buf.String()))
buf.Reset()
verboseInfo("StateMachine: store simple Up query")
stateMachine.print("store simple Up query")
}
case gooseDown:
if endsWithSemicolon(line) {
stmts = append(stmts, cleanupStatement(buf.String()))
buf.Reset()
verboseInfo("StateMachine: store simple Down query")
stateMachine.print("store simple Down query")
}
case gooseStatementEndUp:
stmts = append(stmts, cleanupStatement(buf.String()))
buf.Reset()
verboseInfo("StateMachine: store Up statement")
stateMachine.Set(gooseUp)
stateMachine.print("store Up statement")
stateMachine.set(gooseUp)
case gooseStatementEndDown:
stmts = append(stmts, cleanupStatement(buf.String()))
buf.Reset()
verboseInfo("StateMachine: store Down statement")
stateMachine.Set(gooseDown)
stateMachine.print("store Down statement")
stateMachine.set(gooseDown)
}
}
if err := scanner.Err(); err != nil {
@ -194,7 +230,7 @@ func ParseSQLMigration(r io.Reader, direction bool) (stmts []string, useTx bool,
}
// EOF
switch stateMachine.Get() {
switch stateMachine.get() {
case start:
return nil, false, errors.New("failed to parse migration: must start with '-- +goose Up' annotation, see https://github.com/pressly/goose#sql-migrations")
case gooseStatementBeginUp, gooseStatementBeginDown:
@ -202,7 +238,7 @@ func ParseSQLMigration(r io.Reader, direction bool) (stmts []string, useTx bool,
}
if bufferRemaining := strings.TrimSpace(buf.String()); len(bufferRemaining) > 0 {
return nil, false, fmt.Errorf("failed to parse migration: state %d, direction: %v: unexpected unfinished SQL query: %q: missing semicolon?", stateMachine, direction, bufferRemaining)
return nil, false, fmt.Errorf("failed to parse migration: state %d, direction: %v: unexpected unfinished SQL query: %q: missing semicolon?", stateMachine.state, direction, bufferRemaining)
}
return stmts, useTx, nil
@ -240,20 +276,3 @@ func endsWithSemicolon(line string) bool {
return strings.HasSuffix(prev, ";")
}
var verbose bool
func SetVersbose(b bool) {
verbose = b
}
const (
grayColor = "\033[90m"
resetColor = "\033[00m"
)
func verboseInfo(s string, args ...interface{}) {
if verbose {
log.Printf(grayColor+s+resetColor, args...)
}
}

View File

@ -11,6 +11,15 @@ import (
"github.com/pressly/goose/v3/internal/check"
)
var (
debug = false
)
func TestMain(m *testing.M) {
debug, _ = strconv.ParseBool(os.Getenv("DEBUG_TEST"))
os.Exit(m.Run())
}
func TestSemicolons(t *testing.T) {
t.Parallel()
@ -38,7 +47,6 @@ func TestSemicolons(t *testing.T) {
func TestSplitStatements(t *testing.T) {
t.Parallel()
// SetVerbose(true)
type testData struct {
sql string
@ -59,7 +67,7 @@ func TestSplitStatements(t *testing.T) {
for i, test := range tt {
// up
stmts, _, err := ParseSQLMigration(strings.NewReader(test.sql), true)
stmts, _, err := ParseSQLMigration(strings.NewReader(test.sql), DirectionUp, debug)
if err != nil {
t.Error(fmt.Errorf("tt[%v] unexpected error: %w", i, err))
}
@ -68,7 +76,7 @@ func TestSplitStatements(t *testing.T) {
}
// down
stmts, _, err = ParseSQLMigration(strings.NewReader(test.sql), false)
stmts, _, err = ParseSQLMigration(strings.NewReader(test.sql), DirectionDown, debug)
if err != nil {
t.Error(fmt.Errorf("tt[%v] unexpected error: %w", i, err))
}
@ -97,7 +105,7 @@ func TestUseTransactions(t *testing.T) {
if err != nil {
t.Error(err)
}
_, useTx, err := ParseSQLMigration(f, true)
_, useTx, err := ParseSQLMigration(f, DirectionUp, debug)
if err != nil {
t.Error(err)
}
@ -117,7 +125,7 @@ func TestParsingErrors(t *testing.T) {
downFirst,
}
for i, sql := range tt {
_, _, err := ParseSQLMigration(strings.NewReader(sql), true)
_, _, err := ParseSQLMigration(strings.NewReader(sql), DirectionUp, debug)
if err == nil {
t.Errorf("expected error on tt[%v] %q", i, sql)
}
@ -386,7 +394,7 @@ func testValidUp(t *testing.T, dir string, count int) {
f, err := os.Open(filepath.Join(dir, "input.sql"))
check.NoError(t, err)
t.Cleanup(func() { f.Close() })
statements, _, err := ParseSQLMigration(f, true)
statements, _, err := ParseSQLMigration(f, DirectionUp, debug)
check.NoError(t, err)
check.Number(t, len(statements), count)
compareStatements(t, dir, statements)

View File

@ -61,8 +61,7 @@ func (m *Migration) run(db *sql.DB, direction bool) error {
}
defer f.Close()
sqlparser.SetVersbose(verbose)
statements, useTx, err := sqlparser.ParseSQLMigration(f, direction)
statements, useTx, err := sqlparser.ParseSQLMigration(f, sqlparser.FromBool(direction), verbose)
if err != nil {
return fmt.Errorf("ERROR %v: failed to parse SQL migration file: %w", filepath.Base(m.Source), err)
}

View File

@ -321,8 +321,6 @@ func setupTestDB(t *testing.T, version int64) *sql.DB {
db, err := newDockerDB(t)
check.NoError(t, err)
check.NoError(t, goose.SetDialect(*dialect))
// Create goose table.
current, err := goose.EnsureDBVersion(db)
check.NoError(t, err)

View File

@ -11,6 +11,7 @@ import (
"syscall"
"testing"
"github.com/pressly/goose/v3"
"github.com/pressly/goose/v3/internal/check"
"github.com/pressly/goose/v3/internal/testdb"
)
@ -73,6 +74,11 @@ func TestMain(m *testing.M) {
migrationsDir = filepath.Join("testdata", *dialect, "migrations")
seedDir = filepath.Join("testdata", *dialect, "seed")
if err := goose.SetDialect(*dialect); err != nil {
log.Printf("failed to set dialect %q: %v\n", *dialect, err)
os.Exit(1)
}
exitCode := m.Run()
// Useful for debugging test services.
if *debug {

View File

@ -17,7 +17,6 @@ func TestMigrateUpWithReset(t *testing.T) {
db, err := newDockerDB(t)
check.NoError(t, err)
check.NoError(t, goose.SetDialect(*dialect))
migrations, err := goose.CollectMigrations(migrationsDir, 0, goose.MaxVersion)
check.NoError(t, err)
check.NumberNotZero(t, len(migrations))
@ -46,7 +45,6 @@ func TestMigrateUpWithRedo(t *testing.T) {
db, err := newDockerDB(t)
check.NoError(t, err)
check.NoError(t, goose.SetDialect(*dialect))
migrations, err := goose.CollectMigrations(migrationsDir, 0, goose.MaxVersion)
check.NoError(t, err)
@ -84,7 +82,6 @@ func TestMigrateUpTo(t *testing.T) {
)
db, err := newDockerDB(t)
check.NoError(t, err)
check.NoError(t, goose.SetDialect(*dialect))
migrations, err := goose.CollectMigrations(migrationsDir, 0, goose.MaxVersion)
check.NoError(t, err)
check.NumberNotZero(t, len(migrations))
@ -106,7 +103,6 @@ func TestMigrateUpByOne(t *testing.T) {
db, err := newDockerDB(t)
check.NoError(t, err)
check.NoError(t, goose.SetDialect(*dialect))
migrations, err := goose.CollectMigrations(migrationsDir, 0, goose.MaxVersion)
check.NoError(t, err)
check.NumberNotZero(t, len(migrations))
@ -137,7 +133,6 @@ func TestMigrateFull(t *testing.T) {
db, err := newDockerDB(t)
check.NoError(t, err)
check.NoError(t, goose.SetDialect(*dialect))
migrations, err := goose.CollectMigrations(migrationsDir, 0, goose.MaxVersion)
check.NoError(t, err)
check.NumberNotZero(t, len(migrations))

View File

@ -20,7 +20,6 @@ func TestNoVersioning(t *testing.T) {
)
db, err := newDockerDB(t)
check.NoError(t, err)
check.NoError(t, goose.SetDialect(*dialect))
err = goose.Up(db, migrationsDir)
check.NoError(t, err)