From f0927ea00c84e7be19e95fa22e34d1fe51011af8 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Sun, 5 Sep 2021 12:44:44 +0200 Subject: [PATCH] ci: master only runs coverage and only with go1.17 (+ fix flaky test) (#452) * fix: disable debianrepo build on master branch This just mitigates https://github.com/ooni/probe/issues/1741 and does not fully address it, but I'd rather avoid delving into this problem until I open a release/v3.11.0 branch and have to really fix this issue. * fix: only run coverage using go1.17 This is the version of Go with which we are going to bless v3.11.0 therefore it's the only version of Go that matters. Reference issue: https://github.com/ooni/probe/issues/1738. * fix(ptx/obfs4_test.go): avoid context-vs-normal-code race We want to test whether we get the context failure if the error generated inside normal code happens _after_ the context cancellation. The best way to do that is to write code that is not racy. To this end, we just need to pause normal code until we know that the context has returned to the caller. We also need to ensure we do not leak a goroutine, hence we use a WaitGroup to check that. Fixes https://github.com/ooni/probe/issues/1750 --- .github/workflows/coverage.yml | 2 +- .github/workflows/debianrepo.yml | 3 +-- internal/ptx/obfs4_test.go | 14 ++++++++++---- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 43e0420..f3ac09b 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - go: [ "1.16", "1.17" ] + go: [ "1.17" ] steps: - uses: actions/setup-go@v1 with: diff --git a/.github/workflows/debianrepo.yml b/.github/workflows/debianrepo.yml index e7b21ac..32414e7 100644 --- a/.github/workflows/debianrepo.yml +++ b/.github/workflows/debianrepo.yml @@ -3,8 +3,7 @@ name: linux on: push: branches: - - "master" - - "issue/1484" + - "release/**" jobs: test_386: diff --git a/internal/ptx/obfs4_test.go b/internal/ptx/obfs4_test.go index fd60610..d0924c1 100644 --- a/internal/ptx/obfs4_test.go +++ b/internal/ptx/obfs4_test.go @@ -5,6 +5,7 @@ import ( "errors" "net" "strings" + "sync" "testing" "github.com/ooni/probe-cli/v3/internal/atomicx" @@ -65,14 +66,17 @@ func TestOBFS4DialerFailsWithConnectionErrorAndNoContextExpiration(t *testing.T) func TestOBFS4DialerFailsWithConnectionErrorAndContextExpiration(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - expected := errors.New("mocked error") + unexpected := errors.New("mocked error") o4d := DefaultTestingOBFS4Bridge() + sigch := make(chan interface{}) + wg := &sync.WaitGroup{} + wg.Add(1) o4d.UnderlyingDialer = &netxmocks.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - // We cancel the context before returning the error, which makes - // the context cancellation happen before us returning. cancel() - return nil, expected + <-sigch + wg.Done() + return nil, unexpected }, } conn, err := o4d.DialContext(ctx) @@ -82,6 +86,8 @@ func TestOBFS4DialerFailsWithConnectionErrorAndContextExpiration(t *testing.T) { if conn != nil { t.Fatal("expected nil conn here") } + close(sigch) + wg.Wait() } // obfs4connwrapper allows us to observe that Close has been called