Repository: scionproto/scion (go)

Issue:
test: Get rid of goconvey
Goconvey is slightly painful to work with. Also, upstream seems very inactive.

The vanilla golang sub-tests provide the functionality we need already.


Pull request description:
revcache: remove goconvey
Remove use of goconvey from private/revcache, replacing it with the default go testrunner with subtests and stretchr/testify/assert.

Remove some unused functions instead of updating their tests.

Completely aside: change NewKey to return a Key by value instead of a reference. It is used as value everywhere and it just hurt my eyes.

Fixes #3016


Patch:
diff --git a/go.mod b/go.mod
index 8561b32bb..4bfaae221 100644
--- a/go.mod
+++ b/go.mod
@@ -34,7 +34,6 @@ require (
 	github.com/prometheus/procfs v0.14.0
 	github.com/quic-go/quic-go v0.43.1
 	github.com/sergi/go-diff v1.3.1
-	github.com/smartystreets/goconvey v1.8.1
 	github.com/songgao/water v0.0.0-20200317203138-2b4b6d7c09d8
 	github.com/spf13/cobra v1.8.0
 	github.com/spf13/pflag v1.0.5
@@ -75,14 +74,12 @@ require (
 	github.com/google/go-querystring v1.1.0 // indirect
 	github.com/google/pprof v0.0.0-20240509144519-723abb6459b7 // indirect
 	github.com/google/uuid v1.6.0 // indirect
-	github.com/gopherjs/gopherjs v1.17.2 // indirect
 	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
 	github.com/hashicorp/hcl v1.0.0 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/invopop/yaml v0.3.1 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/josharian/native v1.1.0 // indirect
-	github.com/jtolds/gls v4.20.0+incompatible // indirect
 	github.com/lestrrat-go/backoff/v2 v2.0.8 // indirect
 	github.com/lestrrat-go/blackmagic v1.0.2 // indirect
 	github.com/lestrrat-go/httpcc v1.0.1 // indirect
@@ -107,7 +104,6 @@ require (
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/sagikazarmark/locafero v0.4.0 // indirect
 	github.com/sagikazarmark/slog-shim v0.1.0 // indirect
-	github.com/smarty/assertions v1.16.0 // indirect
 	github.com/sourcegraph/conc v0.3.0 // indirect
 	github.com/spf13/afero v1.11.0 // indirect
 	github.com/spf13/cast v1.6.0 // indirect
diff --git a/go.sum b/go.sum
index 02446fd3d..90e7b0c62 100644
--- a/go.sum
+++ b/go.sum
@@ -106,8 +106,6 @@ github.com/google/pprof v0.0.0-20240509144519-723abb6459b7 h1:velgFPYr1X9TDwLIfk
 github.com/google/pprof v0.0.0-20240509144519-723abb6459b7/go.mod h1:kf6iHlnVGwgKolg33glAes7Yg/8iWP8ukqeldJSO7jw=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
-github.com/gopherjs/gopherjs v1.17.2 h1:fQnZVsXk8uxXIStYb0N4bGk7jeyTalG/wsZjQ25dO0g=
-github.com/gopherjs/gopherjs v1.17.2/go.mod h1:pRRIvn/QzFLrKfvEz3qUuEhtE/zLCWfreZ6J5gM2i+k=
 github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 h1:UH//fgunKIs4JdUbpDl1VZCDaL56wXCB/5+wF6uHfaI=
 github.com/grpc-ecosystem/go-grpc-middleware v1.4.0/go.mod h1:g5qyo/la0ALbONm6Vbp88Yd8NsDy6rZz+RcrMPxvld8=
 github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 h1:Ovs26xHkKqVztRpIrF/92BcuyuQ/YW4NSIpoGtfXNho=
@@ -131,8 +129,6 @@ github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFF
 github.com/josharian/native v1.0.0/go.mod h1:7X/raswPFr05uY3HiLlYeyQntB6OO7E/d2Cu7qoaN2w=
 github.com/josharian/native v1.1.0 h1:uuaP0hAbW7Y4l0ZRQ6C9zfb7Mg1mbFKry/xzDAfmtLA=
 github.com/josharian/native v1.1.0/go.mod h1:7X/raswPFr05uY3HiLlYeyQntB6OO7E/d2Cu7qoaN2w=
-github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo=
-github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU=
 github.com/juju/gnuflag v0.0.0-20171113085948-2ce1bb71843d/go.mod h1:2PavIy+JPciBPrBUjwbNvtwB6RQlve+hkpll6QSNmOE=
 github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
@@ -239,10 +235,6 @@ github.com/sagikazarmark/slog-shim v0.1.0/go.mod h1:SrcSrq8aKtyuqEI1uvTDTK1arOWR
 github.com/sergi/go-diff v1.3.1 h1:xkr+Oxo4BOQKmkn/B9eMK0g5Kg/983T9DqqPHwYqD+8=
 github.com/sergi/go-diff v1.3.1/go.mod h1:aMJSSKb2lpPvRNec0+w3fl7LP9IOFzdc9Pa4NFbPK1I=
 github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
-github.com/smarty/assertions v1.16.0 h1:EvHNkdRA4QHMrn75NZSoUQ/mAUXAYWfatfB01yTCzfY=
-github.com/smarty/assertions v1.16.0/go.mod h1:duaaFdCS0K9dnoM50iyek/eYINOZ64gbh1Xlf6LG7AI=
-github.com/smartystreets/goconvey v1.8.1 h1:qGjIddxOk4grTu9JPOU31tVfq3cNdBlNa5sSznIX1xY=
-github.com/smartystreets/goconvey v1.8.1/go.mod h1:+/u4qLyY6x1jReYOp7GOM2FSt8aP9CzCZL03bI28W60=
 github.com/songgao/water v0.0.0-20200317203138-2b4b6d7c09d8 h1:TG/diQgUe0pntT/2D9tmUCz4VNwm9MfrtPr0SU2qSX8=
 github.com/songgao/water v0.0.0-20200317203138-2b4b6d7c09d8/go.mod h1:P5HUIBuIWKbyjl083/loAegFkfbFNx5i2qEP4CNbm7E=
 github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo=
diff --git a/go_deps.bzl b/go_deps.bzl
index 573a52770..b775a95c5 100644
--- a/go_deps.bzl
+++ b/go_deps.bzl
@@ -559,12 +559,6 @@ def go_deps():
         sum = "h1:zC34cGQu69FG7qzJ3WiKW244WfhDC3xxYMeNOX2gtUQ=",
         version = "v0.0.0-20210719221736-1c9a4c676720",
     )
-    go_repository(
-        name = "com_github_gopherjs_gopherjs",
-        importpath = "github.com/gopherjs/gopherjs",
-        sum = "h1:fQnZVsXk8uxXIStYb0N4bGk7jeyTalG/wsZjQ25dO0g=",
-        version = "v1.17.2",
-    )
     go_repository(
         name = "com_github_gorilla_css",
         importpath = "github.com/gorilla/css",
@@ -721,12 +715,6 @@ def go_deps():
         sum = "h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=",
         version = "v1.1.12",
     )
-    go_repository(
-        name = "com_github_jtolds_gls",
-        importpath = "github.com/jtolds/gls",
-        sum = "h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo=",
-        version = "v4.20.0+incompatible",
-    )
     go_repository(
         name = "com_github_juju_gnuflag",
         importpath = "github.com/juju/gnuflag",
@@ -1027,18 +1015,6 @@ def go_deps():
         sum = "h1:bY0MQC28UADQmHmaF5dgpLmImcShSi2kHU9XLdhx/f4=",
         version = "v0.1.9",
     )
-    go_repository(
-        name = "com_github_neelance_astrewrite",
-        importpath = "github.com/neelance/astrewrite",
-        sum = "h1:D6paGObi5Wud7xg83MaEFyjxQB1W5bz5d0IFppr+ymk=",
-        version = "v0.0.0-20160511093645-99348263ae86",
-    )
-    go_repository(
-        name = "com_github_neelance_sourcemap",
-        importpath = "github.com/neelance/sourcemap",
-        sum = "h1:bY6ktFuJkt+ZXkX0RChQch2FtHpWQLVS8Qo1YasiIVk=",
-        version = "v0.0.0-20200213170602-2833bce08e4c",
-    )
     go_repository(
         name = "com_github_niemeyer_pretty",
         importpath = "github.com/niemeyer/pretty",
@@ -1213,42 +1189,12 @@ def go_deps():
         sum = "h1:KkH3I3sJuOLP3TjA/dfr4NAY8bghDwnXiU7cTKxQqo0=",
         version = "v0.0.0-20220729165902-8cddb4f5de06",
     )
-    go_repository(
-        name = "com_github_shurcool_go",
-        importpath = "github.com/shurcooL/go",
-        sum = "h1:aSISeOcal5irEhJd1M+IrApc0PdcN7e7Aj4yuEnOrfQ=",
-        version = "v0.0.0-20200502201357-93f07166e636",
-    )
-    go_repository(
-        name = "com_github_shurcool_httpfs",
-        importpath = "github.com/shurcooL/httpfs",
-        sum = "h1:bUGsEnyNbVPw06Bs80sCeARAlK8lhwqGyi6UT8ymuGk=",
-        version = "v0.0.0-20190707220628-8d4bc4ba7749",
-    )
-    go_repository(
-        name = "com_github_shurcool_vfsgen",
-        importpath = "github.com/shurcooL/vfsgen",
-        sum = "h1:pXY9qYc/MP5zdvqWEUH6SjNiu7VhSjuVFTFiTcphaLU=",
-        version = "v0.0.0-20200824052919-0d455de96546",
-    )
     go_repository(
         name = "com_github_sirupsen_logrus",
         importpath = "github.com/sirupsen/logrus",
         sum = "h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=",
         version = "v1.8.1",
     )
-    go_repository(
-        name = "com_github_smarty_assertions",
-        importpath = "github.com/smarty/assertions",
-        sum = "h1:EvHNkdRA4QHMrn75NZSoUQ/mAUXAYWfatfB01yTCzfY=",
-        version = "v1.16.0",
-    )
-    go_repository(
-        name = "com_github_smartystreets_goconvey",
-        importpath = "github.com/smartystreets/goconvey",
-        sum = "h1:qGjIddxOk4grTu9JPOU31tVfq3cNdBlNa5sSznIX1xY=",
-        version = "v1.8.1",
-    )
     go_repository(
         name = "com_github_songgao_water",
         importpath = "github.com/songgao/water",
diff --git a/private/revcache/BUILD.bazel b/private/revcache/BUILD.bazel
index 2c1672930..99bea4419 100644
--- a/private/revcache/BUILD.bazel
+++ b/private/revcache/BUILD.bazel
@@ -33,6 +33,6 @@ go_test(
         "//pkg/segment:go_default_library",
         "//private/revcache/mock_revcache: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",
     ],
 )
diff --git a/private/revcache/memrevcache/BUILD.bazel b/private/revcache/memrevcache/BUILD.bazel
index 10550d693..b6b7e5a6b 100644
--- a/private/revcache/memrevcache/BUILD.bazel
+++ b/private/revcache/memrevcache/BUILD.bazel
@@ -20,6 +20,5 @@ go_test(
         "//pkg/private/ctrl/path_mgmt:go_default_library",
         "//private/revcache:go_default_library",
         "//private/revcache/revcachetest:go_default_library",
-        "@com_github_smartystreets_goconvey//convey:go_default_library",
     ],
 )
diff --git a/private/revcache/revcache.go b/private/revcache/revcache.go
index a57e4e3f3..c369a6111 100644
--- a/private/revcache/revcache.go
+++ b/private/revcache/revcache.go
@@ -32,8 +32,8 @@ type Key struct {
 }
 
 // NewKey creates a new key for the revocation cache.
-func NewKey(ia addr.IA, ifId common.IFIDType) *Key {
-	return &Key{
+func NewKey(ia addr.IA, ifId common.IFIDType) Key {
+	return Key{
 		IA:   ia,
 		IfId: ifId,
 	}
@@ -48,7 +48,7 @@ type KeySet map[Key]struct{}
 
 // SingleKey is a convenience function to return a KeySet with a single key.
 func SingleKey(ia addr.IA, ifId common.IFIDType) KeySet {
-	return KeySet{*NewKey(ia, ifId): {}}
+	return KeySet{Key{IA: ia, IfId: ifId}: {}}
 }
 
 // RevOrErr is either a revocation or an error.
@@ -86,48 +86,3 @@ type RevCache interface {
 
 // Revocations is the map of revocations.
 type Revocations map[Key]*path_mgmt.RevInfo
-
-// RevocationToMap converts a slice of revocations to a revocation map. If extracting the info field
-// fails from a revocation, nil and the error is returned.
-func RevocationToMap(revs []*path_mgmt.RevInfo) (Revocations, error) {
-	res := make(Revocations)
-	for _, rev := range revs {
-		res[Key{IA: rev.IA(), IfId: rev.IfID}] = rev
-	}
-	return res, nil
-}
-
-// Keys returns the set of keys in this revocation map.
-func (r Revocations) Keys() KeySet {
-	keys := make(KeySet, len(r))
-	for key := range r {
-		keys[key] = struct{}{}
-	}
-	return keys
-}
-
-// ToSlice extracts the values from this revocation map.
-func (r Revocations) ToSlice() []*path_mgmt.RevInfo {
-	res := make([]*path_mgmt.RevInfo, 0, len(r))
-	for _, rev := range r {
-		res = append(res, rev)
-	}
-	return res
-}
-
-// FilterNew drops all revocations in r that are already in the revCache.
-func (r Revocations) FilterNew(ctx context.Context, revCache RevCache) error {
-	inCache, err := revCache.Get(ctx, r.Keys())
-	if err != nil {
-		return err
-	}
-	for key, rev := range r {
-		existingRev, exists := inCache[key]
-		if exists {
-			if !newerInfo(existingRev, rev) {
-				delete(r, key)
-			}
-		}
-	}
-	return nil
-}
diff --git a/private/revcache/util.go b/private/revcache/util.go
index 8d1dad4ca..bbd13fd1f 100644
--- a/private/revcache/util.go
+++ b/private/revcache/util.go
@@ -19,7 +19,6 @@ import (
 
 	"github.com/scionproto/scion/pkg/addr"
 	"github.com/scionproto/scion/pkg/private/common"
-	"github.com/scionproto/scion/pkg/private/ctrl/path_mgmt"
 	seg "github.com/scionproto/scion/pkg/segment"
 	"github.com/scionproto/scion/private/storage/cleaner"
 )
@@ -32,28 +31,6 @@ func NewCleaner(rc RevCache, s string) *cleaner.Cleaner {
 	}, s)
 }
 
-// FilterNew filters the given revocations against the revCache, only the ones
-// which are not in the cache are returned. This is a convenience wrapper
-// around the Revocations type and its filter new method.
-func FilterNew(ctx context.Context, revCache RevCache,
-	revocations []*path_mgmt.RevInfo) ([]*path_mgmt.RevInfo, error) {
-
-	rMap, err := RevocationToMap(revocations)
-	if err != nil {
-		return nil, err
-	}
-	if err = rMap.FilterNew(ctx, revCache); err != nil {
-		return nil, err
-	}
-	return rMap.ToSlice(), nil
-}
-
-// newerInfo returns whether the received info is newer than the existing.
-func newerInfo(existing, received *path_mgmt.RevInfo) bool {
-	return !received.SameIntf(existing) ||
-		received.Timestamp().After(existing.Timestamp())
-}
-
 // NoRevokedHopIntf returns true if there is no on-segment revocation.
 func NoRevokedHopIntf(ctx context.Context, revCache RevCache,
 	s *seg.PathSegment) (bool, error) {
@@ -64,34 +41,15 @@ func NoRevokedHopIntf(ctx context.Context, revCache RevCache,
 	return len(revs) == 0, err
 }
 
-// RelevantRevInfos finds all revocations for the given segments.
-func RelevantRevInfos(ctx context.Context, revCache RevCache,
-	allSegs ...[]*seg.PathSegment) ([]*path_mgmt.RevInfo, error) {
-
-	revKeys := make(KeySet)
-	for _, segs := range allSegs {
-		addRevKeys(segs, revKeys, false)
-	}
-	revs, err := revCache.Get(ctx, revKeys)
-	if err != nil {
-		return nil, err
-	}
-	allRevs := make([]*path_mgmt.RevInfo, 0, len(revs))
-	for _, rev := range revs {
-		allRevs = append(allRevs, rev)
-	}
-	return allRevs, nil
-}
-
 // addRevKeys adds all revocations keys for the given segments to the keys set.
 // If hopOnly is set, only the first hop entry is considered.
 func addRevKeys(segs []*seg.PathSegment, keys KeySet, hopOnly bool) {
 	addIntfs := func(ia addr.IA, ingress, egress uint16) {
 		if ingress != 0 {
-			keys[*NewKey(ia, common.IFIDType(ingress))] = struct{}{}
+			keys[Key{IA: ia, IfId: common.IFIDType(ingress)}] = struct{}{}
 		}
 		if egress != 0 {
-			keys[*NewKey(ia, common.IFIDType(egress))] = struct{}{}
+			keys[Key{IA: ia, IfId: common.IFIDType(egress)}] = struct{}{}
 		}
 	}
 	for _, s := range segs {


Test patch:
diff --git a/private/revcache/memrevcache/memrevcache_test.go b/private/revcache/memrevcache/memrevcache_test.go
index bb213d68a..1ee2ff509 100644
--- a/private/revcache/memrevcache/memrevcache_test.go
+++ b/private/revcache/memrevcache/memrevcache_test.go
@@ -19,8 +19,6 @@ import (
 	"testing"
 	"time"
 
-	. "github.com/smartystreets/goconvey/convey"
-
 	"github.com/scionproto/scion/pkg/private/ctrl/path_mgmt"
 	"github.com/scionproto/scion/private/revcache"
 	"github.com/scionproto/scion/private/revcache/revcachetest"
@@ -54,7 +52,5 @@ func (c *testRevCache) Prepare(t *testing.T, _ context.Context) {
 }
 
 func TestRevCacheSuite(t *testing.T) {
-	Convey("RevCache Suite", t, func() {
-		revcachetest.TestRevCache(t, &testRevCache{memRevCache: New()})
-	})
+	revcachetest.TestRevCache(t, &testRevCache{memRevCache: New()})
 }
diff --git a/private/revcache/revcachetest/BUILD.bazel b/private/revcache/revcachetest/BUILD.bazel
index 6c10abce8..19a4187ee 100644
--- a/private/revcache/revcachetest/BUILD.bazel
+++ b/private/revcache/revcachetest/BUILD.bazel
@@ -12,7 +12,6 @@ go_library(
         "//pkg/private/ctrl/path_mgmt/proto:go_default_library",
         "//pkg/private/util:go_default_library",
         "//private/revcache: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",
     ],
diff --git a/private/revcache/revcachetest/revcachetest.go b/private/revcache/revcachetest/revcachetest.go
index 64dedaf31..12e9f21f4 100644
--- a/private/revcache/revcachetest/revcachetest.go
+++ b/private/revcache/revcachetest/revcachetest.go
@@ -20,7 +20,6 @@ import (
 	"testing"
 	"time"
 
-	. "github.com/smartystreets/goconvey/convey"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -57,23 +56,25 @@ type TestableRevCache interface {
 // setup should return a RevCache in a clean state, i.e. no entries in the cache.
 // cleanup can be used to release any resources that have been allocated during setup.
 func TestRevCache(t *testing.T, revCache TestableRevCache) {
-	testWrapper := func(test func(*testing.T, TestableRevCache)) func() {
-		return func() {
+	subtest := func(name string, test func(*testing.T, TestableRevCache)) {
+		t.Helper()
+		t.Run(name, func(t *testing.T) {
 			prepareCtx, cancelF := context.WithTimeout(context.Background(), TimeOut)
 			defer cancelF()
 			revCache.Prepare(t, prepareCtx)
 			test(t, revCache)
-		}
+		})
 	}
-	Convey("InsertGet", testWrapper(testInsertGet))
-	Convey("GetMultikey", testWrapper(testGetMultikey))
-	Convey("GetAll", testWrapper(testGetAll))
-	Convey("GetAllExpired", testWrapper(testGetAllExpired))
-	Convey("InsertExpired", testWrapper(testInsertExpired))
-	Convey("InsertNewer", testWrapper(testInsertNewer))
-	Convey("GetExpired", testWrapper(testGetExpired))
-	Convey("GetMultikeyExpired", testWrapper(testGetMuliKeysExpired))
-	Convey("DeleteExpired", testWrapper(testDeleteExpired))
+
+	subtest("InsertGet", testInsertGet)
+	subtest("GetMultikey", testGetMultikey)
+	subtest("GetAll", testGetAll)
+	subtest("GetAllExpired", testGetAllExpired)
+	subtest("InsertExpired", testInsertExpired)
+	subtest("InsertNewer", testInsertNewer)
+	subtest("GetExpired", testGetExpired)
+	subtest("GetMultikeyExpired", testGetMuliKeysExpired)
+	subtest("DeleteExpired", testDeleteExpired)
 }
 
 func testInsertGet(t *testing.T, revCache TestableRevCache) {
@@ -81,19 +82,19 @@ func testInsertGet(t *testing.T, revCache TestableRevCache) {
 	ctx, cancelF := context.WithTimeout(context.Background(), TimeOut)
 	defer cancelF()
 	inserted, err := revCache.Insert(ctx, rev)
-	SoMsg("Insert should return true for a new entry", inserted, ShouldBeTrue)
-	SoMsg("Insert a new entry should not err", err, ShouldBeNil)
-	key1 := *revcache.NewKey(ia110, ifId15)
+	assert.True(t, inserted, "Insert should return true for a new entry")
+	assert.NoError(t, err, "Insert a new entry should not err")
+	key1 := revcache.NewKey(ia110, ifId15)
 	revs, err := revCache.Get(ctx, revcache.KeySet{key1: {}})
-	SoMsg("Get should not err for existing entry", err, ShouldBeNil)
-	SoMsg("Get should return existing entry", revs, ShouldNotBeEmpty)
-	SoMsg("Get should return previously inserted value", revs[key1], ShouldResemble, rev)
+	assert.NoError(t, err, "Get should not err for existing entry")
+	assert.NotEmpty(t, revs, "Get should return existing entry")
+	assert.Equal(t, rev, revs[key1], "Get should return previously inserted value")
 	inserted, err = revCache.Insert(ctx, rev)
-	SoMsg("Insert should return false for already existing entry", inserted, ShouldBeFalse)
-	SoMsg("Insert should not err", err, ShouldBeNil)
+	assert.False(t, inserted, "Insert should return false for already existing entry")
+	assert.NoError(t, err, "Insert should not err")
 	revs, err = revCache.Get(ctx, revcache.SingleKey(ia110, ifId19))
-	SoMsg("Get should not err", err, ShouldBeNil)
-	SoMsg("Get should return empty result for not present value", revs, ShouldBeEmpty)
+	assert.NoError(t, err, "Get should not err")
+	assert.Empty(t, revs, "Get should return empty result for not present value")
 }
 
 func testGetMultikey(t *testing.T, revCache TestableRevCache) {
@@ -106,8 +107,8 @@ func testGetMultikey(t *testing.T, revCache TestableRevCache) {
 
 	// First test the empty cache
 	revs, err := revCache.Get(ctx, revcache.KeySet{})
-	SoMsg("Get should not err", err, ShouldBeNil)
-	SoMsg("Should return no revs", revs, ShouldBeEmpty)
+	assert.NoError(t, err, "Get should not err")
+	assert.Empty(t, revs, "Should return no revs")
 
 	_, err = revCache.Insert(ctx, rev1)
 	require.NoError(t, err)
@@ -118,24 +119,23 @@ func testGetMultikey(t *testing.T, revCache TestableRevCache) {
 	_, err = revCache.Insert(ctx, rev4)
 	require.NoError(t, err)
 
-	key1 := *revcache.NewKey(ia110, ifId15)
+	key1 := revcache.NewKey(ia110, ifId15)
 	revs, err = revCache.Get(ctx, revcache.KeySet{key1: {}})
-	SoMsg("Get should not err", err, ShouldBeNil)
-	SoMsg("Should contain one rev", 1, ShouldEqual, len(revs))
-	SoMsg("Get should return revs for the given keys", revs, ShouldResemble,
-		revcache.Revocations{key1: rev1})
+	assert.NoError(t, err, "Get should not err")
+	assert.Equal(t, len(revs), 1, "Should contain one rev")
+	assert.Equal(t, revcache.Revocations{key1: rev1}, revs,
+		"Get should return revs for the given keys")
 
-	key2 := *revcache.NewKey(ia110, ifId19)
-	key3 := *revcache.NewKey(ia120, ifId15)
-	key4 := *revcache.NewKey(ia120, ifId19) // not the key of sr4
+	key2 := revcache.NewKey(ia110, ifId19)
+	key3 := revcache.NewKey(ia120, ifId15)
+	key4 := revcache.NewKey(ia120, ifId19) // not the key of sr4
 	searchKeys := revcache.KeySet{key1: {}, key2: {}, key3: {}, key4: {}}
 	revs, err = revCache.Get(ctx, searchKeys)
-	SoMsg("Get should not err", err, ShouldBeNil)
+	assert.NoError(t, err, "Get should not err")
 	expectedResult := revcache.Revocations{
 		key1: rev1, key2: rev2, key3: rev3,
 	}
-	SoMsg("Get should return the requested revocations", revs, ShouldResemble,
-		expectedResult)
+	assert.Equal(t, expectedResult, revs, "Get should return the requested revocations")
 }
 
 func testGetAll(t *testing.T, revCache TestableRevCache) {
@@ -143,10 +143,10 @@ func testGetAll(t *testing.T, revCache TestableRevCache) {
 	defer cancelF()
 	// Empty cache should return an empty chan
 	resChan, err := revCache.GetAll(ctx)
-	SoMsg("No error expected", err, ShouldBeNil)
+	assert.NoError(t, err)
 	res, more := <-resChan
-	SoMsg("No result expected", res, ShouldResemble, revcache.RevOrErr{})
-	SoMsg("No more entries expected", more, ShouldBeFalse)
+	assert.Equal(t, revcache.RevOrErr{}, res, "No result expected")
+	assert.False(t, more, "No more entries expected")
 
 	// Insert some stuff and query again
 	rev1 := defaultRevInfo(ia110, ifId15)
@@ -165,11 +165,11 @@ func testGetAll(t *testing.T, revCache TestableRevCache) {
 	expectedRevs := []*path_mgmt.RevInfo{rev1, rev2, rev3, rev4}
 
 	resChan, err = revCache.GetAll(ctx)
-	SoMsg("No error expected", err, ShouldBeNil)
+	assert.NoError(t, err)
 	revs := make([]*path_mgmt.RevInfo, 0, len(expectedRevs))
 	for res := range resChan {
-		SoMsg("No error expected", res.Err, ShouldBeNil)
-		SoMsg("Revocation expected", res.Rev, ShouldNotBeNil)
+		assert.NoError(t, res.Err)
+		assert.NotNil(t, res.Rev, "Revocation expected")
 		revs = append(revs, res.Rev)
 	}
 	// we don't care about the order, so sort here to make sure the comparison always works.
@@ -179,7 +179,7 @@ func testGetAll(t *testing.T, revCache TestableRevCache) {
 		return iInfo.IA() < jInfo.IA() ||
 			(iInfo.IA() == jInfo.IA() && iInfo.IfID < jInfo.IfID)
 	})
-	SoMsg("All revocations should have been returned", revs, ShouldResemble, expectedRevs)
+	assert.Equal(t, expectedRevs, revs, "All revocations should have been returned")
 }
 
 func testGetAllExpired(t *testing.T, revCache TestableRevCache) {
@@ -196,10 +196,10 @@ func testGetAllExpired(t *testing.T, revCache TestableRevCache) {
 	revCache.InsertExpired(t, ctx, revNew)
 	// Now test that we don't get the expired rev
 	resChan, err := revCache.GetAll(ctx)
-	SoMsg("No error expected", err, ShouldBeNil)
+	assert.NoError(t, err)
 	res, more := <-resChan
-	SoMsg("No result expected", res, ShouldResemble, revcache.RevOrErr{})
-	SoMsg("No more entries expected", more, ShouldBeFalse)
+	assert.Equal(t, revcache.RevOrErr{}, res, "No result expected")
+	assert.False(t, more, "No more entries expected")
 }
 
 func testInsertExpired(t *testing.T, revCache TestableRevCache) {
@@ -213,8 +213,8 @@ func testInsertExpired(t *testing.T, revCache TestableRevCache) {
 	ctx, cancelF := context.WithTimeout(context.Background(), TimeOut)
 	defer cancelF()
 	inserted, err := revCache.Insert(ctx, r)
-	SoMsg("Insert should return false for expired rev", inserted, ShouldBeFalse)
-	SoMsg("Insert should not err", err, ShouldBeNil)
+	assert.False(t, inserted, "Insert should return false for expired rev")
+	assert.NoError(t, err, "Insert should not err")
 }
 
 func testInsertNewer(t *testing.T, revCache TestableRevCache) {
@@ -232,13 +232,13 @@ func testInsertNewer(t *testing.T, revCache TestableRevCache) {
 	}
 	require.NoError(t, err)
 	inserted, err := revCache.Insert(ctx, revNew)
-	SoMsg("Insert should return true for a new entry", inserted, ShouldBeTrue)
-	SoMsg("Insert a new entry should not err", err, ShouldBeNil)
-	key1 := *revcache.NewKey(ia110, ifId15)
+	assert.True(t, inserted, "Insert should return true for a new entry")
+	assert.NoError(t, err, "Insert a new entry should not err")
+	key1 := revcache.NewKey(ia110, ifId15)
 	revs, err := revCache.Get(ctx, revcache.KeySet{key1: {}})
-	SoMsg("Get should not err for existing entry", err, ShouldBeNil)
-	SoMsg("Get should return non empty map for inserted value", revs, ShouldNotBeEmpty)
-	SoMsg("Get should return previously inserted value", revs[key1], ShouldResemble, revNew)
+	assert.NoError(t, err, "Get should not err for existing entry")
+	assert.NotEmpty(t, revs, "Get should return non empty map for inserted value")
+	assert.Equal(t, revNew, revs[key1], "Get should return previously inserted value")
 }
 
 func testGetExpired(t *testing.T, revCache TestableRevCache) {
@@ -253,8 +253,8 @@ func testGetExpired(t *testing.T, revCache TestableRevCache) {
 	}
 	revCache.InsertExpired(t, ctx, revNew)
 	revs, err := revCache.Get(ctx, revcache.SingleKey(ia110, ifId15))
-	SoMsg("Expired entry should not be returned", revs, ShouldBeEmpty)
-	SoMsg("Should not error for expired entry", err, ShouldBeNil)
+	assert.Empty(t, revs, "Expired entry should not be returned")
+	assert.NoError(t, err, "Should not error for expired entry")
 }
 
 func testGetMuliKeysExpired(t *testing.T, revCache TestableRevCache) {
@@ -271,28 +271,28 @@ func testGetMuliKeysExpired(t *testing.T, revCache TestableRevCache) {
 	rev110_19 := defaultRevInfo(ia110, ifId19)
 	_, err := revCache.Insert(ctx, rev110_19)
 	assert.NoError(t, err)
-	validKey := *revcache.NewKey(ia110, ifId19)
+	validKey := revcache.NewKey(ia110, ifId19)
 	srCache, err := revCache.Get(ctx, revcache.KeySet{
-		*revcache.NewKey(ia110, ifId15): {},
-		validKey:                        {},
+		revcache.NewKey(ia110, ifId15): {},
+		validKey:                       {},
 	})
-	SoMsg("Should not error for expired entry", err, ShouldBeNil)
-	SoMsg("Expired entry should not be returned", srCache, ShouldResemble,
-		revcache.Revocations{validKey: rev110_19})
+	assert.NoError(t, err, "Should not error for expired entry")
+	assert.Equal(t, revcache.Revocations{validKey: rev110_19}, srCache,
+		"Expired entry should not be returned")
 }
 
 func testDeleteExpired(t *testing.T, revCache TestableRevCache) {
 	ctx, cancelF := context.WithTimeout(context.Background(), TimeOut)
 	defer cancelF()
 	del, err := revCache.DeleteExpired(ctx)
-	SoMsg("DeleteExpired on empty should not error", err, ShouldBeNil)
-	SoMsg("DeleteExpired on empty should delete 0", del, ShouldEqual, 0)
+	assert.NoError(t, err, "DeleteExpired on empty should not error")
+	assert.EqualValues(t, 0, del, "DeleteExpired on empty should delete 0")
 	rev110_19 := defaultRevInfo(ia110, ifId19)
 	_, err = revCache.Insert(ctx, rev110_19)
 	assert.NoError(t, err)
 	del, err = revCache.DeleteExpired(ctx)
-	SoMsg("DeleteExpired should not error", err, ShouldBeNil)
-	SoMsg("DeleteExpired should delete 0 if entry is not expired", del, ShouldEqual, 0)
+	assert.NoError(t, err, "DeleteExpired should not error")
+	assert.EqualValues(t, 0, del, "DeleteExpired should delete 0 if entry is not expired")
 	revNew := &path_mgmt.RevInfo{
 		IfID:         ifId15,
 		RawIsdas:     ia110,
@@ -302,11 +302,11 @@ func testDeleteExpired(t *testing.T, revCache TestableRevCache) {
 	}
 	revCache.InsertExpired(t, ctx, revNew)
 	del, err = revCache.DeleteExpired(ctx)
-	SoMsg("DeleteExpired should not error", err, ShouldBeNil)
-	SoMsg("DeleteExpired should delete 1 if entry is expired", del, ShouldEqual, 1)
+	assert.NoError(t, err, "DeleteExpired should not error")
+	assert.EqualValues(t, 1, del, "DeleteExpired should delete 1 if entry is expired")
 	del, err = revCache.DeleteExpired(ctx)
-	SoMsg("DeleteExpired should not error", err, ShouldBeNil)
-	SoMsg("DeleteExpired should delete 0 if entry is not expired", del, ShouldEqual, 0)
+	assert.NoError(t, err, "DeleteExpired should not error")
+	assert.EqualValues(t, 0, del, "DeleteExpired should delete 0 if entry is not expired")
 }
 
 func defaultRevInfo(ia addr.IA, ifId common.IFIDType) *path_mgmt.RevInfo {
diff --git a/private/revcache/util_test.go b/private/revcache/util_test.go
index 50cf65070..ddde37b56 100644
--- a/private/revcache/util_test.go
+++ b/private/revcache/util_test.go
@@ -20,7 +20,7 @@ import (
 	"time"
 
 	"github.com/golang/mock/gomock"
-	. "github.com/smartystreets/goconvey/convey"
+	"github.com/stretchr/testify/assert"
 
 	"github.com/scionproto/scion/pkg/addr"
 	"github.com/scionproto/scion/pkg/private/common"
@@ -35,135 +35,48 @@ import (
 )
 
 var (
-	ia110  = addr.MustParseIA("1-ff00:0:110")
-	ia211  = addr.MustParseIA("2-ff00:0:211")
-	ifid10 = uint16(10)
-	ifid11 = uint16(11)
+	ia211 = addr.MustParseIA("2-ff00:0:211")
 
 	timeout = time.Second
 )
 
-func TestFilterNew(t *testing.T) {
+func TestNoRevokedHopIntf(t *testing.T) {
 	now := time.Now()
-	sr10 := defaultRevInfo(ia110, ifid10, now)
-	sr11 := defaultRevInfo(ia110, ifid11, now)
-	sr11Old := defaultRevInfo(ia110, ifid11, now.Add(-10*time.Second))
-	Convey("TestFilterNew", t, func() {
-		ctrl := gomock.NewController(t)
-		defer ctrl.Finish()
-		revCache := mock_revcache.NewMockRevCache(ctrl)
-		Convey("Given an empty cache", func() {
-			revCache.EXPECT().Get(gomock.Any(), gomock.Any()).Return(nil, nil)
-			rMap, err := revcache.RevocationToMap([]*path_mgmt.RevInfo{sr10})
-			expectedMap := copy(rMap)
-			SoMsg("No error expected", err, ShouldBeNil)
-			err = rMap.FilterNew(context.Background(), revCache)
-			SoMsg("No error expected", err, ShouldBeNil)
-			SoMsg("All revocations should be considered new", rMap, ShouldResemble, expectedMap)
-		})
-		Convey("Given a cache with an old revocation", func() {
-			revCache.EXPECT().Get(gomock.Any(), gomock.Any()).Return(revcache.Revocations{
-				revcache.Key{IA: ia110, IfId: common.IFIDType(ifid11)}: sr11Old,
-			}, nil)
-			rMap, err := revcache.RevocationToMap([]*path_mgmt.RevInfo{sr10, sr11})
-			expectedMap := copy(rMap)
-			SoMsg("No error expected", err, ShouldBeNil)
-			err = rMap.FilterNew(context.Background(), revCache)
-			SoMsg("No error expected", err, ShouldBeNil)
-			SoMsg("All revocations should be considered new", rMap, ShouldResemble, expectedMap)
-		})
-		Convey("Given a cache with a newer revocation", func() {
-			revCache.EXPECT().Get(gomock.Any(), gomock.Any()).Return(revcache.Revocations{
-				revcache.Key{IA: ia110, IfId: common.IFIDType(ifid11)}: sr11,
-			}, nil)
-			rMap, err := revcache.RevocationToMap([]*path_mgmt.RevInfo{sr10, sr11Old})
-			expectedMap := copy(rMap)
-			delete(expectedMap, revcache.Key{IA: ia110, IfId: common.IFIDType(ifid11)})
-			SoMsg("No error expected", err, ShouldBeNil)
-			err = rMap.FilterNew(context.Background(), revCache)
-			SoMsg("No error expected", err, ShouldBeNil)
-			SoMsg("Only new revocations should stay in map", rMap, ShouldResemble, expectedMap)
-		})
-		Convey("Given a cache with an error", func() {
-			expectedErr := serrors.New("TESTERR")
-			revCache.EXPECT().Get(gomock.Any(), gomock.Any()).Return(nil, expectedErr)
-			rMap, err := revcache.RevocationToMap([]*path_mgmt.RevInfo{})
-			SoMsg("No error expected", err, ShouldBeNil)
-			err = rMap.FilterNew(context.Background(), revCache)
-			SoMsg("Error from cache expected", err, ShouldResemble, expectedErr)
-		})
-	})
 
-}
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	ctx, cancelF := context.WithTimeout(context.Background(), timeout)
+	defer cancelF()
+	seg210_222_1 := createSeg(ctrl)
 
-func TestNoRevokedHopIntf(t *testing.T) {
-	now := time.Now()
-	Convey("NoRevokedHopIntf", t, func() {
-		ctrl := gomock.NewController(t)
-		defer ctrl.Finish()
-		seg210_222_1 := createSeg(ctrl)
-		ctx, cancelF := context.WithTimeout(context.Background(), timeout)
-		defer cancelF()
+	t.Run("empty", func(t *testing.T) {
 		revCache := mock_revcache.NewMockRevCache(ctrl)
-		Convey("Given an empty revcache", func() {
-			revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any())
-			noR, err := revcache.NoRevokedHopIntf(ctx, revCache, seg210_222_1)
-			SoMsg("No err expected", err, ShouldBeNil)
-			SoMsg("No revocation expected", noR, ShouldBeTrue)
-		})
-		Convey("Given a revcache with an on segment revocation", func() {
-			sRev := defaultRevInfo(ia211, graph.If_210_X_211_A, now)
-			revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any()).Return(
-				revcache.Revocations{
-					revcache.Key{IA: addr.MustParseIA("2-ff00:0:211"),
-						IfId: common.IFIDType(graph.If_210_X_211_A)}: sRev,
-				}, nil,
-			)
-			noR, err := revcache.NoRevokedHopIntf(ctx, revCache, seg210_222_1)
-			SoMsg("No err expected", err, ShouldBeNil)
-			SoMsg("Revocation expected", noR, ShouldBeFalse)
-		})
-		Convey("Given an error in the revache it is propagated", func() {
-			revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any()).Return(
-				nil, serrors.New("TestError"),
-			)
-			_, err := revcache.NoRevokedHopIntf(ctx, revCache, seg210_222_1)
-			SoMsg("Err expected", err, ShouldNotBeNil)
-		})
+		revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any())
+		noR, err := revcache.NoRevokedHopIntf(ctx, revCache, seg210_222_1)
+		assert.NoError(t, err)
+		assert.True(t, noR, "no revocation expected")
 	})
-}
-
-func TestRelevantRevInfos(t *testing.T) {
-	Convey("TestRelevantRevInfos", t, func() {
-		ctx, cancelF := context.WithTimeout(context.Background(), timeout)
-		defer cancelF()
-		ctrl := gomock.NewController(t)
-		defer ctrl.Finish()
-		segs := []*seg.PathSegment{createSeg(ctrl)}
+	t.Run("on segment revocation", func(t *testing.T) {
+		sRev := defaultRevInfo(ia211, graph.If_210_X_211_A, now)
 		revCache := mock_revcache.NewMockRevCache(ctrl)
-		Convey("Given an empty revcache", func() {
-			revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any())
-			revs, err := revcache.RelevantRevInfos(ctx, revCache, segs)
-			SoMsg("No err expected", err, ShouldBeNil)
-			SoMsg("No revocation expected", revs, ShouldBeEmpty)
-		})
-		// TODO(lukedirtwalker): Add test with revocations
-		Convey("Given an error in the revache it is propagated", func() {
-			revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any()).Return(
-				nil, serrors.New("TestError"),
-			)
-			_, err := revcache.RelevantRevInfos(ctx, revCache, segs)
-			SoMsg("Err expected", err, ShouldNotBeNil)
-		})
+		revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any()).Return(
+			revcache.Revocations{
+				revcache.Key{IA: addr.MustParseIA("2-ff00:0:211"),
+					IfId: common.IFIDType(graph.If_210_X_211_A)}: sRev,
+			}, nil,
+		)
+		noR, err := revcache.NoRevokedHopIntf(ctx, revCache, seg210_222_1)
+		assert.NoError(t, err)
+		assert.False(t, noR, "revocation expected")
+	})
+	t.Run("error progation", func(t *testing.T) {
+		revCache := mock_revcache.NewMockRevCache(ctrl)
+		revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any()).Return(
+			nil, serrors.New("TestError"),
+		)
+		_, err := revcache.NoRevokedHopIntf(ctx, revCache, seg210_222_1)
+		assert.Error(t, err)
 	})
-}
-
-func copy(revs revcache.Revocations) revcache.Revocations {
-	res := make(revcache.Revocations, len(revs))
-	for k, v := range revs {
-		res[k] = v
-	}
-	return res
 }
 
 func defaultRevInfo(ia addr.IA, ifId uint16, ts time.Time) *path_mgmt.RevInfo {