Skip to content

Commit

Permalink
ci: master only runs coverage and only with go1.17 (+ fix flaky test) (
Browse files Browse the repository at this point in the history
…ooni#452)

* fix: disable debianrepo build on master branch

This just mitigates ooni/probe#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: ooni/probe#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 ooni/probe#1750
  • Loading branch information
bassosimone authored Sep 5, 2021
1 parent d038959 commit 428fc66
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/debianrepo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ name: linux
on:
push:
branches:
- "master"
- "issue/1484"
- "release/**"

jobs:
test_386:
Expand Down
14 changes: 10 additions & 4 deletions internal/ptx/obfs4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"net"
"strings"
"sync"
"testing"

"github.com/ooni/probe-cli/v3/internal/atomicx"
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down

0 comments on commit 428fc66

Please sign in to comment.