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

using t.Fatal in a goroutine #2043

Closed
whyrusleeping opened this issue Dec 6, 2015 · 6 comments
Closed

using t.Fatal in a goroutine #2043

whyrusleeping opened this issue Dec 6, 2015 · 6 comments
Labels
help wanted Seeking public contribution on this issue kind/test Testing work need/verification This issue needs verification

Comments

@whyrusleeping
Copy link
Member

Apparently (it makes sense thinking about it) you can't use t.Fatal in a goroutine.

A cursory search reveals at least a few occurences of this bug: https://gist.github.com/whyrusleeping/6ff875746eccfc2611ab

Lots of them in our dependencies unfortunately, but a few in our code:
exchange/bitswap/bitswap_test.go-173
fuse/readonly/ipfs_test.go-160
merkledag/merkledag_test.go-198-
test/supernode_client/main.go-145-
test/supernode_client/main.go-183

@whyrusleeping whyrusleeping added the kind/test Testing work label Dec 6, 2015
@chriscool
Copy link
Contributor

@whyrusleeping is there something else that should be used instead?

@whyrusleeping
Copy link
Member Author

the errors need to be somehow propogated back to the main goroutine (the one executing the Test* function) and t.Fatal needs to be called there.

@chriscool
Copy link
Contributor

Ok, thanks!

@jbenet
Copy link
Member

jbenet commented Dec 12, 2015

Here are some examples of what i do:

example:

func TestSimpleAsync(t *testing.T) {

    // use a channel to hand off the error
    errs := make(chan error, 1)
    go func() {
        err := somethingAsync()
        errs <- err
    }()

    // wait for it
    err := <-errs
    if err != nil {
        t.Fatal(err)
    }
}
func TestNAsync(t *testing.T) {

    // we're going to have N children
    N := 10

    // use a channel to hand off the error
    errs := make(chan error, N)
    for i := 0; i < N; i++ {
        go func() {
            err := somethingAsync()
            errs <- err
        }()
    }

    // wait for all N to finish
    for i := 0; i < N; i++ {
        err := <-errs
        if err != nil {
            t.Fatal(err)
        }
    }
}
func TestManyAsync(t *testing.T) {

    var wg sync.WaitGroup
    errs := make(chan error, 10)

    for shouldKeepGoing() {

        wg.Add(1)
        go func() {
            defer wg.Done()

            err := somethingAsync()
            if err != nil {
                errs <- err
            }
        }()
    }

    // close errs once all children finish.
    // wg allows us to do this without knowing
    // how many children a priori.
    go func() {
        wg.Wait()
        close(errs)
    }()

    for err := range errs {
        if err != nil {
            t.Fatal(err)
        }
    }
}

@daviddias daviddias removed the topic/fuse Topic fuse label Jan 2, 2016
@whyrusleeping whyrusleeping added help wanted Seeking public contribution on this issue need/verification This issue needs verification labels Aug 19, 2016
@whyrusleeping
Copy link
Member Author

We should verify that all occurences of this have been resolved, then close the issue

@Stebalien
Copy link
Member

Closing because, realistically, nobody's going to audit every one of our repos for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue kind/test Testing work need/verification This issue needs verification
Projects
None yet
Development

No branches or pull requests

5 participants