diff --git a/.github/workflows/alltests.yml b/.github/workflows/alltests.yml index d1d5c42619..6cdddc5181 100644 --- a/.github/workflows/alltests.yml +++ b/.github/workflows/alltests.yml @@ -1,6 +1,7 @@ # Runs the whole test suite name: alltests on: + pull_request: push: branches: - "release/**" diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 91f485aa80..c25824013d 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -24,7 +24,7 @@ jobs: go-version: "${{ steps.goversion.outputs.version }}" cache-key-suffix: "-coverage-${{ steps.goversion.outputs.version }}" - - run: go test -short -race -tags shaping -coverprofile=probe-cli.cov ./... + - run: ./script/linuxcoverage.bash - uses: shogo82148/actions-goveralls@v1 with: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index deb5860278..2fe1d41621 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,13 +36,26 @@ functionality should pass existing tests. What's more, any new pull request that modifies existing functionality should not decrease the existing code coverage. -Long-running tests should be skipped when running tests in short mode -using `go test -short`. We prefer internal testing to external -testing. We generally have a file called `foo_test.go` with tests -for every `foo.go` file. Sometimes we separate long running -integration tests in a `foo_integration_test.go` file. We also -sometimes have `foo_internal_test.go` when the main body of tests -for `foo`, i.e., `foo_test.go` uses external testing. +New code should have full coverage using either localhost or the +[internal/netemx](./internal/netemx/) package. Try to cover all the +error paths as well as the important properties of the code you've written +that you would like to be sure about. + +Additional integration tests using the host network are good, +but they MUST use this pattern: + +```Go +func TestUsingHostNetwork(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } +} +``` + +The overall objective here is for `go test -short` to only use localhost +and [internal/netemx](./internal/netemx/) such that tests are always +reproducible. Tests using the host network are there to give us extra +confidence that everything is working as intended. If there is a top-level DESIGN.md document, make sure such document is kept in sync with code changes you have applied. diff --git a/internal/cmd/oohelperd/main_test.go b/internal/cmd/oohelperd/main_test.go index 335e07bb3b..3a7013212e 100644 --- a/internal/cmd/oohelperd/main_test.go +++ b/internal/cmd/oohelperd/main_test.go @@ -21,6 +21,10 @@ type ( ) func TestMainRunServerWorkingAsIntended(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + // let the kernel pick a random free port *apiEndpoint = "127.0.0.1:0" diff --git a/internal/database/actions_test.go b/internal/database/actions_test.go index 390ecf6d25..8e9ae7ac48 100644 --- a/internal/database/actions_test.go +++ b/internal/database/actions_test.go @@ -367,6 +367,10 @@ func TestPerformanceTestKeys(t *testing.T) { } func TestGetMeasurementJSON(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + tmpfile, err := ioutil.TempFile("", "dbtest") if err != nil { t.Fatal(err) diff --git a/internal/engine/experiment_integration_test.go b/internal/engine/experiment_integration_test.go index cb83786a3b..865a45203b 100644 --- a/internal/engine/experiment_integration_test.go +++ b/internal/engine/experiment_integration_test.go @@ -130,7 +130,6 @@ func TestRunTelegram(t *testing.T) { } func TestRunTor(t *testing.T) { - t.Skip("https://github.com/ooni/probe/issues/2539") if testing.Short() { t.Skip("skip test in short mode") } diff --git a/internal/engine/session_integration_test.go b/internal/engine/session_integration_test.go index 8a0b941b61..7bea2abff1 100644 --- a/internal/engine/session_integration_test.go +++ b/internal/engine/session_integration_test.go @@ -492,6 +492,10 @@ func TestNewOrchestraClientProbeServicesNewClientFailure(t *testing.T) { } func TestSessionNewSubmitterReturnsNonNilSubmitter(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + sess := newSessionForTesting(t) subm, err := sess.NewSubmitter(context.Background()) if err != nil { diff --git a/internal/engine/session_internal_test.go b/internal/engine/session_internal_test.go index 454ccd5219..7eea180ef2 100644 --- a/internal/engine/session_internal_test.go +++ b/internal/engine/session_internal_test.go @@ -225,6 +225,10 @@ func TestNewProbeServicesClientForCheckIn(t *testing.T) { } func TestSessionNewSubmitterWithCancelledContext(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + sess := newSessionForTesting(t) ctx, cancel := context.WithCancel(context.Background()) cancel() // fail immediately diff --git a/internal/enginelocate/cloudflare_test.go b/internal/enginelocate/cloudflare_test.go index c3213c5060..4fc62e5197 100644 --- a/internal/enginelocate/cloudflare_test.go +++ b/internal/enginelocate/cloudflare_test.go @@ -12,6 +12,10 @@ import ( ) func TestIPLookupWorksUsingcloudlflare(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ip, err := cloudflareIPLookup( context.Background(), http.DefaultClient, diff --git a/internal/enginelocate/geolocate_test.go b/internal/enginelocate/geolocate_test.go index 7a69a68fd1..2154736d8b 100644 --- a/internal/enginelocate/geolocate_test.go +++ b/internal/enginelocate/geolocate_test.go @@ -266,6 +266,10 @@ func TestLocationLookupSuccessWithResolverLookup(t *testing.T) { } func TestSmoke(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + config := Config{} task := NewTask(config) result, err := task.Run(context.Background()) diff --git a/internal/enginelocate/iplookup_test.go b/internal/enginelocate/iplookup_test.go index f8c171c6ce..650c0eefd8 100644 --- a/internal/enginelocate/iplookup_test.go +++ b/internal/enginelocate/iplookup_test.go @@ -13,6 +13,10 @@ import ( ) func TestIPLookupGood(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ip, err := (ipLookupClient{ Logger: log.Log, Resolver: netxlite.NewStdlibResolver(model.DiscardLogger), diff --git a/internal/enginelocate/resolverlookup_test.go b/internal/enginelocate/resolverlookup_test.go index c02110a221..86b9a9e706 100644 --- a/internal/enginelocate/resolverlookup_test.go +++ b/internal/enginelocate/resolverlookup_test.go @@ -9,6 +9,10 @@ import ( ) func TestLookupResolverIPSuccess(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + rlc := resolverLookupClient{ Logger: model.DiscardLogger, } diff --git a/internal/enginelocate/stun_test.go b/internal/enginelocate/stun_test.go index 5efb96207b..d6a0707cd6 100644 --- a/internal/enginelocate/stun_test.go +++ b/internal/enginelocate/stun_test.go @@ -147,6 +147,10 @@ func TestSTUNIPLookupCannotDecodeMessage(t *testing.T) { } func TestIPLookupWorksUsingSTUNEkiga(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ip, err := stunEkigaIPLookup( context.Background(), http.DefaultClient, @@ -163,6 +167,10 @@ func TestIPLookupWorksUsingSTUNEkiga(t *testing.T) { } func TestIPLookupWorksUsingSTUNGoogle(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ip, err := stunGoogleIPLookup( context.Background(), http.DefaultClient, diff --git a/internal/enginelocate/ubuntu_test.go b/internal/enginelocate/ubuntu_test.go index e385b1b0f2..b2ede1f07e 100644 --- a/internal/enginelocate/ubuntu_test.go +++ b/internal/enginelocate/ubuntu_test.go @@ -35,6 +35,10 @@ func TestUbuntuParseError(t *testing.T) { } func TestIPLookupWorksUsingUbuntu(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ip, err := ubuntuIPLookup( context.Background(), http.DefaultClient, diff --git a/internal/experiment/dnscheck/dnscheck_test.go b/internal/experiment/dnscheck/dnscheck_test.go index f7beba6aee..a7fb9b336b 100644 --- a/internal/experiment/dnscheck/dnscheck_test.go +++ b/internal/experiment/dnscheck/dnscheck_test.go @@ -143,6 +143,10 @@ func TestMakeResolverURL(t *testing.T) { } func TestDNSCheckValid(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + measurer := NewExperimentMeasurer(Config{ DefaultAddrs: "1.1.1.1 1.0.0.1", }) @@ -189,6 +193,10 @@ func TestSummaryKeysGeneric(t *testing.T) { } func TestDNSCheckWait(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + endpoints := &Endpoints{ WaitTime: 1 * time.Second, } diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index f18a17d640..f49d61dc1c 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -67,6 +67,10 @@ func TestMeasurerMeasureWithInvalidInput2(t *testing.T) { } func TestMeasurementSuccess(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + sess := &mockable.Session{MockableLogger: log.Log} callbacks := model.NewPrinterCallbacks(sess.Logger()) measurer := NewExperimentMeasurer(Config{}) diff --git a/internal/experiment/hhfm/hhfm_test.go b/internal/experiment/hhfm/hhfm_test.go index d14c29cae0..eb420066d1 100644 --- a/internal/experiment/hhfm/hhfm_test.go +++ b/internal/experiment/hhfm/hhfm_test.go @@ -32,6 +32,10 @@ func TestNewExperimentMeasurer(t *testing.T) { } func TestSuccess(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + measurer := hhfm.NewExperimentMeasurer(hhfm.Config{}) ctx := context.Background() sess := &mockable.Session{ diff --git a/internal/experiment/ndt7/dial_test.go b/internal/experiment/ndt7/dial_test.go index 8f43b4c4ef..b2b5788a02 100644 --- a/internal/experiment/ndt7/dial_test.go +++ b/internal/experiment/ndt7/dial_test.go @@ -14,6 +14,10 @@ import ( ) func TestDialDownloadWithCancelledContext(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ctx, cancel := context.WithCancel(context.Background()) cancel() // immediately halt mgr := newDialManager("wss://hostname.fake", log.Log, "miniooni/0.1.0-dev") @@ -27,6 +31,10 @@ func TestDialDownloadWithCancelledContext(t *testing.T) { } func TestDialUploadWithCancelledContext(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ctx, cancel := context.WithCancel(context.Background()) cancel() // immediately halt mgr := newDialManager("wss://hostname.fake", log.Log, "miniooni/0.1.0-dev") diff --git a/internal/experiment/ndt7/ndt7_test.go b/internal/experiment/ndt7/ndt7_test.go index 23e3b9b587..371a289570 100644 --- a/internal/experiment/ndt7/ndt7_test.go +++ b/internal/experiment/ndt7/ndt7_test.go @@ -131,6 +131,10 @@ func TestGood(t *testing.T) { } func TestFailDownload(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ctx, cancel := context.WithCancel(context.Background()) defer cancel() measurer := NewExperimentMeasurer(Config{}).(*Measurer) @@ -162,6 +166,10 @@ func TestFailDownload(t *testing.T) { } func TestFailUpload(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ctx, cancel := context.WithCancel(context.Background()) defer cancel() measurer := NewExperimentMeasurer(Config{noDownload: true}).(*Measurer) diff --git a/internal/experiment/quicping/quicping_test.go b/internal/experiment/quicping/quicping_test.go index e0b28e37e1..c8705e65c0 100644 --- a/internal/experiment/quicping/quicping_test.go +++ b/internal/experiment/quicping/quicping_test.go @@ -123,6 +123,10 @@ func TestSuccess(t *testing.T) { } func TestWithCancelledContext(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + measurer := NewExperimentMeasurer(Config{}) measurement := new(model.Measurement) measurement.Input = model.MeasurementTarget("google.com") @@ -145,6 +149,10 @@ func TestWithCancelledContext(t *testing.T) { } func TestListenFails(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + expected := errors.New("expected") measurer := NewExperimentMeasurer(Config{ netListenUDP: func(network string, laddr *net.UDPAddr) (model.UDPLikeConn, error) { diff --git a/internal/experiment/signal/signal_test.go b/internal/experiment/signal/signal_test.go index 8dc6aa3805..1b8456bf03 100644 --- a/internal/experiment/signal/signal_test.go +++ b/internal/experiment/signal/signal_test.go @@ -23,6 +23,10 @@ func TestNewExperimentMeasurer(t *testing.T) { } func TestGood(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + measurer := signal.NewExperimentMeasurer(signal.Config{}) measurement := new(model.Measurement) args := &model.ExperimentArgs{ diff --git a/internal/experiment/stunreachability/stunreachability_test.go b/internal/experiment/stunreachability/stunreachability_test.go index b7b1acffd4..8336584aaa 100644 --- a/internal/experiment/stunreachability/stunreachability_test.go +++ b/internal/experiment/stunreachability/stunreachability_test.go @@ -89,6 +89,10 @@ func TestRunWithUnsupportedURLScheme(t *testing.T) { } func TestRunWithInput(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + measurer := NewExperimentMeasurer(Config{}) measurement := new(model.Measurement) measurement.Input = model.MeasurementTarget(defaultInput) @@ -158,6 +162,10 @@ func TestCancelledContext(t *testing.T) { } func TestNewClientFailure(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + config := &Config{} expected := errors.New("mocked error") config.newClient = func(conn stun.Connection, options ...stun.ClientOption) (*stun.Client, error) { diff --git a/internal/experiment/tlstool/internal/internal_test.go b/internal/experiment/tlstool/internal/internal_test.go index 30c694a220..568e0d12b9 100644 --- a/internal/experiment/tlstool/internal/internal_test.go +++ b/internal/experiment/tlstool/internal/internal_test.go @@ -25,17 +25,33 @@ func dial(t *testing.T, d model.Dialer) { } func TestNewSNISplitterDialer(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + dial(t, internal.NewSNISplitterDialer(config)) } func TestNewThriceSplitterDialer(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + dial(t, internal.NewThriceSplitterDialer(config)) } func TestNewRandomSplitterDialer(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + dial(t, internal.NewRandomSplitterDialer(config)) } func TestNewVanillaDialer(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + dial(t, internal.NewVanillaDialer(config)) } diff --git a/internal/experiment/urlgetter/getter_integration_test.go b/internal/experiment/urlgetter/getter_integration_test.go index 50eb0f7331..9ea2677dbb 100644 --- a/internal/experiment/urlgetter/getter_integration_test.go +++ b/internal/experiment/urlgetter/getter_integration_test.go @@ -386,6 +386,10 @@ func TestGetterWithCancelledContextUnknownResolverURL(t *testing.T) { } func TestGetterIntegrationHTTPS(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ctx := context.Background() g := urlgetter.Getter{ Config: urlgetter.Config{ @@ -510,6 +514,10 @@ func TestGetterIntegrationRedirect(t *testing.T) { } func TestGetterIntegrationTLSHandshake(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ctx := context.Background() g := urlgetter.Getter{ Config: urlgetter.Config{ @@ -611,6 +619,10 @@ func TestGetterIntegrationTLSHandshake(t *testing.T) { } func TestGetterHTTPSWithTunnel(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + // quick enough (0.4s) to run with every run ctx := context.Background() g := urlgetter.Getter{ diff --git a/internal/experiment/urlgetter/multi_test.go b/internal/experiment/urlgetter/multi_test.go index 589ba36e22..e4fe27dcf5 100644 --- a/internal/experiment/urlgetter/multi_test.go +++ b/internal/experiment/urlgetter/multi_test.go @@ -17,6 +17,10 @@ import ( ) func TestMultiIntegration(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + multi := urlgetter.Multi{Session: &mockable.Session{}} inputs := []urlgetter.MultiInput{{ Config: urlgetter.Config{Method: "HEAD", NoFollowRedirects: true}, diff --git a/internal/measurexlite/quic_test.go b/internal/measurexlite/quic_test.go index 7a5eba2ca7..e0d1262574 100644 --- a/internal/measurexlite/quic_test.go +++ b/internal/measurexlite/quic_test.go @@ -13,7 +13,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/netxlite/quictesting" + "github.com/ooni/probe-cli/v3/internal/testingquic" "github.com/ooni/probe-cli/v3/internal/testingx" "github.com/quic-go/quic-go" ) @@ -276,6 +276,10 @@ func TestNewQUICDialerWithoutResolver(t *testing.T) { } func TestOnQUICHandshakeDoneExtractsTheConnectionState(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + // create a trace trace := NewTrace(0, time.Now()) @@ -286,7 +290,7 @@ func TestOnQUICHandshakeDoneExtractsTheConnectionState(t *testing.T) { // dial with the endpoint we use for testing quicConn, err := quicDialer.DialContext( context.Background(), - quictesting.Endpoint("443"), + testingquic.MustEndpoint("443"), &tls.Config{ InsecureSkipVerify: true, }, diff --git a/internal/mlablocatev2/mlablocatev2_test.go b/internal/mlablocatev2/mlablocatev2_test.go index 11e54374a8..1e497d20d6 100644 --- a/internal/mlablocatev2/mlablocatev2_test.go +++ b/internal/mlablocatev2/mlablocatev2_test.go @@ -14,7 +14,9 @@ import ( ) func TestQueryNDT7Success(t *testing.T) { - // this integration test is ~0.5 s, so we can always run it + if testing.Short() { + t.Skip("skip test in short mode") + } client := NewClient(http.DefaultClient, model.DiscardLogger, "miniooni/0.1.0-dev") result, err := client.QueryNDT7(context.Background()) @@ -48,7 +50,9 @@ func TestQueryNDT7Success(t *testing.T) { } func TestQueryDashSuccess(t *testing.T) { - // this integration test is ~0.5 s, so we can always run it + if testing.Short() { + t.Skip("skip test in short mode") + } client := NewClient(http.DefaultClient, model.DiscardLogger, "miniooni/0.1.0-dev") result, err := client.QueryDash(context.Background()) @@ -82,7 +86,9 @@ func TestQueryDashSuccess(t *testing.T) { } func TestQuery404Response(t *testing.T) { - // this integration test is ~0.5 s, so we can always run it + if testing.Short() { + t.Skip("skip test in short mode") + } client := NewClient(http.DefaultClient, model.DiscardLogger, "miniooni/0.1.0-dev") result, err := client.query(context.Background(), "nonexistent") diff --git a/internal/netxlite/integration_test.go b/internal/netxlite/integration_test.go index 7199599b6c..cdf8378351 100644 --- a/internal/netxlite/integration_test.go +++ b/internal/netxlite/integration_test.go @@ -16,9 +16,9 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/netxlite/quictesting" "github.com/ooni/probe-cli/v3/internal/randx" "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/testingquic" "github.com/ooni/probe-cli/v3/internal/testingx" "github.com/quic-go/quic-go" utls "gitlab.com/yawning/utls.git" @@ -487,11 +487,11 @@ func TestMeasureWithQUICDialer(t *testing.T) { // why we're using nil to force netxlite to use the cached // default Mozilla cert pool. config := &tls.Config{ - ServerName: quictesting.Domain, + ServerName: testingquic.MustDomain(), NextProtos: []string{"h3"}, RootCAs: nil, } - sess, err := d.DialContext(ctx, quictesting.Endpoint("443"), config, &quic.Config{}) + sess, err := d.DialContext(ctx, testingquic.MustEndpoint("443"), config, &quic.Config{}) if err != nil { t.Fatal(err) } @@ -510,12 +510,12 @@ func TestMeasureWithQUICDialer(t *testing.T) { // why we're using nil to force netxlite to use the cached // default Mozilla cert pool. config := &tls.Config{ - ServerName: quictesting.Domain, + ServerName: testingquic.MustDomain(), NextProtos: []string{"h3"}, RootCAs: nil, } // Here we assume :1 is filtered - sess, err := d.DialContext(ctx, quictesting.Endpoint("1"), config, &quic.Config{}) + sess, err := d.DialContext(ctx, testingquic.MustEndpoint("1"), config, &quic.Config{}) if err == nil || err.Error() != netxlite.FailureGenericTimeoutError { t.Fatal("not the error we expected", err) } @@ -588,7 +588,7 @@ func TestHTTP3Transport(t *testing.T) { ) txp := netxlite.NewHTTP3Transport(log.Log, d, &tls.Config{}) client := &http.Client{Transport: txp} - URL := (&url.URL{Scheme: "https", Host: quictesting.Domain, Path: "/"}).String() + URL := (&url.URL{Scheme: "https", Host: testingquic.MustDomain(), Path: "/"}).String() resp, err := client.Get(URL) if err != nil { t.Fatal(err) diff --git a/internal/netxlite/quictesting/quictesting.go b/internal/netxlite/quictesting/quictesting.go deleted file mode 100644 index a2a7761455..0000000000 --- a/internal/netxlite/quictesting/quictesting.go +++ /dev/null @@ -1,38 +0,0 @@ -// Package quictesting contains code useful to test QUIC. -package quictesting - -import ( - "context" - "net" - "strings" - "time" - - "github.com/ooni/probe-cli/v3/internal/runtimex" -) - -// Domain is the the domain we should be testing using QUIC. -const Domain = "www.cloudflare.com" - -// Address is the address we should be testing using QUIC. -var Address string - -// Endpoint returns the endpoint to test using QUIC by combining -// the Address variable with the given port. -func Endpoint(port string) string { - return net.JoinHostPort(Address, port) -} - -func init() { - const timeout = 10 * time.Second - ctx, cancel := context.WithTimeout(context.Background(), timeout) - defer cancel() - reso := &net.Resolver{} - addrs, err := reso.LookupHost(ctx, Domain) - runtimex.PanicOnError(err, "reso.LookupHost failed") - for _, addr := range addrs { - if !strings.Contains(addr, ":") { - Address = addr - break - } - } -} diff --git a/internal/netxlite/utls_test.go b/internal/netxlite/utls_test.go index 29d7fbd6ec..f771262d89 100644 --- a/internal/netxlite/utls_test.go +++ b/internal/netxlite/utls_test.go @@ -108,6 +108,10 @@ func TestUTLSConn(t *testing.T) { } func Test_newConnUTLSWithHelloID(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + tests := []struct { name string config *tls.Config diff --git a/internal/probeservices/checkin_test.go b/internal/probeservices/checkin_test.go index 00e3c2bac0..b9ea1ace94 100644 --- a/internal/probeservices/checkin_test.go +++ b/internal/probeservices/checkin_test.go @@ -9,6 +9,10 @@ import ( ) func TestCheckInSuccess(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + client := newclient() client.BaseURL = "https://ams-pg-test.ooni.org" config := model.OOAPICheckInConfig{ diff --git a/internal/probeservices/collector_test.go b/internal/probeservices/collector_test.go index c4bd1359fe..f5f9810d0d 100644 --- a/internal/probeservices/collector_test.go +++ b/internal/probeservices/collector_test.go @@ -71,6 +71,10 @@ func TestNewReportTemplate(t *testing.T) { } func TestReportLifecycle(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ctx := context.Background() template := model.OOAPIReportTemplate{ DataFormatVersion: model.OOAPIReportDefaultDataFormatVersion, @@ -101,6 +105,10 @@ func TestReportLifecycle(t *testing.T) { } func TestReportLifecycleWrongExperiment(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ctx := context.Background() template := model.OOAPIReportTemplate{ DataFormatVersion: model.OOAPIReportDefaultDataFormatVersion, @@ -355,6 +363,10 @@ func TestOpenReportCancelledContext(t *testing.T) { } func TestSubmitMeasurementCancelledContext(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + ctx, cancel := context.WithCancel(context.Background()) defer cancel() template := model.OOAPIReportTemplate{ diff --git a/internal/probeservices/login_test.go b/internal/probeservices/login_test.go index b051937fe4..a2bf3345fd 100644 --- a/internal/probeservices/login_test.go +++ b/internal/probeservices/login_test.go @@ -54,7 +54,10 @@ func TestMaybeLogin(t *testing.T) { } func TestMaybeLoginIdempotent(t *testing.T) { - t.Skip("TODO(https://github.com/ooni/probe/issues/2539)") + if testing.Short() { + t.Skip("skip test in short mode") + } + clnt := newclient() ctx := context.Background() metadata := MetadataFixture() diff --git a/internal/probeservices/measurementmeta_test.go b/internal/probeservices/measurementmeta_test.go index 5cb44f37e7..63ed736d73 100644 --- a/internal/probeservices/measurementmeta_test.go +++ b/internal/probeservices/measurementmeta_test.go @@ -15,6 +15,10 @@ import ( ) func TestGetMeasurementMetaWorkingAsIntended(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + client := Client{ APIClientTemplate: httpx.APIClientTemplate{ BaseURL: "https://api.ooni.io/", diff --git a/internal/probeservices/probeservices_test.go b/internal/probeservices/probeservices_test.go index 00c24dc741..dcb32667bf 100644 --- a/internal/probeservices/probeservices_test.go +++ b/internal/probeservices/probeservices_test.go @@ -574,6 +574,10 @@ func TestSelectBestSelectsTheFastest(t *testing.T) { } func TestGetCredsAndAuthNotLoggedIn(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + clnt := newclient() if err := clnt.MaybeRegister(context.Background(), MetadataFixture()); err != nil { t.Fatal(err) diff --git a/internal/probeservices/psiphon_test.go b/internal/probeservices/psiphon_test.go index c216808ca2..3bfacd2de2 100644 --- a/internal/probeservices/psiphon_test.go +++ b/internal/probeservices/psiphon_test.go @@ -8,7 +8,10 @@ import ( ) func TestFetchPsiphonConfig(t *testing.T) { - t.Skip("TODO(https://github.com/ooni/probe/issues/2539)") + if testing.Short() { + t.Skip("skip test in short mode") + } + clnt := newclient() if err := clnt.MaybeRegister(context.Background(), MetadataFixture()); err != nil { t.Fatal(err) diff --git a/internal/probeservices/register_test.go b/internal/probeservices/register_test.go index 12059aa9c3..488132b7d9 100644 --- a/internal/probeservices/register_test.go +++ b/internal/probeservices/register_test.go @@ -47,6 +47,10 @@ func TestMaybeRegister(t *testing.T) { } func TestMaybeRegisterIdempotent(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + clnt := newclient() ctx := context.Background() metadata := MetadataFixture() diff --git a/internal/probeservices/tor_test.go b/internal/probeservices/tor_test.go index 1205bd70e4..62f7065143 100644 --- a/internal/probeservices/tor_test.go +++ b/internal/probeservices/tor_test.go @@ -7,7 +7,6 @@ import ( ) func TestFetchTorTargets(t *testing.T) { - t.Skip("TODO(https://github.com/ooni/probe/issues/2539)") if testing.Short() { t.Skip("skip test in short mode") } @@ -60,7 +59,10 @@ func (clnt *FetchTorTargetsHTTPTransport) RoundTrip(req *http.Request) (*http.Re } func TestFetchTorTargetsSetsQueryString(t *testing.T) { - t.Skip("TODO(https://github.com/ooni/probe/issues/2539)") + if testing.Short() { + t.Skip("skip test in short mode") + } + clnt := newclient() txp := new(FetchTorTargetsHTTPTransport) clnt.HTTPClient = &http.Client{Transport: txp} diff --git a/internal/ptx/fake_test.go b/internal/ptx/fake_test.go index d9d0742636..ed3af55486 100644 --- a/internal/ptx/fake_test.go +++ b/internal/ptx/fake_test.go @@ -6,6 +6,10 @@ import ( ) func TestFakeDialerWorks(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + fd := &FakeDialer{Address: "8.8.8.8:53"} conn, err := fd.DialContext(context.Background()) if err != nil { diff --git a/internal/ptx/ptx_test.go b/internal/ptx/ptx_test.go index f5d1f026ba..88f231cf88 100644 --- a/internal/ptx/ptx_test.go +++ b/internal/ptx/ptx_test.go @@ -25,6 +25,10 @@ func TestListenerLoggerWorks(t *testing.T) { } func TestListenerWorksWithFakeDialer(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + // start the fake PT fd := &FakeDialer{Address: "google.com:80"} lst := &Listener{PTDialer: fd} diff --git a/internal/testingquic/testingquic.go b/internal/testingquic/testingquic.go new file mode 100644 index 0000000000..a19aafa87c --- /dev/null +++ b/internal/testingquic/testingquic.go @@ -0,0 +1,59 @@ +// Package testingquic allows to retrieve the domain and endpoint to use +// for all the integration tests that use QUIC. +// +// See https://github.com/ooni/probe/issues/1873 for context. +package testingquic + +import ( + "context" + "net" + "strings" + "sync" + "time" + + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +const domain = "www.cloudflare.com" + +var ( + address string + initOnce sync.Once +) + +func mustInit() { + // create a context using a reasonable timeout + const timeout = 10 * time.Second + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + // instantiate the system resolver + reso := &net.Resolver{} + + // perform the lookup and panic on failure + addrs := runtimex.Try1(reso.LookupHost(ctx, domain)) + + // use the first non IPv6 addr + for _, addr := range addrs { + if !strings.Contains(addr, ":") { + address = addr + return + } + } +} + +// MustEndpoint returns a QUIC endpoint using this package's default address and then given port. +// +// This function PANICS if we cannot find out the IP addr we should use. +func MustEndpoint(port string) string { + initOnce.Do(mustInit) + return net.JoinHostPort(address, port) +} + +// MustDomain returns the domain to use for QUIC testing. +// +// This function PANICS if we cannot find out the IP addr we should use. +func MustDomain() string { + initOnce.Do(mustInit) + return domain +} diff --git a/internal/netxlite/quictesting/quictesting_test.go b/internal/testingquic/testingquic_test.go similarity index 50% rename from internal/netxlite/quictesting/quictesting_test.go rename to internal/testingquic/testingquic_test.go index 0a8f99e390..47c67c4483 100644 --- a/internal/netxlite/quictesting/quictesting_test.go +++ b/internal/testingquic/testingquic_test.go @@ -1,4 +1,4 @@ -package quictesting +package testingquic import ( "net" @@ -6,12 +6,17 @@ import ( ) func TestWorksAsIntended(t *testing.T) { - epnt := Endpoint("443") - addr, port, err := net.SplitHostPort(epnt) + if testing.Short() { + t.Skip("skip test in short mode") + } + + endpoint := MustEndpoint("443") + addr, port, err := net.SplitHostPort(endpoint) if err != nil { t.Fatal(err) } - if addr != Address { + + if addr != address { t.Fatal("invalid addr") } if port != "443" { diff --git a/pkg/oonimkall/xoonirun_test.go b/pkg/oonimkall/xoonirun_test.go index 32fae4d214..7a88193049 100644 --- a/pkg/oonimkall/xoonirun_test.go +++ b/pkg/oonimkall/xoonirun_test.go @@ -16,6 +16,10 @@ import ( func TestOONIRunFetch(t *testing.T) { t.Run("we can fetch a OONI Run link descriptor", func(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + sess, err := NewSessionForTesting() if err != nil { t.Fatal(err) diff --git a/script/linuxcoverage.bash b/script/linuxcoverage.bash new file mode 100755 index 0000000000..944c69fe14 --- /dev/null +++ b/script/linuxcoverage.bash @@ -0,0 +1,18 @@ +#!/bin/bash + +# +# Computes coverage inside an environment where we unshared the network namespace +# to ensure unit tests don't depend on the network. +# + +set -euxo pipefail + +# obtain the full path of the go executable +go=$(which go) + +# populate the vendor directory so we don't need the network in `go test` +go mod vendor + +# run tests using a different network namespace +sudo unshare --net ./script/linuxcoveragerun.bash $go + diff --git a/script/linuxcoveragerun.bash b/script/linuxcoveragerun.bash new file mode 100755 index 0000000000..a3eb15b483 --- /dev/null +++ b/script/linuxcoveragerun.bash @@ -0,0 +1,18 @@ +#!/bin/bash + +# +# Script invoked by ./script/linuxcoverage.bash to run tests with coverage +# using a separate network namespace with only loopback support. +# +# The first an unique argument is the path to the go binary to use. +# + +set -euxo pipefail + +# make sure we have access to loopback since we have many ~unit +# tests using the loopback interface +ip link set lo up + +# make sure we run all the "unit" tests (where "unit" means proper unit +# tests or tests using localhost or tests using netemx). +$1 test -short -race -count 1 -coverprofile=probe-cli.cov ./...