Repository: quic-go/quic-go
Pull request #4876 (2025-01-15 12:03:55)

http3: migrate the error tests away from Ginkgo
Part of #3652.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4876?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 83.22%. Comparing base [(`a2dccf5`)](https://app.codecov.io/gh/quic-go/quic-go/commit/a2dccf54cacad39e509fa15a42b5b22597f13a44?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`7988951`)](https://app.codecov.io/gh/quic-go/quic-go/commit/798895176fe859b7127d7e8db5c83c6aec2e7893?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4876      +/-   ##
==========================================
- Coverage   83.34%   83.22%   -0.12%     
==========================================
  Files         151      148       -3     
  Lines       16207    15862     -345     
==========================================
- Hits        13507    13200     -307     
+ Misses       2163     2140      -23     
+ Partials      537      522      -15     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4876?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).


Patch:
diff --git a/conn_id_manager.go b/conn_id_manager.go
index 4aa3f749..0e65231c 100644
--- a/conn_id_manager.go
+++ b/conn_id_manager.go
@@ -66,6 +66,12 @@ func (h *connIDManager) Add(f *wire.NewConnectionIDFrame) error {
 }
 
 func (h *connIDManager) add(f *wire.NewConnectionIDFrame) error {
+	if h.activeConnectionID.Len() == 0 {
+		return &qerr.TransportError{
+			ErrorCode:    qerr.ProtocolViolation,
+			ErrorMessage: "received NEW_CONNECTION_ID frame but zero-length connection IDs are in use",
+		}
+	}
 	// If the NEW_CONNECTION_ID frame is reordered, such that its sequence number is smaller than the currently active
 	// connection ID or if it was already retired, send the RETIRE_CONNECTION_ID frame immediately.
 	if f.SequenceNumber < h.activeSequenceNumber || f.SequenceNumber < h.highestRetired {


Test patch:
diff --git a/conn_id_manager_test.go b/conn_id_manager_test.go
index ffe435de..193d74c2 100644
--- a/conn_id_manager_test.go
+++ b/conn_id_manager_test.go
@@ -209,6 +209,26 @@ func TestConnIDManagerConnIDRotation(t *testing.T) {
 	require.Equal(t, []wire.Frame{&wire.RetireConnectionIDFrame{SequenceNumber: 2}}, frameQueue)
 }
 
+func TestConnIDManagerZeroLengthConnectionID(t *testing.T) {
+	m := newConnIDManager(
+		protocol.ConnectionID{},
+		func(protocol.StatelessResetToken) {},
+		func(protocol.StatelessResetToken) {},
+		func(f wire.Frame) {},
+	)
+	require.Equal(t, protocol.ConnectionID{}, m.Get())
+	for i := 0; i < 5*protocol.PacketsPerConnectionID; i++ {
+		m.SentPacket()
+		require.Equal(t, protocol.ConnectionID{}, m.Get())
+	}
+
+	require.ErrorIs(t, m.Add(&wire.NewConnectionIDFrame{
+		SequenceNumber:      1,
+		ConnectionID:        protocol.ConnectionID{},
+		StatelessResetToken: protocol.StatelessResetToken{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1},
+	}), &qerr.TransportError{ErrorCode: qerr.ProtocolViolation})
+}
+
 func TestConnIDManagerClose(t *testing.T) {
 	var addedTokens, removedTokens []protocol.StatelessResetToken
 	m := newConnIDManager(
diff --git a/http3/error_codes_test.go b/http3/error_codes_test.go
index a50afc60..af7642b1 100644
--- a/http3/error_codes_test.go
+++ b/http3/error_codes_test.go
@@ -7,33 +7,31 @@ import (
 	"path"
 	"runtime"
 	"strconv"
+	"testing"
 
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
+	"github.com/stretchr/testify/require"
 )
 
-var _ = Describe("error codes", func() {
-	It("has a string representation for every error code", func() {
-		// We parse the error code file, extract all constants, and verify that
-		// each of them has a string version. Go FTW!
-		_, thisfile, _, ok := runtime.Caller(0)
-		if !ok {
-			panic("Failed to get current frame")
-		}
-		filename := path.Join(path.Dir(thisfile), "error_codes.go")
-		fileAst, err := parser.ParseFile(token.NewFileSet(), filename, nil, 0)
-		Expect(err).NotTo(HaveOccurred())
-		constSpecs := fileAst.Decls[2].(*ast.GenDecl).Specs
-		Expect(len(constSpecs)).To(BeNumerically(">", 4)) // at time of writing
-		for _, c := range constSpecs {
-			valString := c.(*ast.ValueSpec).Values[0].(*ast.BasicLit).Value
-			val, err := strconv.ParseInt(valString, 0, 64)
-			Expect(err).NotTo(HaveOccurred())
-			Expect(ErrCode(val).String()).ToNot(Equal("unknown error code"))
-		}
-	})
+func TestErrorCodes(t *testing.T) {
+	// We parse the error code file, extract all constants, and verify that
+	// each of them has a string version. Go FTW!
+	_, thisfile, _, ok := runtime.Caller(0)
+	require.True(t, ok, "Failed to get current frame")
 
-	It("has a string representation for unknown error codes", func() {
-		Expect(ErrCode(0x1337).String()).To(Equal("unknown error code: 0x1337"))
-	})
-})
+	filename := path.Join(path.Dir(thisfile), "error_codes.go")
+	fileAst, err := parser.ParseFile(token.NewFileSet(), filename, nil, 0)
+	require.NoError(t, err)
+
+	constSpecs := fileAst.Decls[2].(*ast.GenDecl).Specs
+	require.Greater(t, len(constSpecs), 4) // at time of writing
+
+	for _, c := range constSpecs {
+		valString := c.(*ast.ValueSpec).Values[0].(*ast.BasicLit).Value
+		val, err := strconv.ParseInt(valString, 0, 64)
+		require.NoError(t, err)
+		require.NotEqual(t, "unknown error code", ErrCode(val).String())
+	}
+
+	// Test unknown error code
+	require.Equal(t, "unknown error code: 0x1337", ErrCode(0x1337).String())
+}
diff --git a/http3/error_test.go b/http3/error_test.go
index 71567128..2ee08739 100644
--- a/http3/error_test.go
+++ b/http3/error_test.go
@@ -2,40 +2,88 @@ package http3
 
 import (
 	"errors"
+	"testing"
 
 	"github.com/quic-go/quic-go"
 
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
+	"github.com/stretchr/testify/require"
 )
 
-var _ = Describe("HTTP/3 errors", func() {
-	It("converts", func() {
-		Expect(maybeReplaceError(nil)).To(BeNil())
-		Expect(maybeReplaceError(errors.New("foobar"))).To(MatchError("foobar"))
-		Expect(maybeReplaceError(&quic.StreamError{
-			ErrorCode: 1337,
-			Remote:    true,
-		})).To(Equal(&Error{
-			Remote:    true,
-			ErrorCode: 1337,
-		}))
-		Expect(maybeReplaceError(&quic.ApplicationError{
-			ErrorCode:    42,
-			Remote:       true,
-			ErrorMessage: "foobar",
-		})).To(Equal(&Error{
-			Remote:       true,
-			ErrorCode:    42,
-			ErrorMessage: "foobar",
-		}))
-	})
-
-	It("has a string representation", func() {
-		Expect((&Error{ErrorCode: 0x10c, Remote: true}).Error()).To(Equal("H3_REQUEST_CANCELLED"))
-		Expect((&Error{ErrorCode: 0x10c, Remote: true, ErrorMessage: "foobar"}).Error()).To(Equal("H3_REQUEST_CANCELLED: foobar"))
-		Expect((&Error{ErrorCode: 0x10c, Remote: false}).Error()).To(Equal("H3_REQUEST_CANCELLED (local)"))
-		Expect((&Error{ErrorCode: 0x10c, Remote: false, ErrorMessage: "foobar"}).Error()).To(Equal("H3_REQUEST_CANCELLED (local): foobar"))
-		Expect((&Error{ErrorCode: 0x1337, Remote: true}).Error()).To(Equal("H3 error (0x1337)"))
-	})
-})
+func TestErrorConversion(t *testing.T) {
+	regularErr := errors.New("foobar")
+
+	tests := []struct {
+		name     string
+		input    error
+		expected error
+	}{
+		{name: "nil error", input: nil, expected: nil},
+		{name: "regular error", input: regularErr, expected: regularErr},
+		{
+			name:     "stream error",
+			input:    &quic.StreamError{ErrorCode: 1337, Remote: true},
+			expected: &Error{Remote: true, ErrorCode: 1337},
+		},
+		{
+			name:     "application error",
+			input:    &quic.ApplicationError{ErrorCode: 42, Remote: true, ErrorMessage: "foobar"},
+			expected: &Error{Remote: true, ErrorCode: 42, ErrorMessage: "foobar"},
+		},
+		{
+			name:     "transport error",
+			input:    &quic.TransportError{ErrorCode: 42, Remote: true, ErrorMessage: "foobar"},
+			expected: &quic.TransportError{ErrorCode: 42, Remote: true, ErrorMessage: "foobar"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := maybeReplaceError(tt.input)
+			if tt.expected == nil {
+				require.Nil(t, result)
+			} else {
+				require.ErrorIs(t, tt.expected, result)
+			}
+		})
+	}
+}
+
+func TestErrorString(t *testing.T) {
+	tests := []struct {
+		name     string
+		err      *Error
+		expected string
+	}{
+		{
+			name:     "remote error",
+			err:      &Error{ErrorCode: 0x10c, Remote: true},
+			expected: "H3_REQUEST_CANCELLED",
+		},
+		{
+			name:     "remote error with message",
+			err:      &Error{ErrorCode: 0x10c, Remote: true, ErrorMessage: "foobar"},
+			expected: "H3_REQUEST_CANCELLED: foobar",
+		},
+		{
+			name:     "local error",
+			err:      &Error{ErrorCode: 0x10c, Remote: false},
+			expected: "H3_REQUEST_CANCELLED (local)",
+		},
+		{
+			name:     "local error with message",
+			err:      &Error{ErrorCode: 0x10c, Remote: false, ErrorMessage: "foobar"},
+			expected: "H3_REQUEST_CANCELLED (local): foobar",
+		},
+		{
+			name:     "unknown error code",
+			err:      &Error{ErrorCode: 0x1337, Remote: true},
+			expected: "H3 error (0x1337)",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.expected, tt.err.Error())
+		})
+	}
+}