diff --git a/go.mod b/go.mod index 8561b32bb3..4bfaae2219 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 02446fd3d1..90e7b0c623 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 573a52770f..b775a95c55 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 2c1672930f..99bea4419e 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 10550d6939..b6b7e5a6b7 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/memrevcache/memrevcache_test.go b/private/revcache/memrevcache/memrevcache_test.go index bb213d68ac..1ee2ff5095 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/revcache.go b/private/revcache/revcache.go index a57e4e3f39..c369a6111a 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/revcachetest/BUILD.bazel b/private/revcache/revcachetest/BUILD.bazel index 6c10abce83..19a4187eee 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 64dedaf310..12e9f21f46 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.go b/private/revcache/util.go index 8d1dad4ca4..bbd13fd1f9 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 { diff --git a/private/revcache/util_test.go b/private/revcache/util_test.go index 50cf650701..ddde37b569 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 {