Convert more goconvey tests to normal go tests (#3330)

Also introduce `xtest.AssertErrorsIs` and use it instead of manual asserts with `xerrors.Is`

Contributes #3016

go/godispatcher/network/BUILD.bazel

@@ -44,6 +44,7 @@ go_test(
         "//go/lib/spkt:go_default_library",
         "//go/lib/xtest:go_default_library",
         "@com_github_golang_mock//gomock:go_default_library",
-        "@com_github_smartystreets_goconvey//convey:go_default_library",
+        "@com_github_stretchr_testify//assert:go_default_library",
+        "@com_github_stretchr_testify//require:go_default_library",
     ],
 )

go/godispatcher/network/overlay_test.go

@@ -19,7 +19,8 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
-	. "github.com/smartystreets/goconvey/convey"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/scionproto/scion/go/lib/addr"
 	"github.com/scionproto/scion/go/lib/common"
@@ -41,7 +42,7 @@ func TestComputeDestination(t *testing.T) {
 		Description string
 		Packet      *spkt.ScnPkt
 		ExpectedDst Destination
-		ExpectedErr common.ErrMsg
+		ExpectedErr error
 	}
 	var testCases = []*TestCase{
 		{
@@ -208,19 +209,17 @@ func TestComputeDestination(t *testing.T) {
 			ExpectedErr: ErrMalformedL4Quote,
 		},
 	}
-	Convey("", t, func() {
-		for _, test := range testCases {
-			Convey(test.Description, func() {
-				destination, err := ComputeDestination(test.Packet)
-				xtest.SoMsgErrorStr("err", err, test.ExpectedErr.Error())
-				SoMsg("destination", destination, ShouldResemble, test.ExpectedDst)
-			})
-		}
-	})
+	for _, test := range testCases {
+		t.Run(test.Description, func(t *testing.T) {
+			destination, err := ComputeDestination(test.Packet)
+			xtest.AssertErrorsIs(t, err, test.ExpectedErr)
+			assert.Equal(t, test.ExpectedDst, destination)
+		})
+	}
 }
 
 func MustPackL4Header(t *testing.T, header l4.L4Header) common.RawBytes {
 	b, err := header.Pack(false)
-	xtest.FailOnErr(t, err)
+	require.NoError(t, err)
 	return b
 }

go/hidden_path_srv/internal/hpsegreq/BUILD.bazel

@@ -58,6 +58,5 @@ go_test(
         "@com_github_golang_mock//gomock:go_default_library",
         "@com_github_stretchr_testify//assert:go_default_library",
         "@com_github_stretchr_testify//require:go_default_library",
-        "@org_golang_x_xerrors//:go_default_library",
     ],
 )

go/hidden_path_srv/internal/hpsegreq/fetcher_test.go

@@ -22,7 +22,6 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/xerrors"
 
 	"github.com/scionproto/scion/go/hidden_path_srv/internal/hiddenpathdb/adapter"
 	"github.com/scionproto/scion/go/hidden_path_srv/internal/hpsegreq"
@@ -344,12 +343,8 @@ func TestFetcher(t *testing.T) {
 			f := hpsegreq.NewDefaultFetcher(groupInfo, mockMsgr, adapter.New(mockDB))
 			test.setup(mockDB, mockMsgr)
 			recs, err := f.Fetch(context.Background(), test.req, test.peer)
-			if test.err == nil {
-				require.NoError(t, err)
-				assert.ElementsMatch(t, test.res, recs)
-			} else {
-				assert.True(t, xerrors.Is(err, test.err))
-			}
+			xtest.AssertErrorsIs(t, err, test.err)
+			assert.ElementsMatch(t, test.res, recs)
 		})
 	}
 }

go/hidden_path_srv/internal/registration/BUILD.bazel

@@ -53,6 +53,5 @@ go_test(
         "@com_github_golang_mock//gomock:go_default_library",
         "@com_github_stretchr_testify//assert:go_default_library",
         "@com_github_stretchr_testify//require:go_default_library",
-        "@org_golang_x_xerrors//:go_default_library",
     ],
 )

go/hidden_path_srv/internal/registration/validator_test.go

@@ -18,9 +18,7 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/xerrors"
 
 	"github.com/scionproto/scion/go/hidden_path_srv/internal/registration"
 	"github.com/scionproto/scion/go/lib/addr"
@@ -181,13 +179,7 @@ func TestValidator(t *testing.T) {
 				},
 			}
 			err := validator.Validate(msg, test.peer)
-			if test.Err == nil {
-				assert.NoError(t, err)
-			} else {
-				require.Error(t, err)
-				assert.True(t, xerrors.Is(err, test.Err))
-			}
-
+			xtest.AssertErrorsIs(t, err, test.Err)
 		})
 	}
 }

go/lib/infra/BUILD.bazel

@@ -36,6 +36,5 @@ go_test(
         "//go/lib/infra/mock_infra:go_default_library",
         "//go/proto:go_default_library",
         "@com_github_golang_mock//gomock:go_default_library",
-        "@com_github_smartystreets_goconvey//convey:go_default_library",
     ],
 )

