diff --git a/internal/enginenetx/httpsdialer_internal_test.go b/internal/enginenetx/httpsdialer_internal_test.go index 37ff85980d..4ec6766494 100644 --- a/internal/enginenetx/httpsdialer_internal_test.go +++ b/internal/enginenetx/httpsdialer_internal_test.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "errors" + "fmt" "sync" "sync/atomic" "testing" @@ -25,17 +26,13 @@ func TestHTTPSDialerTacticsEmitter(t *testing.T) { wg: &sync.WaitGroup{}, } - tactics := []HTTPSDialerTactic{ - &httpsDialerNullTactic{ - Address: "10.0.0.1", + var tactics []HTTPSDialerTactic + for idx := 0; idx < 255; idx++ { + tactics = append(tactics, &httpsDialerNullTactic{ + Address: fmt.Sprintf("10.0.0.%d", idx), Delay: 0, Domain: "www.example.com", - }, - &httpsDialerNullTactic{ - Address: "10.0.0.2", - Delay: 0, - Domain: "www.example.com", - }, + }) } ctx, cancel := context.WithCancel(context.Background()) @@ -43,13 +40,14 @@ func TestHTTPSDialerTacticsEmitter(t *testing.T) { out := hd.tacticsEmitter(ctx, tactics...) - var count int for range out { - count++ - } - - if count != 0 { - t.Fatal("nothing should have been emitted here") + // Here we do nothing! + // + // Ideally, we would like to count and assert that we have + // got no tactic from the channel but the selection of ready + // channels is nondeterministic, so we cannot really be + // asserting that. This leaves us with asking the question + // of what we should be asserting here? } }) } diff --git a/internal/enginenetx/httpsdialer_test.go b/internal/enginenetx/httpsdialer_test.go index b3a6eb2c92..5590f26d85 100644 --- a/internal/enginenetx/httpsdialer_test.go +++ b/internal/enginenetx/httpsdialer_test.go @@ -11,6 +11,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netemx" "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/testingx" ) // Flags controlling when [httpsDialerPolicyCancelingContext] cancels the context @@ -355,70 +356,81 @@ func TestHTTPSDialerWAI(t *testing.T) { t.Skip("skip test in short mode") } - // create the QA environment - env := netemx.MustNewScenario(tc.scenario) - defer env.Close() + // track all the connections so we can check whether we close them all + cv := &testingx.CloseVerify{} - // possibly add specific DPI rules - tc.configureDPI(env.DPIEngine()) + func() { + // create the QA environment + env := netemx.MustNewScenario(tc.scenario) + defer env.Close() - // create the proper underlying network - unet := &netxlite.NetemUnderlyingNetworkAdapter{UNet: env.ClientStack} + // possibly add specific DPI rules + tc.configureDPI(env.DPIEngine()) - // create the network proper - netx := &netxlite.Netx{Underlying: unet} + // create the proper underlying network and wrap it such that + // we track whether we close all the connections + unet := cv.WrapUnderlyingNetwork(&netxlite.NetemUnderlyingNetworkAdapter{UNet: env.ClientStack}) - // create the getaddrinfo resolver - resolver := netx.NewStdlibResolver(log.Log) + // create the network proper + netx := &netxlite.Netx{Underlying: unet} - // create the TLS dialer - dialer := enginenetx.NewHTTPSDialer( - log.Log, - tc.policy, - resolver, - unet, - ) - defer dialer.CloseIdleConnections() + // create the getaddrinfo resolver + resolver := netx.NewStdlibResolver(log.Log) - // configure cancellable context--some tests are going to use cancel - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + // create the TLS dialer + dialer := enginenetx.NewHTTPSDialer( + log.Log, + tc.policy, + resolver, + unet, + ) + defer dialer.CloseIdleConnections() - // Possibly tell the httpsDialerPolicyCancelingContext about the cancel func - // depending on which flags have been configured. - if p, ok := tc.policy.(*httpsDialerPolicyCancelingContext); ok { - p.cancel = cancel - } + // configure cancellable context--some tests are going to use cancel + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Possibly tell the httpsDialerPolicyCancelingContext about the cancel func + // depending on which flags have been configured. + if p, ok := tc.policy.(*httpsDialerPolicyCancelingContext); ok { + p.cancel = cancel + } + + // dial the TLS connection + tlsConn, err := dialer.DialTLSContext(ctx, "tcp", tc.endpoint) - // dial the TLS connection - tlsConn, err := dialer.DialTLSContext(ctx, "tcp", tc.endpoint) + t.Logf("%+v %+v", tlsConn, err) - t.Logf("%+v %+v", tlsConn, err) + // make sure the error is the one we expected + switch { + case err != nil && tc.expectErr == "": + t.Fatal("expected", tc.expectErr, "got", err) - // make sure the error is the one we expected - switch { - case err != nil && tc.expectErr == "": - t.Fatal("expected", tc.expectErr, "got", err) + case err == nil && tc.expectErr != "": + t.Fatal("expected", tc.expectErr, "got", err) - case err == nil && tc.expectErr != "": - t.Fatal("expected", tc.expectErr, "got", err) + case err != nil && tc.expectErr != "": + if diff := cmp.Diff(tc.expectErr, err.Error()); diff != "" { + t.Fatal(diff) + } - case err != nil && tc.expectErr != "": - if diff := cmp.Diff(tc.expectErr, err.Error()); diff != "" { - t.Fatal(diff) + case err == nil && tc.expectErr == "": + // all good } - case err == nil && tc.expectErr == "": - // all good - } + // make sure we close the conn + if tlsConn != nil { + defer tlsConn.Close() + } - // make sure we close the conn - if tlsConn != nil { - defer tlsConn.Close() - } + // wait for background connections to join + dialer.WaitGroup().Wait() + }() - // wait for background connections to join - dialer.WaitGroup().Wait() + // now verify that we have closed all the connections + if err := cv.CheckForOpenConns(); err != nil { + t.Fatal(err) + } }) } }