Add basic test for hooks

Author: timvaillancourt

go/logic/hooks.go

@@ -7,6 +7,7 @@ package logic
 
 import (
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -42,10 +43,6 @@ func NewHooksExecutor(migrationContext *base.MigrationContext) *HooksExecutor {
 	}
 }
 
-func (this *HooksExecutor) initHooks() error {
-	return nil
-}
-
 func (this *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) []string {
 	env := os.Environ()
 	env = append(env, fmt.Sprintf("GH_OST_DATABASE_NAME=%s", this.migrationContext.DatabaseName))
@@ -76,13 +73,13 @@ func (this *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) [
 }
 
 // executeHook executes a command, and sets relevant environment variables
-// combined output & error are printed to gh-ost's standard error.
-func (this *HooksExecutor) executeHook(hook string, extraVariables ...string) error {
+// combined output & error are printed to the provided io.Writer.
+func (this *HooksExecutor) executeHook(out io.Writer, hook string, extraVariables ...string) error {
 	cmd := exec.Command(hook)
 	cmd.Env = this.applyEnvironmentVariables(extraVariables...)
 
 	combinedOutput, err := cmd.CombinedOutput()
-	fmt.Fprintln(os.Stderr, string(combinedOutput))
+	fmt.Fprintln(out, string(combinedOutput))
 	return log.Errore(err)
 }
 
@@ -95,69 +92,69 @@ func (this *HooksExecutor) detectHooks(baseName string) (hooks []string, err err
 	return hooks, err
 }
 
-func (this *HooksExecutor) executeHooks(baseName string, extraVariables ...string) error {
+func (this *HooksExecutor) executeHooks(out io.Writer, baseName string, extraVariables ...string) error {
 	hooks, err := this.detectHooks(baseName)
 	if err != nil {
 		return err
 	}
 	for _, hook := range hooks {
 		log.Infof("executing %+v hook: %+v", baseName, hook)
-		if err := this.executeHook(hook, extraVariables...); err != nil {
+		if err := this.executeHook(out, hook, extraVariables...); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
 func (this *HooksExecutor) onStartup() error {
-	return this.executeHooks(onStartup)
+	return this.executeHooks(os.Stderr, onStartup)
 }
 
 func (this *HooksExecutor) onValidated() error {
-	return this.executeHooks(onValidated)
+	return this.executeHooks(os.Stderr, onValidated)
 }
 
 func (this *HooksExecutor) onRowCountComplete() error {
-	return this.executeHooks(onRowCountComplete)
+	return this.executeHooks(os.Stderr, onRowCountComplete)
 }
 func (this *HooksExecutor) onBeforeRowCopy() error {
-	return this.executeHooks(onBeforeRowCopy)
+	return this.executeHooks(os.Stderr, onBeforeRowCopy)
 }
 
 func (this *HooksExecutor) onRowCopyComplete() error {
-	return this.executeHooks(onRowCopyComplete)
+	return this.executeHooks(os.Stderr, onRowCopyComplete)
 }
 
 func (this *HooksExecutor) onBeginPostponed() error {
-	return this.executeHooks(onBeginPostponed)
+	return this.executeHooks(os.Stderr, onBeginPostponed)
 }
 
 func (this *HooksExecutor) onBeforeCutOver() error {
-	return this.executeHooks(onBeforeCutOver)
+	return this.executeHooks(os.Stderr, onBeforeCutOver)
 }
 
 func (this *HooksExecutor) onInteractiveCommand(command string) error {
 	v := fmt.Sprintf("GH_OST_COMMAND='%s'", command)
-	return this.executeHooks(onInteractiveCommand, v)
+	return this.executeHooks(os.Stderr, onInteractiveCommand, v)
 }
 
 func (this *HooksExecutor) onSuccess() error {
-	return this.executeHooks(onSuccess)
+	return this.executeHooks(os.Stderr, onSuccess)
 }
 
 func (this *HooksExecutor) onFailure() error {
-	return this.executeHooks(onFailure)
+	return this.executeHooks(os.Stderr, onFailure)
 }
 
 func (this *HooksExecutor) onStatus(statusMessage string) error {
 	v := fmt.Sprintf("GH_OST_STATUS='%s'", statusMessage)
-	return this.executeHooks(onStatus, v)
+	return this.executeHooks(os.Stderr, onStatus, v)
 }
 
 func (this *HooksExecutor) onStopReplication() error {
-	return this.executeHooks(onStopReplication)
+	return this.executeHooks(os.Stderr, onStopReplication)
 }
 
 func (this *HooksExecutor) onStartReplication() error {
-	return this.executeHooks(onStartReplication)
+	return this.executeHooks(os.Stderr, onStartReplication)
 }

go/logic/hooks_test.go

@@ -0,0 +1,80 @@
+/*
+   Copyright 2022 GitHub Inc.
+         See https://github.com/github/gh-ost/blob/master/LICENSE
+*/
+
+package logic
+
+import (
+	"bufio"
+	"bytes"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/openark/golib/tests"
+
+	"github.com/github/gh-ost/go/base"
+)
+
+func TestHooksExecutorExecuteHooks(t *testing.T) {
+	migrationContext := base.NewMigrationContext()
+	migrationContext.AlterStatement = "ENGINE=InnoDB"
+	migrationContext.DatabaseName = "test"
+	migrationContext.OriginalTableName = "tablename"
+	hooksExecutor := NewHooksExecutor(migrationContext)
+
+	t.Run("does-not-exist", func(t *testing.T) {
+		migrationContext.HooksPath = "/does/not/exist"
+		tests.S(t).ExpectNil(hooksExecutor.executeHooks(os.Stderr, "test-hook"))
+	})
+
+	t.Run("failed", func(t *testing.T) {
+		var err error
+		if migrationContext.HooksPath, err = os.MkdirTemp("", "TestHooksExecutorExecuteHooks-failed"); err != nil {
+			panic(err)
+		}
+		defer os.RemoveAll(migrationContext.HooksPath)
+
+		// write hook that fails with 'exit 1'
+		hookFile := filepath.Join(migrationContext.HooksPath, "failed-hook")
+		os.WriteFile(hookFile, []byte("#!/bin/sh\nexit 1"), 0777)
+		tests.S(t).ExpectNotNil(hooksExecutor.executeHooks(os.Stderr, "failed-hook"))
+	})
+
+	t.Run("success", func(t *testing.T) {
+		var err error
+		if migrationContext.HooksPath, err = os.MkdirTemp("", "TestHooksExecutorExecuteHooks-success"); err != nil {
+			panic(err)
+		}
+		defer os.RemoveAll(migrationContext.HooksPath)
+
+		// write hook that prints the environment with 'env'
+		hookFile := filepath.Join(migrationContext.HooksPath, "success-hook")
+		os.WriteFile(hookFile, []byte("#!/bin/sh\nenv"), 0777)
+
+		// check output
+		var buf bytes.Buffer
+		tests.S(t).ExpectNil(hooksExecutor.executeHooks(&buf, "success-hook", "TEST="+t.Name()))
+		scanner := bufio.NewScanner(&buf)
+		for scanner.Scan() {
+			split := strings.SplitN(scanner.Text(), "=", 2)
+			switch split[0] {
+			case "GH_OST_DATABASE_NAME":
+				tests.S(t).ExpectEquals(split[1], migrationContext.DatabaseName)
+			case "GH_OST_DDL":
+				tests.S(t).ExpectEquals(split[1], migrationContext.AlterStatement)
+			case "GH_OST_TABLE_NAME":
+				tests.S(t).ExpectEquals(split[1], migrationContext.OriginalTableName)
+			case "GH_OST_OLD_TABLE_NAME":
+				tests.S(t).ExpectEquals(split[1], fmt.Sprintf("_%s_del", migrationContext.OriginalTableName))
+			case "GH_OST_GHOST_TABLE_NAME":
+				tests.S(t).ExpectEquals(split[1], fmt.Sprintf("_%s_gho", migrationContext.OriginalTableName))
+			case "TEST":
+				tests.S(t).ExpectEquals(split[1], t.Name())
+			}
+		}
+	})
+}

go/logic/migrator.go

@@ -98,6 +98,7 @@ type Migrator struct {
 func NewMigrator(context *base.MigrationContext, appVersion string) *Migrator {
 	migrator := &Migrator{
 		appVersion:                 appVersion,
+		hooksExecutor:              NewHooksExecutor(context),
 		migrationContext:           context,
 		parser:                     sql.NewAlterTableParser(),
 		ghostTableMigrated:         make(chan bool),
@@ -113,15 +114,6 @@ func NewMigrator(context *base.MigrationContext, appVersion string) *Migrator {
 	return migrator
 }
 
-// initiateHooksExecutor
-func (this *Migrator) initiateHooksExecutor() (err error) {
-	this.hooksExecutor = NewHooksExecutor(this.migrationContext)
-	if err := this.hooksExecutor.initHooks(); err != nil {
-		return err
-	}
-	return nil
-}
-
 // sleepWhileTrue sleeps indefinitely until the given function returns 'false'
 // (or fails with error)
 func (this *Migrator) sleepWhileTrue(operation func() (bool, error)) error {
@@ -342,9 +334,6 @@ func (this *Migrator) Migrate() (err error) {
 
 	go this.listenOnPanicAbort()
 
-	if err := this.initiateHooksExecutor(); err != nil {
-		return err
-	}
 	if err := this.hooksExecutor.onStartup(); err != nil {
 		return err
 	}

script/test

@@ -14,4 +14,4 @@ script/build
 cd .gopath/src/github.com/github/gh-ost
 
 echo "Running unit tests"
-go test ./go/...
+go test -v -covermode=atomic ./go/...