Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the source command #128

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions r/source.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
before source
Hello from the included file
SELECT 1
1
1
after source
Goodbye from the included file
SELECT 3
3
3
146 changes: 112 additions & 34 deletions src/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,13 @@ var (
retryConnCount int
collationDisable bool
checkErr bool
disableSource bool
)

func init() {
// Disable the `--source` command by default to avoid breaking existing tests
disableSource = true

Comment on lines +54 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a program argument to set it enabled by default, similar to --check-error argument, so it would work with test files that contain --source'd files that does not exists or have not yet been tested successfully?

flag.StringVar(&host, "host", "127.0.0.1", "The host of the TiDB/MySQL server.")
flag.StringVar(&port, "port", "4000", "The listen port of TiDB/MySQL server.")
flag.StringVar(&user, "user", "root", "The user for connecting to the database.")
Expand All @@ -72,10 +76,15 @@ const (
type query struct {
firstWord string
Query string
File string
Line int
tp int
}

func (q *query) location() string {
return fmt.Sprintf("%s:%d", q.File, q.Line)
}

type Conn struct {
// DB might be a shared one by multiple Conn, if the connection information are the same.
mdb *sql.DB
Expand Down Expand Up @@ -325,7 +334,7 @@ func (t *tester) addSuccess(testSuite *XUnitTestSuite, startTime *time.Time, cnt
func (t *tester) Run() error {
t.preProcess()
defer t.postProcess()
queries, err := t.loadQueries()
queries, err := t.loadQueries(t.testFileName())
if err != nil {
err = errors.Trace(err)
t.addFailure(&testSuite, &err, 0)
Expand All @@ -338,17 +347,33 @@ func (t *tester) Run() error {
return err
}

var s string
defer func() {
if t.resultFD != nil {
t.resultFD.Close()
}
}()

testCnt := 0
startTime := time.Now()
testCnt, err := t.runQueries(queries)
if err != nil {
return err
}

fmt.Printf("%s: ok! %d test cases passed, take time %v s\n", t.testFileName(), testCnt, time.Since(startTime).Seconds())

if xmlPath != "" {
t.addSuccess(&testSuite, &startTime, testCnt)
}

return t.flushResult()
}

func (t *tester) runQueries(queries []query) (int, error) {
testCnt := 0
var concurrentQueue []query
var concurrentSize int
var s string
var err error
for _, q := range queries {
s = q.Query
switch q.tp {
Expand Down Expand Up @@ -379,15 +404,15 @@ func (t *tester) Run() error {
if err != nil {
err = errors.Annotate(err, "Atoi failed")
t.addFailure(&testSuite, &err, testCnt)
return err
return testCnt, err
}
}
case Q_END_CONCURRENT:
t.enableConcurrent = false
if err = t.concurrentRun(concurrentQueue, concurrentSize); err != nil {
err = errors.Annotate(err, fmt.Sprintf("concurrent test failed in %v", t.name))
t.addFailure(&testSuite, &err, testCnt)
return err
return testCnt, err
}
t.expectedErrs = nil
case Q_ERROR:
Expand All @@ -404,9 +429,9 @@ func (t *tester) Run() error {
if t.enableConcurrent {
concurrentQueue = append(concurrentQueue, q)
} else if err = t.execute(q); err != nil {
err = errors.Annotate(err, fmt.Sprintf("sql:%v", q.Query))
err = errors.Annotate(err, fmt.Sprintf("sql:%v line:%s", q.Query, q.location()))
t.addFailure(&testSuite, &err, testCnt)
return err
return testCnt, err
}

testCnt++
Expand All @@ -426,7 +451,7 @@ func (t *tester) Run() error {
if err != nil {
err = errors.Annotate(err, fmt.Sprintf("Could not parse column in --replace_column: sql:%v", q.Query))
t.addFailure(&testSuite, &err, testCnt)
return err
return testCnt, err
}

t.replaceColumn = append(t.replaceColumn, ReplaceColumn{col: colNr, replace: []byte(cols[i+1])})
Expand Down Expand Up @@ -473,7 +498,7 @@ func (t *tester) Run() error {
r, err := t.executeStmtString(s)
if err != nil {
log.WithFields(log.Fields{
"query": s, "line": q.Line},
"query": s, "line": q.location()},
).Error("failed to perform let query")
return ""
}
Expand All @@ -484,27 +509,68 @@ func (t *tester) Run() error {
case Q_REMOVE_FILE:
err = os.Remove(strings.TrimSpace(q.Query))
if err != nil {
return errors.Annotate(err, "failed to remove file")
return testCnt, errors.Annotate(err, "failed to remove file")
}
case Q_REPLACE_REGEX:
t.replaceRegex = nil
regex, err := ParseReplaceRegex(q.Query)
if err != nil {
return errors.Annotate(err, fmt.Sprintf("Could not parse regex in --replace_regex: line: %d sql:%v", q.Line, q.Query))
return testCnt, errors.Annotate(
err, fmt.Sprintf("Could not parse regex in --replace_regex: line: %s sql:%v",
q.location(), q.Query))
}
t.replaceRegex = regex
default:
log.WithFields(log.Fields{"command": q.firstWord, "arguments": q.Query, "line": q.Line}).Warn("command not implemented")
}
}
case Q_ENABLE_SOURCE:
disableSource = false
case Q_DISABLE_SOURCE:
disableSource = true
case Q_SOURCE:
if disableSource {
log.WithFields(log.Fields{"line": q.location()}).Warn("source command disabled, add '--enable_source' to your file to enable")
break
}
fileName := strings.TrimSpace(q.Query)
cwd, err := os.Getwd()
if err != nil {
return testCnt, err
}

fmt.Printf("%s: ok! %d test cases passed, take time %v s\n", t.testFileName(), testCnt, time.Since(startTime).Seconds())
// For security, don't allow to include files from other locations
fullpath, err := filepath.Abs(fileName)
if err != nil {
return testCnt, err
}
if !strings.HasPrefix(fullpath, cwd) {
return testCnt, errors.Errorf("included file %s is not prefixed with %s", fullpath, cwd)
}

if xmlPath != "" {
t.addSuccess(&testSuite, &startTime, testCnt)
}
// Make sure we have a useful error message if the file can't be found or isn't a regular file
s, err := os.Stat(fileName)
if err != nil {
return testCnt, errors.Annotate(err,
fmt.Sprintf("file sourced with --source doesn't exist: line %s, file: %s",
q.location(), fileName))
}
if !s.Mode().IsRegular() {
return testCnt, errors.Errorf("file sourced with --source isn't a regular file: line %s, file: %s",
q.location(), fileName)
}

return t.flushResult()
// Process the queries in the file
includedQueries, err := t.loadQueries(fileName)
if err != nil {
return testCnt, errors.Annotate(err, fmt.Sprintf("error loading queries from %s", fileName))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this lead to a full 'stack trace'? I.e. so you can follow from the originating file's line, to the current --source'd file and line?
Is this testable? Maybe with a unit test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dvaneeden@dve-carbon:~/dev/pingcap/mysql-tester$ chmod 000 include/hello3.inc
dvaneeden@dve-carbon:~/dev/pingcap/mysql-tester$ ./mysql-tester source
ERRO[0000] run test [source] err: error loading queries from include/hello3.inc: open include/hello3.inc: permission denied 

ERRO[0000] 1 tests failed                               
ERRO[0000] run test [source] err: error loading queries from include/hello3.inc: open include/hello3.inc: permission denied 

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add the location:

dvaneeden@dve-carbon:~/dev/pingcap/mysql-tester$ ./mysql-tester source
ERRO[0000] run test [source] err: error loading queries from include/hello3.inc, line ./t/source.test:11: open include/hello3.inc: permission denied 

ERRO[0000] 1 tests failed                               
ERRO[0000] run test [source] err: error loading queries from include/hello3.inc, line ./t/source.test:11: open include/hello3.inc: permission denied 
dvaneeden@dve-carbon:~/dev/pingcap/mysql-tester$ git diff
diff --git a/src/main.go b/src/main.go
index 5e09363..3808f49 100644
--- a/src/main.go
+++ b/src/main.go
@@ -559,7 +559,7 @@ func (t *tester) runQueries(queries []query) (int, error) {
                        // Process the queries in the file
                        includedQueries, err := t.loadQueries(fileName)
                        if err != nil {
-                               return testCnt, errors.Annotate(err, fmt.Sprintf("error loading queries from %s", fileName))
+                               return testCnt, errors.Annotate(err, fmt.Sprintf("error loading queries from %s, line %s", fileName, q.location()))
                        }
                        includeCnt, err := t.runQueries(includedQueries)
                        if err != nil {

}
includeCnt, err := t.runQueries(includedQueries)
if err != nil {
return testCnt, err
}
testCnt += includeCnt
default:
log.WithFields(log.Fields{"command": q.firstWord, "arguments": q.Query, "line": q.location()}).Warn("command not implemented")
}
}
return testCnt, nil
}

func (t *tester) concurrentRun(concurrentQueue []query, concurrentSize int) error {
Expand Down Expand Up @@ -606,8 +672,8 @@ func (t *tester) concurrentExecute(querys []query, wg *sync.WaitGroup, errOccure
}
}

func (t *tester) loadQueries() ([]query, error) {
data, err := os.ReadFile(t.testFileName())
func (t *tester) loadQueries(fileName string) ([]query, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have a new tester for a new --source'd file instead of using the upper level tester with a different filename argument?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, the included files are still part of the same test.

data, err := os.ReadFile(fileName)
if err != nil {
return nil, err
}
Expand All @@ -623,18 +689,30 @@ func (t *tester) loadQueries() ([]query, error) {
newStmt = true
continue
} else if strings.HasPrefix(s, "--") {
queries = append(queries, query{Query: s, Line: i + 1})
queries = append(queries, query{
Query: s,
Line: i + 1,
File: fileName,
})
newStmt = true
continue
} else if len(s) == 0 {
continue
}

if newStmt {
queries = append(queries, query{Query: s, Line: i + 1})
queries = append(queries, query{
Query: s,
Line: i + 1,
File: fileName,
})
} else {
lastQuery := queries[len(queries)-1]
lastQuery = query{Query: fmt.Sprintf("%s\n%s", lastQuery.Query, s), Line: lastQuery.Line}
lastQuery = query{
Query: fmt.Sprintf("%s\n%s", lastQuery.Query, s),
Line: lastQuery.Line,
File: fileName,
}
queries[len(queries)-1] = lastQuery
}

Expand Down Expand Up @@ -668,8 +746,8 @@ func (t *tester) checkExpectedError(q query, err error) error {
}
}
if !checkErr {
log.Warnf("%s:%d query succeeded, but expected error(s)! (expected errors: %s) (query: %s)",
t.name, q.Line, strings.Join(t.expectedErrs, ","), q.Query)
log.Warnf("%s query succeeded, but expected error(s)! (expected errors: %s) (query: %s)",
q.location(), strings.Join(t.expectedErrs, ","), q.Query)
return nil
}
return errors.Errorf("Statement succeeded, expected error(s) '%s'", strings.Join(t.expectedErrs, ","))
Expand All @@ -684,7 +762,7 @@ func (t *tester) checkExpectedError(q query, err error) error {
errNo = int(innerErr.Number)
}
if errNo == 0 {
log.Warnf("%s:%d Could not parse mysql error: %s", t.name, q.Line, err.Error())
log.Warnf("%s Could not parse mysql error: %s", q.location(), err.Error())
return err
}
for _, s := range t.expectedErrs {
Expand All @@ -696,9 +774,9 @@ func (t *tester) checkExpectedError(q query, err error) error {
checkErrNo = i
} else {
if len(t.expectedErrs) > 1 {
log.Warnf("%s:%d Unknown named error %s in --error %s", t.name, q.Line, s, strings.Join(t.expectedErrs, ","))
log.Warnf("%s Unknown named error %s in --error %s", q.location(), s, strings.Join(t.expectedErrs, ","))
} else {
log.Warnf("%s:%d Unknown named --error %s", t.name, q.Line, s)
log.Warnf("%s Unknown named --error %s", q.location(), s)
}
continue
}
Expand Down Expand Up @@ -726,11 +804,11 @@ func (t *tester) checkExpectedError(q query, err error) error {
}
}
if len(t.expectedErrs) > 1 {
log.Warnf("%s:%d query failed with non expected error(s)! (%s not in %s) (err: %s) (query: %s)",
t.name, q.Line, gotErrCode, strings.Join(t.expectedErrs, ","), err.Error(), q.Query)
log.Warnf("%s query failed with non expected error(s)! (%s not in %s) (err: %s) (query: %s)",
q.location(), gotErrCode, strings.Join(t.expectedErrs, ","), err.Error(), q.Query)
} else {
log.Warnf("%s:%d query failed with non expected error(s)! (%s != %s) (err: %s) (query: %s)",
t.name, q.Line, gotErrCode, t.expectedErrs[0], err.Error(), q.Query)
log.Warnf("%s query failed with non expected error(s)! (%s != %s) (err: %s) (query: %s)",
q.location(), gotErrCode, t.expectedErrs[0], err.Error(), q.Query)
}
errStr := err.Error()
for _, reg := range t.replaceRegex {
Expand Down
3 changes: 3 additions & 0 deletions src/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ const (
Q_COMMENT /* Comments, ignored. */
Q_COMMENT_WITH_COMMAND
Q_EMPTY_LINE
Q_DISABLE_SOURCE
Q_ENABLE_SOURCE
)

// ParseQueries parses an array of string into an array of query object.
Expand All @@ -136,6 +138,7 @@ func ParseQueries(qs ...query) ([]query, error) {
q := query{}
q.tp = Q_UNKNOWN
q.Line = rs.Line
q.File = rs.File
// a valid query's length should be at least 3.
if len(s) < 3 {
continue
Expand Down
2 changes: 2 additions & 0 deletions src/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ var commandMap = map[string]int{
"single_query": Q_SINGLE_QUERY,
"begin_concurrent": Q_BEGIN_CONCURRENT,
"end_concurrent": Q_END_CONCURRENT,
"disable_source": Q_DISABLE_SOURCE,
"enable_source": Q_ENABLE_SOURCE,
}

func findType(cmdName string) int {
Expand Down
11 changes: 11 additions & 0 deletions t/source.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
--enable_source

--echo before source
--source include/hello1.inc
--echo after source

--disable_source
--source include/hello2.inc
--enable_source

--source include/hello3.inc
Loading