Skip to content

Commit

Permalink
pkg/private: cleanup common, util, xtest packages (#4311)
Browse files Browse the repository at this point in the history
The pkg/private/common, util and xtest packages have rather fuzzy scope,
and have accumulated a bit of cruft and unused or outdated
functionality. Clean this up a bit:

* pkg/private/common: 
    * remove unused constants
    * remove outdated error handling helpers and replace remaining use
    * remove NativeOrder and IsBigEndian: No longer needed.
      Native byte order is not often needed, but will eventually show up
      in standard library anyway (golang/go#57237).
* pkg/private/util:
    * remove unused helper functionality
    * remove Checksum: only used to compute reference value in slayers
      test cases. Use a simpler, non-optimized implementation for this.
      Closes #4262.
    * move RunsInDocker to private/env
    * move ASList to tools/integration
* pkg/private/xtest: 
    * remove unused helpers
    * remove unused Callback and MockCallback
    * replace FailOnErr with require.NoError
    * replace AssertErrorsIs with assert.ErrorIs


There are still more things to clean up in `pkg/private`, in future PRs,
in particular: 
* `common.ErrMsg` should be integrated in `serrors`
* `common.IFIDType` should be removed or renamed and moved somewhere
  more appropriate
* Merge the remainder of `util` and `common` 
* Clean up  `LinkType` and `RevInfo` from `pkg/private/ctrl`
  • Loading branch information
matzf committed Feb 9, 2023
1 parent a0fcc9e commit 756c8b4
Show file tree
Hide file tree
Showing 59 changed files with 212 additions and 1,038 deletions.
2 changes: 1 addition & 1 deletion dispatcher/internal/registration/iatable_test.go
Expand Up @@ -98,7 +98,7 @@ func TestIATableRegister(t *testing.T) {
t.Run("already registered ports will cause error", func(t *testing.T) {
table := NewIATable(minPort, maxPort)
_, err := table.Register(ia, public, nil, addr.SvcNone, value)
xtest.FailOnErr(t, err)
require.NoError(t, err)
ref, err := table.Register(ia, public, nil, addr.SvcNone, value)
assert.Error(t, err)
assert.Nil(t, ref)
Expand Down
5 changes: 2 additions & 3 deletions dispatcher/internal/registration/scmp_table_test.go
Expand Up @@ -18,8 +18,7 @@ import (
"testing"

. "github.com/smartystreets/goconvey/convey"

"github.com/scionproto/scion/pkg/private/xtest"
"github.com/stretchr/testify/require"
)

func TestSCMPEmptyTable(t *testing.T) {
Expand Down Expand Up @@ -48,7 +47,7 @@ func TestSCMPTableWithOneItem(t *testing.T) {
table := NewSCMPTable()
value := "test value"
err := table.Register(42, value)
xtest.FailOnErr(t, err)
require.NoError(t, err)
Convey("Lookup for the id succeeds", func() {
retValue, ok := table.Lookup(42)
SoMsg("ok", ok, ShouldBeTrue)
Expand Down
1 change: 0 additions & 1 deletion pkg/private/common/BUILD.bazel
Expand Up @@ -3,7 +3,6 @@ load("//tools/lint:go.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
srcs = [
"binary.go",
"defs.go",
"errors.go",
],
Expand Down
36 changes: 0 additions & 36 deletions pkg/private/common/binary.go

This file was deleted.

10 changes: 2 additions & 8 deletions pkg/private/common/defs.go
Expand Up @@ -22,10 +22,8 @@ import (
)

const (
// LineLen is the number of bytes that all SCION headers are padded to a multiple of.
LineLen = 8
MinMTU = 1280
MaxMTU = (1 << 16) - 1
MinMTU = 1280
MaxMTU = (1 << 16) - 1
// SupportedMTU is the MTU supported by dispatcher/snet and router.
// Smaller than MaxMTU to avoid excessive overallocation for packet buffers.
// It's chosen as a common ethernet jumbo frame size minus IP/UDP headers.
Expand All @@ -34,10 +32,6 @@ const (
TimeFmtSecs = "2006-01-02 15:04:05-0700"
)

const (
CPService = "Control Plane Service"
)

// IFIDType is the type for interface IDs.
//
// Deprecated: with version 2 of the SCION header, there is no interface ID type anymore.
Expand Down
78 changes: 0 additions & 78 deletions pkg/private/common/errors.go
Expand Up @@ -15,88 +15,10 @@

package common

import (
"strings"
)

// ErrorMsger allows extracting the message from an error. This means a caller
// can determine the type of error by comparing the returned message with a
// const error string. E.g.:
//
// if GetErrorMsg(err) == addr.ErrorBadHostAddrType {
// // Handle bad host addr error
// }
type ErrorMsger interface {
error
GetMsg() string
}

// ErrorNester allows recursing into nested errors.
type ErrorNester interface {
error
TopError() string // should not include the nested error
GetErr() error
}

// GetNestedError returns the nested error, if any. Returns nil otherwise.
func GetNestedError(e error) error {
if n, _ := e.(ErrorNester); n != nil {
return n.GetErr()
}
return nil
}

// ErrMsg should be used for error string constants. The constant can then be
// used for Is checking in the calling code.
type ErrMsg string

func (e ErrMsg) Error() string {
return string(e)
}

// FmtError formats e for logging. It walks through all nested errors, putting each on a new line,
// and indenting multi-line errors.
func FmtError(e error) string {
var s, ns []string
for {
ns, e = innerFmtError(e)
s = append(s, ns...)
if e == nil {
break
}
}
return strings.Join(s, "\n ")
}

func innerFmtError(e error) ([]string, error) {
var s []string
var lines []string
switch e := e.(type) {
case ErrorNester:
lines = strings.Split(e.TopError(), "\n")
default:
lines = strings.Split(e.Error(), "\n")
}
for i, line := range lines {
if i == len(lines)-1 && len(line) == 0 {
// Don't output an empty line if caused by a trailing newline in
// the input.
break
}
if i == 0 {
s = append(s, line)
} else {
s = append(s, "> "+line)
}
}
return s, GetNestedError(e)
}

// FmtErrors formats a slice of errors for logging.
func FmtErrors(es []error) string {
s := make([]string, 0, len(es))
for _, e := range es {
s = append(s, e.Error())
}
return strings.Join(s, "\n")
}
22 changes: 11 additions & 11 deletions pkg/private/serrors/errors_test.go
Expand Up @@ -98,8 +98,8 @@ func TestWithCtx(t *testing.T) {
t.Run("Is", func(t *testing.T) {
err := serrors.New("simple err")
errWithCtx := serrors.WithCtx(err, "someCtx", "someValue")
xtest.AssertErrorsIs(t, errWithCtx, err)
xtest.AssertErrorsIs(t, errWithCtx, errWithCtx)
assert.ErrorIs(t, errWithCtx, err)
assert.ErrorIs(t, errWithCtx, errWithCtx)
})
t.Run("As", func(t *testing.T) {
err := &testErrType{msg: "test err"}
Expand All @@ -115,9 +115,9 @@ func TestWrap(t *testing.T) {
err := serrors.New("simple err")
msg := serrors.New("msg err")
wrappedErr := serrors.Wrap(msg, err, "someCtx", "someValue")
xtest.AssertErrorsIs(t, wrappedErr, err)
xtest.AssertErrorsIs(t, wrappedErr, msg)
xtest.AssertErrorsIs(t, wrappedErr, wrappedErr)
assert.ErrorIs(t, wrappedErr, err)
assert.ErrorIs(t, wrappedErr, msg)
assert.ErrorIs(t, wrappedErr, wrappedErr)
})
t.Run("As", func(t *testing.T) {
err := &testErrType{msg: "test err"}
Expand All @@ -135,8 +135,8 @@ func TestWrapStr(t *testing.T) {
err := serrors.New("simple err")
msg := "msg"
wrappedErr := serrors.WrapStr(msg, err, "someCtx", "someValue")
xtest.AssertErrorsIs(t, wrappedErr, err)
xtest.AssertErrorsIs(t, wrappedErr, wrappedErr)
assert.ErrorIs(t, wrappedErr, err)
assert.ErrorIs(t, wrappedErr, wrappedErr)
})
t.Run("As", func(t *testing.T) {
err := &testErrType{msg: "test err"}
Expand All @@ -153,14 +153,14 @@ func TestNew(t *testing.T) {
t.Run("Is", func(t *testing.T) {
err1 := serrors.New("err msg")
err2 := serrors.New("err msg")
xtest.AssertErrorsIs(t, err1, err1)
xtest.AssertErrorsIs(t, err2, err2)
assert.ErrorIs(t, err1, err1)
assert.ErrorIs(t, err2, err2)
assert.False(t, errors.Is(err1, err2))
assert.False(t, errors.Is(err2, err1))
err1 = serrors.New("err msg", "someCtx", "value")
err2 = serrors.New("err msg", "someCtx", "value")
xtest.AssertErrorsIs(t, err1, err1)
xtest.AssertErrorsIs(t, err2, err2)
assert.ErrorIs(t, err1, err1)
assert.ErrorIs(t, err2, err2)
assert.False(t, errors.Is(err1, err2))
assert.False(t, errors.Is(err2, err1))
})
Expand Down
19 changes: 0 additions & 19 deletions pkg/private/util/BUILD.bazel
Expand Up @@ -3,50 +3,31 @@ load("//tools/lint:go.bzl", "go_library", "go_test")
go_library(
name = "go_default_library",
srcs = [
"aslist.go",
"checksum.go",
"docker.go",
"duration.go",
"duration_wrap.go",
"file.go",
"file_mode.go",
"fs.go",
"map.go",
"padding.go",
"raw.go",
"sync.go",
"time.go",
"yaml.go",
],
importpath = "github.com/scionproto/scion/pkg/private/util",
visibility = ["//visibility:public"],
deps = [
"//pkg/addr:go_default_library",
"//pkg/private/common:go_default_library",
"//pkg/private/serrors:go_default_library",
"@in_gopkg_yaml_v2//:go_default_library",
],
)

go_test(
name = "go_default_test",
srcs = [
"aslist_test.go",
"checksum_test.go",
"duration_test.go",
"export_test.go",
"map_test.go",
"padding_test.go",
"time_test.go",
"yaml_test.go",
],
data = glob(["testdata/**"]),
embed = [":go_default_library"],
deps = [
"//pkg/addr:go_default_library",
"//pkg/private/xtest:go_default_library",
"@com_github_stretchr_testify//assert:go_default_library",
"@com_github_stretchr_testify//require:go_default_library",
"@in_gopkg_yaml_v2//:go_default_library",
],
)
59 changes: 0 additions & 59 deletions pkg/private/util/checksum.go

This file was deleted.

0 comments on commit 756c8b4

Please sign in to comment.