Skip to content

Commit 659e440

Browse files
dm-2dm-2
authored
Merge branch 'master' into master
2 parents 321e584 + df4cf7b commit 659e440

File tree

4 files changed

+120
-19
lines changed

4 files changed

+120
-19
lines changed

‎go/logic/hooks.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package logic
77

88
import (
99
"fmt"
10+
"io"
1011
"os"
1112
"os/exec"
1213
"path/filepath"
@@ -34,18 +35,16 @@ const (
3435

3536
type HooksExecutor struct {
3637
migrationContext *base.MigrationContext
38+
writer io.Writer
3739
}
3840

3941
func NewHooksExecutor(migrationContext *base.MigrationContext) *HooksExecutor {
4042
return &HooksExecutor{
4143
migrationContext: migrationContext,
44+
writer: os.Stderr,
4245
}
4346
}
4447

45-
func (this *HooksExecutor) initHooks() error {
46-
return nil
47-
}
48-
4948
func (this *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) []string {
5049
env := os.Environ()
5150
env = append(env, fmt.Sprintf("GH_OST_DATABASE_NAME=%s", this.migrationContext.DatabaseName))
@@ -76,13 +75,13 @@ func (this *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) [
7675
}
7776

7877
// executeHook executes a command, and sets relevant environment variables
79-
// combined output & error are printed to gh-ost's standard error.
78+
// combined output & error are printed to the configured writer.
8079
func (this *HooksExecutor) executeHook(hook string, extraVariables ...string) error {
8180
cmd := exec.Command(hook)
8281
cmd.Env = this.applyEnvironmentVariables(extraVariables...)
8382

8483
combinedOutput, err := cmd.CombinedOutput()
85-
fmt.Fprintln(os.Stderr, string(combinedOutput))
84+
fmt.Fprintln(this.writer, string(combinedOutput))
8685
return log.Errore(err)
8786
}
8887

‎go/logic/hooks_test.go

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/*
2+
Copyright 2022 GitHub Inc.
3+
See https://github.com/github/gh-ost/blob/master/LICENSE
4+
*/
5+
6+
package logic
7+
8+
import (
9+
"bufio"
10+
"bytes"
11+
"fmt"
12+
"os"
13+
"path/filepath"
14+
"strconv"
15+
"strings"
16+
"testing"
17+
"time"
18+
19+
"github.com/openark/golib/tests"
20+
21+
"github.com/github/gh-ost/go/base"
22+
)
23+
24+
func TestHooksExecutorExecuteHooks(t *testing.T) {
25+
migrationContext := base.NewMigrationContext()
26+
migrationContext.AlterStatement = "ENGINE=InnoDB"
27+
migrationContext.DatabaseName = "test"
28+
migrationContext.Hostname = "test.example.com"
29+
migrationContext.OriginalTableName = "tablename"
30+
migrationContext.RowsDeltaEstimate = 1
31+
migrationContext.RowsEstimate = 122
32+
migrationContext.TotalRowsCopied = 123456
33+
migrationContext.SetETADuration(time.Minute)
34+
migrationContext.SetProgressPct(50)
35+
hooksExecutor := NewHooksExecutor(migrationContext)
36+
37+
writeTmpHookFunc := func(testName, hookName, script string) (path string, err error) {
38+
if path, err = os.MkdirTemp("", testName); err != nil {
39+
return path, err
40+
}
41+
err = os.WriteFile(filepath.Join(path, hookName), []byte(script), 0777)
42+
return path, err
43+
}
44+
45+
t.Run("does-not-exist", func(t *testing.T) {
46+
migrationContext.HooksPath = "/does/not/exist"
47+
tests.S(t).ExpectNil(hooksExecutor.executeHooks("test-hook"))
48+
})
49+
50+
t.Run("failed", func(t *testing.T) {
51+
var err error
52+
if migrationContext.HooksPath, err = writeTmpHookFunc(
53+
"TestHooksExecutorExecuteHooks-failed",
54+
"failed-hook",
55+
"#!/bin/sh\nexit 1",
56+
); err != nil {
57+
panic(err)
58+
}
59+
defer os.RemoveAll(migrationContext.HooksPath)
60+
tests.S(t).ExpectNotNil(hooksExecutor.executeHooks("failed-hook"))
61+
})
62+
63+
t.Run("success", func(t *testing.T) {
64+
var err error
65+
if migrationContext.HooksPath, err = writeTmpHookFunc(
66+
"TestHooksExecutorExecuteHooks-success",
67+
"success-hook",
68+
"#!/bin/sh\nenv",
69+
); err != nil {
70+
panic(err)
71+
}
72+
defer os.RemoveAll(migrationContext.HooksPath)
73+
74+
var buf bytes.Buffer
75+
hooksExecutor.writer = &buf
76+
tests.S(t).ExpectNil(hooksExecutor.executeHooks("success-hook", "TEST="+t.Name()))
77+
78+
scanner := bufio.NewScanner(&buf)
79+
for scanner.Scan() {
80+
split := strings.SplitN(scanner.Text(), "=", 2)
81+
switch split[0] {
82+
case "GH_OST_COPIED_ROWS":
83+
copiedRows, _ := strconv.ParseInt(split[1], 10, 64)
84+
tests.S(t).ExpectEquals(copiedRows, migrationContext.TotalRowsCopied)
85+
case "GH_OST_DATABASE_NAME":
86+
tests.S(t).ExpectEquals(split[1], migrationContext.DatabaseName)
87+
case "GH_OST_DDL":
88+
tests.S(t).ExpectEquals(split[1], migrationContext.AlterStatement)
89+
case "GH_OST_DRY_RUN":
90+
tests.S(t).ExpectEquals(split[1], "false")
91+
case "GH_OST_ESTIMATED_ROWS":
92+
estimatedRows, _ := strconv.ParseInt(split[1], 10, 64)
93+
tests.S(t).ExpectEquals(estimatedRows, int64(123))
94+
case "GH_OST_ETA_SECONDS":
95+
etaSeconds, _ := strconv.ParseInt(split[1], 10, 64)
96+
tests.S(t).ExpectEquals(etaSeconds, int64(60))
97+
case "GH_OST_EXECUTING_HOST":
98+
tests.S(t).ExpectEquals(split[1], migrationContext.Hostname)
99+
case "GH_OST_GHOST_TABLE_NAME":
100+
tests.S(t).ExpectEquals(split[1], fmt.Sprintf("_%s_gho", migrationContext.OriginalTableName))
101+
case "GH_OST_OLD_TABLE_NAME":
102+
tests.S(t).ExpectEquals(split[1], fmt.Sprintf("_%s_del", migrationContext.OriginalTableName))
103+
case "GH_OST_PROGRESS":
104+
progress, _ := strconv.ParseFloat(split[1], 64)
105+
tests.S(t).ExpectEquals(progress, 50.0)
106+
case "GH_OST_TABLE_NAME":
107+
tests.S(t).ExpectEquals(split[1], migrationContext.OriginalTableName)
108+
case "TEST":
109+
tests.S(t).ExpectEquals(split[1], t.Name())
110+
}
111+
}
112+
})
113+
}

‎go/logic/migrator.go

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ type Migrator struct {
9797
func NewMigrator(context *base.MigrationContext, appVersion string) *Migrator {
9898
migrator := &Migrator{
9999
appVersion: appVersion,
100+
hooksExecutor: NewHooksExecutor(context),
100101
migrationContext: context,
101102
parser: sql.NewAlterTableParser(),
102103
ghostTableMigrated: make(chan bool),
@@ -112,15 +113,6 @@ func NewMigrator(context *base.MigrationContext, appVersion string) *Migrator {
112113
return migrator
113114
}
114115

115-
// initiateHooksExecutor
116-
func (this *Migrator) initiateHooksExecutor() (err error) {
117-
this.hooksExecutor = NewHooksExecutor(this.migrationContext)
118-
if err := this.hooksExecutor.initHooks(); err != nil {
119-
return err
120-
}
121-
return nil
122-
}
123-
124116
// sleepWhileTrue sleeps indefinitely until the given function returns 'false'
125117
// (or fails with error)
126118
func (this *Migrator) sleepWhileTrue(operation func() (bool, error)) error {
@@ -341,9 +333,6 @@ func (this *Migrator) Migrate() (err error) {
341333

342334
go this.listenOnPanicAbort()
343335

344-
if err := this.initiateHooksExecutor(); err != nil {
345-
return err
346-
}
347336
if err := this.hooksExecutor.onStartup(); err != nil {
348337
return err
349338
}

‎script/test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ script/build
1414
cd .gopath/src/github.com/github/gh-ost
1515

1616
echo "Running unit tests"
17-
go test ./go/...
17+
go test -v -covermode=atomic ./go/...

0 commit comments

Comments
 (0)