Remove goconvey from go/lib/hpkt

[karampok]

This change is 

[oncilla]

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @karampok)

---

go/lib/hpkt/extension_test.go, line 32 at r1 (raw file):

		ExpectedOutputHBH []common.Extension
		ExpectedOutputE2E []common.Extension
		ExpectedError     bool

use require.ErrorAssertionFunc instead

i.e.

...
    ExpectedOutputE2E []common.Extension
    Assertion require.ErrorAssertionFunc
}

and replace ExpectedError: true with require.Error and ExpectedError: false with require.NoError

see how it is used in comment at t.Run

---

go/lib/hpkt/extension_test.go, line 34 at r1 (raw file):

		ExpectedError     bool
	}
	testCases := []*TestCase{

make this a map and use the description as key.
Because of go's non-deterministic map range, we can avoid having execution order dependencies in tests.
(This probably does not mattere here, but in other places where we test some stateful objects it is really useful)

tests := map[string]struct{
		InputExtensions   []common.Extension
		ExpectedOutputHBH []common.Extension
		ExpectedOutputE2E []common.Extension
		ExpectedError     bool
}{
...
}

---

go/lib/hpkt/extension_test.go, line 140 at r1 (raw file):

		t.Run(tc.Description, func(t *testing.T) {
			hbh, e2e, err := ValidateExtensions(tc.InputExtensions)
			if tc.ExpectedError == true {

replace if/else with tc.Assertion(t, err)

---

go/lib/hpkt/hpkt_test.go, line 52 at r1 (raw file):

	s := &spkt.ScnPkt{
		DstIA: addr.IA{},

remove those two lines. They initialize the zero value?

---

go/lib/hpkt/hpkt_test.go, line 59 at r1 (raw file):

	assert.Equal(t, s.DstIA.I, addr.ISD(2), "AddrHdr.DstIA.I")
	assert.Equal(t, s.DstIA.A, addr.AS(0xff0000000222), "AddrHdr.DstIA.A")

do xtest.MustParseAS(ff00:0:222)

---

go/lib/hpkt/hpkt_test.go, line 61 at r1 (raw file):

	assert.Equal(t, s.DstIA.A, addr.AS(0xff0000000222), "AddrHdr.DstIA.A")
	assert.Equal(t, s.SrcIA.I, addr.ISD(1), "AddrHdr.SrcIA.I")
	assert.Equal(t, s.SrcIA.A, addr.AS(0xff0000000133), "AddrHdr.SrcIA.A")

ditto

---

go/lib/hpkt/hpkt_test.go, line 94 at r1 (raw file):

	scmpHdr, ok := s.L4.(*scmp.Hdr)
	require.Equal(t, ok, true, "Bad L4Hdr, cannot continue")

require.True

---

go/lib/hpkt/hpkt_test.go, line 98 at r1 (raw file):

	assert.Equal(t, scmpHdr.Class, scmp.C_Path, "SCMP.Class")
	assert.Equal(t, scmpHdr.Type, scmp.T_P_RevokedIF, "SCMP.Type")
	//// XXX(kormat): not maintainable:

remove the two added //

[lukedirtwalker]

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @karampok)

---

go/lib/hpkt/hpkt_test.go, line 39 at r1 (raw file):

)

func mustLoad(path string) common.RawBytes {

you could use xtest.MustReadFromFile(t, path) instead of mustLoad. Note that xtest.MustReadFromFile prepends the testdata folder to the given path.

[karampok]

---

go/lib/hpkt/hpkt_test.go, line 98 at r1 (raw file):

Previously, Oncilla wrote…

remove the two added //

done

[karampok]

Reviewable status: 1 of 3 files reviewed, 9 unresolved discussions (waiting on @lukedirtwalker and @oncilla)

---

go/lib/hpkt/extension_test.go, line 32 at r1 (raw file):

Previously, Oncilla wrote…

use require.ErrorAssertionFunc instead

i.e.

...
    ExpectedOutputE2E []common.Extension
    Assertion require.ErrorAssertionFunc
}

and replace ExpectedError: true with require.Error and ExpectedError: false with require.NoError

see how it is used in comment at t.Run

Done.

---

go/lib/hpkt/extension_test.go, line 34 at r1 (raw file):

Previously, Oncilla wrote…

make this a map and use the description as key.
Because of go's non-deterministic map range, we can avoid having execution order dependencies in tests.
(This probably does not mattere here, but in other places where we test some stateful objects it is really useful)

tests := map[string]struct{
		InputExtensions   []common.Extension
		ExpectedOutputHBH []common.Extension
		ExpectedOutputE2E []common.Extension
		ExpectedError     bool
}{
...
}

Done.

---

go/lib/hpkt/extension_test.go, line 140 at r1 (raw file):

Previously, Oncilla wrote…

replace if/else with tc.Assertion(t, err)

Done.

---

go/lib/hpkt/hpkt_test.go, line 39 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

you could use xtest.MustReadFromFile(t, path) instead of mustLoad. Note that xtest.MustReadFromFile prepends the testdata folder to the given path.

Done.

---

go/lib/hpkt/hpkt_test.go, line 52 at r1 (raw file):

Previously, Oncilla wrote…

remove those two lines. They initialize the zero value?

Done.

---

go/lib/hpkt/hpkt_test.go, line 59 at r1 (raw file):

Previously, Oncilla wrote…

do xtest.MustParseAS(ff00:0:222)

Done.

---

go/lib/hpkt/hpkt_test.go, line 61 at r1 (raw file):

Previously, Oncilla wrote…

ditto

Done.

---

go/lib/hpkt/hpkt_test.go, line 94 at r1 (raw file):

Previously, Oncilla wrote…

require.True

Done.

[oncilla]

Reviewed 3 of 3 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @karampok and @lukedirtwalker)

---

go/lib/hpkt/extension_test.go, line 130 at r2 (raw file):

	}

	for dscr, input := range tests {

we generally use name and test here.

---

go/lib/hpkt/hpkt_test.go, line 39 at r2 (raw file):

func TestParseScnPkt(t *testing.T) {
	//raw := common.RawBytes(xtest.MustReadFromFile(t, rawUDPPktFilename))

remove?

[karampok]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @lukedirtwalker and @oncilla)

---

go/lib/hpkt/extension_test.go, line 130 at r2 (raw file):

Previously, Oncilla wrote…

we generally use name and test here.

Done.

---

go/lib/hpkt/hpkt_test.go, line 39 at r2 (raw file):

Previously, Oncilla wrote…

remove?

yes sure

[oncilla]

Reviewed 2 of 2 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)

[oncilla]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)

[lukedirtwalker]

Reviewable status: complete! all files reviewed, all discussions resolved