godispatcher: remove convey

[karampok]

Contributes #3016

---

This change is 

[scrye]

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

---

go/godispatcher/internal/registration/iatable_test.go, line 35 at r1 (raw file):

func TestIATable(t *testing.T) {
	table := NewIATable(minPort, maxPort)

Move under each "parent" t.Run below, otherwise the tests interfere with one another.

In Goconvey this is not the case, because the top-level function runs again for each child Convey block, thus making "if the entry is only public" operate on a fresh table, and "if the entry is public and svc" on a different fresh table.

---

go/godispatcher/internal/registration/iatable_test.go, line 129 at r1 (raw file):

func TestIATableSCMPExistingRegistration(t *testing.T) {
	table := NewIATable(minPort, maxPort)

Same as above.

---

go/godispatcher/internal/registration/table_test.go, line 29 at r1 (raw file):

var dummyValue = "test value"

func TestRegister(t *testing.T) {

All the tests here share the same table, which would make investigation awkward if one of them breaks (because they are no longer independent).

I think a table driven test fits here instead, with each loop iteration creating a new table.

---

go/godispatcher/internal/registration/table_test.go, line 124 at r1 (raw file):

func TestRegisterOnlyPublic(t *testing.T) {
	table := NewTable(minPort, maxPort)

Same as in some previous comments, this should be local to the subtests to make them independent.

---

go/godispatcher/internal/registration/table_test.go, line 199 at r1 (raw file):

}

func TestRegisterWithBind(t *testing.T) {

You can delete this. Binds are never used and going away soon. It should probably be refactored into a table driven test, but it's not worth the effort at this point.

[karampok]

Reviewable status: 0 of 2 files reviewed, 5 unresolved discussions (waiting on @scrye)

---

go/godispatcher/internal/registration/iatable_test.go, line 35 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Move under each "parent" t.Run below, otherwise the tests interfere with one another.

In Goconvey this is not the case, because the top-level function runs again for each child Convey block, thus making "if the entry is only public" operate on a fresh table, and "if the entry is public and svc" on a different fresh table.

Done.

---

go/godispatcher/internal/registration/iatable_test.go, line 129 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Same as above.

Done.k

---

go/godispatcher/internal/registration/table_test.go, line 29 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

All the tests here share the same table, which would make investigation awkward if one of them breaks (because they are no longer independent).

I think a table driven test fits here instead, with each loop iteration creating a new table.

Done.

---

go/godispatcher/internal/registration/table_test.go, line 124 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Same as in some previous comments, this should be local to the subtests to make them independent.

Done.

---

go/godispatcher/internal/registration/table_test.go, line 199 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

You can delete this. Binds are never used and going away soon. It should probably be refactored into a table driven test, but it's not worth the effort at this point.

Is there an issue to remove everything around Binds?
I would suggest we let it until that story is implemented.

[scrye]

---

go/godispatcher/internal/registration/table_test.go, line 199 at r1 (raw file):

Previously, karampok (Konstantinos) wrote…

Is there an issue to remove everything around Binds?
I would suggest we let it until that story is implemented.

Fair enough, this can wait.

[scrye]

Reviewed 2 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved