code review 6942050: sql: adds test for fix in issue 4433.

sql: adds test for fix in issue 4433.
Tests that here should be automatic retries if a database driver's connection returns ErrBadConn on Begin. See "TestTxErrBadConn" in sql_test.go

[chalfant, 2012-12-14 08:17:29]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2012-12-14 17:00:38]

*** Submitted as https://code.google.com/p/go/source/detail?r=de94dfa4fa4a ***

database/sql: adds test for fix in issue 4433.

Tests that here should be automatic retries if a database
driver's connection returns ErrBadConn on Begin. See
"TestTxErrBadConn" in sql_test.go

R=golang-dev
CC=golang-dev
https://codereview.appspot.com/6942050

Committer: Brad Fitzpatrick <bradfitz@golang.org>

[bradfitz, 2012-12-14 17:01:28]

LGTM

Thanks!

I changed the commit message before submitting ("database/sql: ..."), blank
line after, and wrapped body.


On Fri, Dec 14, 2012 at 12:17 AM, <james.chalfant@gmail.com> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> sql: adds test for fix in issue 4433.
> Tests that here should be automatic retries if a database driver's
> connection returns ErrBadConn on Begin. See "TestTxErrBadConn" in
> sql_test.go
>
> Please review this at
https://codereview.appspot.**com/6942050/<https://codereview.appspot.com/6942...
>
> Affected files:
>   M src/pkg/database/sql/fakedb_**test.go
>   M src/pkg/database/sql/sql_test.**go
>
>
> Index: src/pkg/database/sql/fakedb_**test.go
> ==============================**==============================**=======
> --- a/src/pkg/database/sql/fakedb_**test.go
> +++ b/src/pkg/database/sql/fakedb_**test.go
> @@ -42,9 +42,10 @@
>  type fakeDB struct {
>         name string
>
> -       mu     sync.Mutex
> -       free   []*fakeConn
> -       tables map[string]*table
> +       mu      sync.Mutex
> +       free    []*fakeConn
> +       tables  map[string]*table
> +       badConn bool
>  }
>
>  type table struct {
> @@ -83,6 +84,7 @@
>         stmtsMade   int
>         stmtsClosed int
>         numPrepare  int
> +       bad         bool
>  }
>
>  func (c *fakeConn) incrStat(v *int) {
> @@ -122,7 +124,9 @@
>
>  // Supports dsn forms:
>  //    <dbname>
> -//    <dbname>;<opts>  (no currently supported options)
> +//    <dbname>;<opts>  (only currently supported option is `badConn`,
> +//                      which causes driver.ErrBadConn to be returned on
> +//                      every other conn.Begin())
>  func (d *fakeDriver) Open(dsn string) (driver.Conn, error) {
>         parts := strings.Split(dsn, ";")
>         if len(parts) < 1 {
> @@ -135,7 +139,12 @@
>         d.mu.Lock()
>         d.openCount++
>         d.mu.Unlock()
> -       return &fakeConn{db: db}, nil
> +       conn := &fakeConn{db: db}
> +
> +       if len(parts) >= 2 && parts[1] == "badConn" {
> +               conn.bad = true
> +       }
> +       return conn, nil
>  }
>
>  func (d *fakeDriver) getDB(name string) *fakeDB {
> @@ -199,7 +208,20 @@
>         return "", false
>  }
>
> +func (c *fakeConn) isBad() bool {
> +       // if not simulating bad conn, do nothing
> +       if !c.bad {
> +               return false
> +       }
> +       // alternate between bad conn and not bad conn
> +       c.db.badConn = !c.db.badConn
> +       return c.db.badConn
> +}
> +
>  func (c *fakeConn) Begin() (driver.Tx, error) {
> +       if c.isBad() {
> +               return nil, driver.ErrBadConn
> +       }
>         if c.currTx != nil {
>                 return nil, errors.New("already in a transaction")
>         }
> Index: src/pkg/database/sql/sql_test.**go
> ==============================**==============================**=======
> --- a/src/pkg/database/sql/sql_**test.go
> +++ b/src/pkg/database/sql/sql_**test.go
> @@ -402,6 +402,39 @@
>         }
>  }
>
> +// Tests fix for issue 4433, that retries in Begin happen when
> +// conn.Begin() returns ErrBadConn
> +func TestTxErrBadConn(t *testing.T) {
> +       db, err := Open("test", fakeDBName+";badConn")
> +       if err != nil {
> +               t.Fatalf("Open: %v", err)
> +       }
> +       if _, err := db.Exec("WIPE"); err != nil {
> +               t.Fatalf("exec wipe: %v", err)
> +       }
> +       defer closeDB(t, db)
> +       exec(t, db, "CREATE|t1|name=string,age=**int32,dead=bool")
> +       stmt, err := db.Prepare("INSERT|t1|name=?,**age=?")
> +       if err != nil {
> +               t.Fatalf("Stmt, err = %v, %v", stmt, err)
> +       }
> +       defer stmt.Close()
> +       tx, err := db.Begin()
> +       if err != nil {
> +               t.Fatalf("Begin = %v", err)
> +       }
> +       txs := tx.Stmt(stmt)
> +       defer txs.Close()
> +       _, err = txs.Exec("Bobby", 7)
> +       if err != nil {
> +               t.Fatalf("Exec = %v", err)
> +       }
> +       err = tx.Commit()
> +       if err != nil {
> +               t.Fatalf("Commit = %v", err)
> +       }
> +}
> +
>  // Tests fix for issue 2542, that we release a lock when querying on
>  // a closed connection.
>  func TestIssue2542Deadlock(t *testing.T) {
>
>
>