go/lib/infra/common_test.go

@@ -19,7 +19,6 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
-	. "github.com/smartystreets/goconvey/convey"
 
 	"github.com/scionproto/scion/go/lib/ctrl/ack"
 	"github.com/scionproto/scion/go/lib/infra"
@@ -42,22 +41,21 @@ func (r *mockResource) IsHealthy() bool {
 	return r.healthy
 }
 
+// TestResourceHealth tests that an unhealthy resource results in error replied.
 func TestResourceHealth(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
-	Convey("Unhealthy resource results in error replied", t, func() {
-		handler := infra.HandlerFunc(func(r *infra.Request) *infra.HandlerResult {
-			return nil
-		})
-		rHandler := infra.NewResourceAwareHandler(handler,
-			&mockResource{name: "tstFail", healthy: false})
-		rwMock := mock_infra.NewMockResponseWriter(ctrl)
-		ctx := infra.NewContextWithResponseWriter(context.Background(), rwMock)
-		rwMock.EXPECT().SendAckReply(gomock.Eq(ctx), gomock.Eq(&ack.Ack{
-			Err:     proto.Ack_ErrCode_reject,
-			ErrDesc: "Resource tstFail not healthy",
-		}))
-		req := infra.NewRequest(ctx, nil, nil, nil, 1)
-		rHandler.Handle(req)
+	handler := infra.HandlerFunc(func(r *infra.Request) *infra.HandlerResult {
+		return nil
 	})
+	rHandler := infra.NewResourceAwareHandler(handler,
+		&mockResource{name: "tstFail", healthy: false})
+	rwMock := mock_infra.NewMockResponseWriter(ctrl)
+	ctx := infra.NewContextWithResponseWriter(context.Background(), rwMock)
+	rwMock.EXPECT().SendAckReply(gomock.Eq(ctx), gomock.Eq(&ack.Ack{
+		Err:     proto.Ack_ErrCode_reject,
+		ErrDesc: "Resource tstFail not healthy",
+	}))
+	req := infra.NewRequest(ctx, nil, nil, nil, 1)
+	rHandler.Handle(req)
 }

go/lib/infra/modules/trust/v2/router_test.go

@@ -22,7 +22,6 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/xerrors"
 
 	"github.com/scionproto/scion/go/lib/addr"
 	"github.com/scionproto/scion/go/lib/common"
@@ -33,6 +32,7 @@ import (
 	"github.com/scionproto/scion/go/lib/snet/mock_snet"
 	"github.com/scionproto/scion/go/lib/spath"
 	"github.com/scionproto/scion/go/lib/util"
+	"github.com/scionproto/scion/go/lib/xtest"
 )
 
 func TestLocalRouterChooseServer(t *testing.T) {
@@ -135,9 +135,7 @@ func TestCSRouterChooseServer(t *testing.T) {
 			router := trust.NewCSRouter(1, r, db)
 			res, err := router.ChooseServer(context.Background(), test.ISD)
 			if test.ExpectedErr != nil {
-				require.Error(t, err)
-				assert.True(t, xerrors.Is(err, test.ExpectedErr), "Expected: %s Actual: %s",
-					test.ExpectedErr, err)
+				xtest.AssertErrorsIs(t, err, test.ExpectedErr)
 			} else {
 				require.NoError(t, err)
 				expected := &snet.Addr{

go/lib/infra/rpc/BUILD.bazel

@@ -25,9 +25,9 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//go/lib/ctrl:go_default_library",
-        "//go/lib/xtest:go_default_library",
         "//go/proto:go_default_library",
-        "@com_github_smartystreets_goconvey//convey:go_default_library",
+        "@com_github_stretchr_testify//assert:go_default_library",
+        "@com_github_stretchr_testify//require:go_default_library",
         "@com_zombiezen_go_capnproto2//:go_default_library",
         "@com_zombiezen_go_capnproto2//pogs:go_default_library",
     ],

go/lib/infra/rpc/rpc_test.go

@@ -21,12 +21,12 @@ import (
 	"testing"
 	"time"
 
-	. "github.com/smartystreets/goconvey/convey"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	capnp "zombiezen.com/go/capnproto2"
 	"zombiezen.com/go/capnproto2/pogs"
 
 	"github.com/scionproto/scion/go/lib/ctrl"
-	"github.com/scionproto/scion/go/lib/xtest"
 	"github.com/scionproto/scion/go/proto"
 )
 
@@ -39,7 +39,7 @@ type handler struct {
 func (h *handler) ServeRPC(rw ReplyWriter, request *Request) {
 	reply := &Reply{Message: getMessage(h.t)}
 	err := rw.WriteReply(reply)
-	xtest.FailOnErr(h.t, err)
+	require.NoError(h.t, err)
 }
 
 const (
@@ -48,54 +48,46 @@ const (
 )
 
 func TestServer(t *testing.T) {
-	Convey("Given a server", t, func() {
-		cliConn, srvConn := getTestUDPConns(t, 60000, 60001)
-		defer cliConn.Close()
-		defer srvConn.Close()
-
-		server := &Server{
-			Conn: srvConn,
-			TLSConfig: &tls.Config{
-				Certificates: []tls.Certificate{
-					MustLoadCertificate(defPemPath, defKeyPath),
-				},
+	cliConn, srvConn := getTestUDPConns(t, 60000, 60001)
+	defer cliConn.Close()
+	defer srvConn.Close()
+
+	server := &Server{
+		Conn: srvConn,
+		TLSConfig: &tls.Config{
+			Certificates: []tls.Certificate{
+				MustLoadCertificate(defPemPath, defKeyPath),
 			},
-			Handler: &handler{t: t},
-		}
-		go server.ListenAndServe()
-		time.Sleep(40 * time.Millisecond)
-		Convey("Double listen returns an error", func() {
-			err := server.ListenAndServe()
-			So(err, ShouldNotBeNil)
-		})
-		Convey("Closing does not return an error", func() {
-			err := server.Close()
-			So(err, ShouldBeNil)
-		})
+		},
+		Handler: &handler{t: t},
+	}
+	go server.ListenAndServe()
+	time.Sleep(40 * time.Millisecond)
+	t.Run("Double listen returns an error", func(t *testing.T) {
+		err := server.ListenAndServe()
+		assert.Error(t, err)
+	})
+	t.Run("Closing does not return an error", func(t *testing.T) {
+		err := server.Close()
+		assert.NoError(t, err)
 	})
 }
 
 func TestClientServer(t *testing.T) {
-	Convey("", t, func() {
-		client, server, _ := getCliSrv(t, 60002, 60003)
-		go func() {
-			err := server.ListenAndServe()
-			xtest.FailOnErr(t, err)
-		}()
-		ctx, cancelF := context.WithTimeout(context.Background(), time.Second)
-		defer cancelF()
-		reply, err := client.Request(
-			ctx,
-			&Request{Message: getMessage(t)},
-			&net.UDPAddr{IP: net.IP{127, 0, 0, 1}, Port: 60003},
-		)
-		xtest.FailOnErr(t, err)
-		So(
-			mustMarshalMessage(reply.Message),
-			ShouldResemble,
-			mustMarshalMessage(getMessage(t)),
-		)
-	})
+	client, server, _ := getCliSrv(t, 60002, 60003)
+	go func() {
+		err := server.ListenAndServe()
+		require.NoError(t, err)
+	}()
+	ctx, cancelF := context.WithTimeout(context.Background(), time.Second)
+	defer cancelF()
+	reply, err := client.Request(
+		ctx,
+		&Request{Message: getMessage(t)},
+		&net.UDPAddr{IP: net.IP{127, 0, 0, 1}, Port: 60003},
+	)
+	require.NoError(t, err)
+	assert.Equal(t, mustMarshalMessage(t, getMessage(t)), mustMarshalMessage(t, reply.Message))
 }
 
 func MustLoadCertificate(pem, key string) tls.Certificate {
@@ -131,27 +123,25 @@ func getCliSrv(t testing.TB, cliPort, srvPort int) (*Client, *Server, func()) {
 
 func getTestUDPConns(t testing.TB, cliPort, srvPort int) (net.PacketConn, net.PacketConn) {
 	srvConn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IP{127, 0, 0, 1}, Port: srvPort})
-	xtest.FailOnErr(t, err)
+	require.NoError(t, err)
 	cliConn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IP{127, 0, 0, 1}, Port: cliPort})
-	xtest.FailOnErr(t, err)
+	require.NoError(t, err)
 	return cliConn, srvConn
 }
 
 func getMessage(t testing.TB) *capnp.Message {
 	signedPld := &ctrl.SignedPld{}
 	msg, seg, err := capnp.NewMessage(capnp.SingleSegment(nil))
-	xtest.FailOnErr(t, err)
+	require.NoError(t, err)
 	root, err := proto.NewRootSignedCtrlPld(seg)
-	xtest.FailOnErr(t, err)
+	require.NoError(t, err)
 	err = pogs.Insert(proto.SignedCtrlPld_TypeID, root.Struct, signedPld)
-	xtest.FailOnErr(t, err)
+	require.NoError(t, err)
 	return msg
 }
 
-func mustMarshalMessage(msg *capnp.Message) []byte {
+func mustMarshalMessage(t *testing.T, msg *capnp.Message) []byte {
 	b, err := msg.Marshal()
-	if err != nil {
-		panic(err)
-	}
+	require.NoError(t, err)
 	return b
 }

go/lib/l4/BUILD.bazel

@@ -21,6 +21,6 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//go/lib/common:go_default_library",
-        "@com_github_smartystreets_goconvey//convey:go_default_library",
+        "@com_github_stretchr_testify//assert:go_default_library",
     ],
 )