Skip to content

refactoring of driver_test.go #57

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

Merged
merged 9 commits into from
Apr 23, 2013

Conversation

arnehormann
Copy link
Member

introduce a test for long data (longer than defaultBufSize in buffer.go) to test zerocopy code
setup with init() instead of getEnv(), get rid of sync-dependency
extract common initialization code, use callback methods and prepare for further simplifications

@arnehormann
Copy link
Member Author

When I started this, I based it on a prior version. It does not include the time test, I'll readd it and add a comment when it's there... Please don't merge yet!

@arnehormann
Copy link
Member Author

I added the time test back in, but changed the setup structure.
In this version, the table setup/teardown isn't interleaved, it happens twice.
time.Time and string versions are fully separate.
The test still fails with this error message: "rror on Exec INSERT INTO test VALUES (?): Error 1292: Incorrect date value: '0000-00-00' for column 'value' at row 1".
I don't quite get why, will investigate tomorrow.

@xaprb
Copy link

xaprb commented Apr 22, 2013

MySQL's settings can influence that a lot. That sounds like the SQL_MODE
setting (try "select @@sql_mode") is set to a strict option that rejects
zero dates.

@arnehormann
Copy link
Member Author

Sure (and thanks - I had hope for a moment), but it also breaks on Travis, which it didn't before...

@arnehormann
Copy link
Member Author

Still, Travis' error message is different. Apparently I'm using strict settings (thanks to your blog post) but messed up somewhere else. Will have another look...

@arnehormann
Copy link
Member Author

ready to merge now

return run
}
var got string
rows.Scan(&got)
Copy link
Member

Choose a reason for hiding this comment

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

catch err

@julienschmidt
Copy link
Member

Let's try to clean up all that args stuff a bit:

diff --git a/driver_test.go b/driver_test.go
index f8eda58..9e2faab 100644
--- a/driver_test.go
+++ b/driver_test.go
@@ -126,10 +126,6 @@ func runTests(t *testing.T, name string, tests ...func(db *DBTest)) {
    environment.mustExec("DROP TABLE IF EXISTS test")
 }

-func args(args ...interface{}) []interface{} {
-   return args
-}
-
 func (dbt *DBTest) fail(method, query string, err error) {
    if len(query) > 300 {
        query = "[query too large to print]"
@@ -137,7 +133,7 @@ func (dbt *DBTest) fail(method, query string, err error) {
    dbt.Fatalf("Error on %s %s: %v", method, query, err)
 }

-func (dbt *DBTest) execWithArgs(query string, args []interface{}, test func(res sql.Result)) {
+func (dbt *DBTest) testExec(test func(res sql.Result), query string, args ...interface{}) {
    res, err := dbt.Exec(query, args...)
    if err != nil {
        dbt.fail("Exec", query, err)
@@ -145,11 +141,7 @@ func (dbt *DBTest) execWithArgs(query string, args []interface{}, test func(res
    test(res)
 }

-func (dbt *DBTest) exec(query string, test func(result sql.Result)) {
-   dbt.execWithArgs(query, args(), test)
-}
-
-func (dbt *DBTest) queryWithArgs(query string, args []interface{}, test func(rows *sql.Rows)) {
+func (dbt *DBTest) testQuery(test func(rows *sql.Rows), query string, args ...interface{}) {
    rows, err := dbt.Query(query, args...)
    if err != nil {
        dbt.fail("Query", query, err)
@@ -158,10 +150,6 @@ func (dbt *DBTest) queryWithArgs(query string, args []interface{}, test func(row
    test(rows)
 }

-func (dbt *DBTest) query(query string, test func(result *sql.Rows)) {
-   dbt.queryWithArgs(query, args(), test)
-}
-
 func (dbt *DBTest) mustExec(query string, args ...interface{}) (res sql.Result) {
    res, err := dbt.Exec(query, args...)
    if err != nil {
@@ -195,7 +183,7 @@ func TestCRUD(t *testing.T) {
 func _rawBytesResultExceedsBuffer(db *DBTest) {
    // defaultBufSize from buffer.go
    expected := strings.Repeat("abc", defaultBufSize)
-   db.query("SELECT '"+expected+"'", func(rows *sql.Rows) {
+   db.testQuery(func(rows *sql.Rows) {
        if !rows.Next() {
            db.Error("expected result, got none")
        }
@@ -204,7 +192,7 @@ func _rawBytesResultExceedsBuffer(db *DBTest) {
        if expected != string(result) {
            db.Error("result did not match expected value")
        }
-   })
+   }, "SELECT '"+expected+"'")
 }

 func _crud(db *DBTest) {
@@ -452,8 +440,8 @@ func TestDateTime(t *testing.T) {
        d0string  = "0000-00-00"
        dt0string = "0000-00-00 00:00:00"
        modes     = map[string]*testmode{
-           "text":   &testmode{"", args()},
-           "binary": &testmode{" WHERE 1 = ?", args(1)},
+           "text":   &testmode{},
+           "binary": &testmode{" WHERE 1 = ?", []interface{}{1}},
        }
        timetests = map[string][]*timetest{
            "DATE": {

Since we need to change the args order for .query and .exec, .testQuery and .testExec seem less confusing to me.


rows = mustQuery(t, db, ("SELECT value FROM test"))
rows = db.mustQuery("SELECT value FROM test")
if rows.Next() {
rows.Scan(&out)
Copy link
Member

Choose a reason for hiding this comment

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

Although it's old code, please catch err here too

@arnehormann
Copy link
Member Author

You're right.
I originally intended to use query / exec / queryWithArgs / execWithArgs with inline functions, which would be hard to read with arguments following the inline function declaration.
Still, it's too big a price to pay. Yours is better.

@arnehormann
Copy link
Member Author

Another thing I don't really like is the "underscore functions". I'm not done with them yet.
They were inline in my first version, but I dislike the indentation caused by the callback.
I love the callback, though. At least in test code. Any ideas how that could be improved?
Does testing or some extension provide a way to "decorate" test methods?


mustExec(t, db, "DROP TABLE IF EXISTS test")
func TestCRUD(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Slid up to the wrong place? :)

Move it down after _rawBytesResultExceedsBuffer please

julienschmidt added a commit that referenced this pull request Apr 23, 2013
refactoring of driver_test.go
@julienschmidt julienschmidt merged commit 87c17e7 into go-sql-driver:master Apr 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants