Skip to content

Commit

Permalink
test with go 1.22 and 1.23
Browse files Browse the repository at this point in the history
Signed-off-by: Josh Humphries <[email protected]>
  • Loading branch information
jhump committed Nov 11, 2024
1 parent 010ac0a commit 8268e33
Show file tree
Hide file tree
Showing 26 changed files with 111 additions and 127 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
matrix:
# When changing this, don't forget to also update
# the version of Go used in the release.yaml workflow.
go-version: [1.21.x, 1.22.x]
go-version: [1.22.x, 1.23.x]
steps:
- name: Checkout Code
uses: actions/checkout@v4
Expand Down Expand Up @@ -46,10 +46,10 @@ jobs:
# conflicting guidance, run only on the most recent supported version.
# For the same reason, only check generated code on the most recent
# supported version.
if: matrix.go-version == '1.22.x'
if: matrix.go-version == '1.23.x'
run: make checkgenerate && make lint
- name: Check Release
# We'll only be building releases w/ latest Go, so we only need to
# test that it can be built w/ latest.
if: matrix.go-version == '1.22.x'
if: matrix.go-version == '1.23.x'
run: make checkrelease
2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: 1.22.x
go-version: 1.23.x
- name: Release
env:
GITHUB_TOKEN: ${{ github.TOKEN }}
Expand Down
23 changes: 9 additions & 14 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
run:
skip-dirs-use-default: false
linters-settings:
errcheck:
check-type-assertions: true
Expand Down Expand Up @@ -30,35 +28,32 @@ linters:
disable:
- cyclop # covered by gocyclo
- depguard # unnecessary for small libraries
- deadcode # abandoned
- exhaustivestruct # replaced by exhaustruct
- err113 # don't _always_ need to wrap errors
- exhaustive # lots of false positives when using default cases
- exportloopref # deprecated in golangci-lint v1.60.2
- funlen # rely on code review to limit function length
- gocognit # dubious "cognitive overhead" quantification
- gofumpt # prefer standard gofmt
- goimports # rely on gci instead
- golint # deprecated by Go team
- gomnd # some unnamed constants are okay
- ifshort # deprecated by author
- interfacer # deprecated by author
- inamedparam # parameter type is often all that is needed for readability
- ireturn # "accept interfaces, return structs" isn't ironclad
- lll # don't want hard limits for line length
- maintidx # covered by gocyclo
- maligned # readability trumps efficient struct packing
- mnd # some unnamed constants are okay
- nlreturn # generous whitespace violates house style
- nonamedreturns # named returns are fine; it's *bare* returns that are bad
- nosnakecase # deprecated in https://github.com/golangci/golangci-lint/pull/3065
- scopelint # deprecated by author
- structcheck # abandoned
- protogetter # too many false positives
- testpackage # internal tests are fine
- varcheck # abandoned
- wrapcheck # don't _always_ need to wrap errors
- wsl # generous whitespace violates house style
issues:
exclude-dirs-use-default: false
exclude:
# Don't ban use of fmt.Errorf to create new errors, but the remaining
# checks from err113 are useful.
- "err113: do not define dynamic errors.*"
- "do not define dynamic errors.*"
# LOTS of false positives for this gosec check
- "integer overflow conversion"
exclude-rules:
- path: cmd/.*/main.go
linters:
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,11 @@ $(BIN)/buf: Makefile

$(BIN)/license-header: Makefile
@mkdir -p $(@D)
$(GO) install github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@v1.26.1
$(GO) install github.com/bufbuild/buf/private/pkg/licenseheader/cmd/license-header@v1.36.0

$(BIN)/golangci-lint: Makefile
@mkdir -p $(@D)
$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.54.2
$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.62.0

$(BIN)/goreleaser: Makefile
@mkdir -p $(@D)
Expand Down
5 changes: 2 additions & 3 deletions internal/app/connectconformance/client_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ func TestRunClient(t *testing.T) {
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
start := runInProcess([]string{"testclient"}, testCase.clientFunc)
Expand All @@ -122,9 +121,9 @@ func TestRunClient(t *testing.T) {

err = runner.waitForResponses()
if testCase.expectErr != "" {
assert.ErrorContains(t, err, testCase.expectErr)
require.ErrorContains(t, err, testCase.expectErr)
} else {
assert.NoError(t, err)
require.NoError(t, err)
}

assert.Equal(t, testCase.failToSend, actualFailedToSend)
Expand Down
2 changes: 0 additions & 2 deletions internal/app/connectconformance/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,6 @@ func TestParseConfig_ComputesPermutations(t *testing.T) {
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
cases, err := parseConfig("config.yaml", []byte(testCase.config))
Expand Down Expand Up @@ -577,7 +576,6 @@ func TestParseConfig_RejectsInvalidConfigurations(t *testing.T) {
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
_, err := parseConfig("config.yaml", []byte(testCase.config))
Expand Down
2 changes: 1 addition & 1 deletion internal/app/connectconformance/connectconformance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestRun(t *testing.T) {

require.NoError(t, err)
require.True(t, results.report(logger))
require.Equal(t, expectedNumCases, len(results.outcomes))
require.Len(t, results.outcomes, expectedNumCases)
}

type testPrinter struct {
Expand Down
2 changes: 1 addition & 1 deletion internal/app/connectconformance/results.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ func checkError(expected, actual *conformancev1.Error, otherCodes []conformancev
}
actualReqInfo := &conformancev1.ConformancePayload_RequestInfo{}
expectedReqInfo := &conformancev1.ConformancePayload_RequestInfo{}
for i := 0; i < length; i++ {
for i := range length {
// TODO: Should this be more lenient? Are we okay with details getting re-ordered?
// An alternative might be to create a map keyed by type, and for each type
// remove expected messages as they are matched against actual ones.
Expand Down
63 changes: 35 additions & 28 deletions internal/app/connectconformance/results_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,16 @@ func TestResults_SetOutcome(t *testing.T) {
require.False(t, success)
lines := errorMessages(logger.Messages)
require.Len(t, lines, 7)
require.Equal(t, lines[0], "FAILED: foo/bar/2:\n\tfail\n")
require.Equal(t, lines[1], "FAILED: foo/bar/3:\n\tfail\n")
require.Equal(t, lines[2], "FAILED: known-to-fail/1 was expected to fail but did not\n")
require.Equal(t, lines[3], "FAILED: known-to-fail/2:\n\tfail\n")
require.Equal(t, lines[4], "INFO: known-to-fail/3 failed (as expected):\n\tfail\n")
// since known-to-flake/1 is flaky, it is allowed to pass
require.Equal(t, lines[5], "FAILED: known-to-flake/2:\n\tflake\n")
require.Equal(t, lines[6], "INFO: known-to-flake/3 failed (as expected):\n\tflake\n")
require.Equal(t, []string{
"FAILED: foo/bar/2:\n\tfail\n",
"FAILED: foo/bar/3:\n\tfail\n",
"FAILED: known-to-fail/1 was expected to fail but did not\n",
"FAILED: known-to-fail/2:\n\tfail\n",
"INFO: known-to-fail/3 failed (as expected):\n\tfail\n",
// since known-to-flake/1 is marked flaky, it is allowed to pass
"FAILED: known-to-flake/2:\n\tflake\n",
"INFO: known-to-flake/3 failed (as expected):\n\tflake\n",
}, lines)
}

func TestResults_FailedToStart(t *testing.T) {
Expand All @@ -69,9 +71,11 @@ func TestResults_FailedToStart(t *testing.T) {
require.False(t, success)
lines := errorMessages(logger.Messages)
require.Len(t, lines, 2)
require.Equal(t, lines[0], "FAILED: foo/bar/1:\n\tfail\n")
// Marked as failure even though expected to fail because it failed to start.
require.Equal(t, lines[1], "FAILED: known-to-fail/1:\n\tfail\n")
require.Equal(t, []string{
"FAILED: foo/bar/1:\n\tfail\n",
// Marked as failure even though expected to fail because it failed to start.
"FAILED: known-to-fail/1:\n\tfail\n",
}, lines)
}

func TestResults_FailRemaining(t *testing.T) {
Expand All @@ -91,12 +95,14 @@ func TestResults_FailRemaining(t *testing.T) {
require.False(t, success)
lines := errorMessages(logger.Messages)
require.Len(t, lines, 3)
require.Equal(t, lines[0], "FAILED: foo/bar/2:\n\tsomething went wrong\n")
require.Equal(t, lines[1], "INFO: known-to-fail/1 failed (as expected):\n\tfail\n")
// Marked as failure even though expected to fail because failRemaining is
// used when a process under test dies (so this error is not due to lack of
// conformance).
require.Equal(t, lines[2], "FAILED: known-to-fail/2:\n\tsomething went wrong\n")
require.Equal(t, []string{
"FAILED: foo/bar/2:\n\tsomething went wrong\n",
"INFO: known-to-fail/1 failed (as expected):\n\tfail\n",
// Marked as failure even though expected to fail because failRemaining is
// used when a process under test dies (so this error is not due to lack of
// conformance).
"FAILED: known-to-fail/2:\n\tsomething went wrong\n",
}, lines)
}

func TestResults_Failed(t *testing.T) {
Expand All @@ -110,8 +116,10 @@ func TestResults_Failed(t *testing.T) {
require.False(t, success)
lines := errorMessages(logger.Messages)
require.Len(t, lines, 2)
require.Equal(t, lines[0], "FAILED: foo/bar/1:\n\tfail\n")
require.Equal(t, lines[1], "INFO: known-to-fail/1 failed (as expected):\n\tfail\n")
require.Equal(t, []string{
"FAILED: foo/bar/1:\n\tfail\n",
"INFO: known-to-fail/1 failed (as expected):\n\tfail\n",
}, lines)
}

func TestResults_Assert(t *testing.T) {
Expand Down Expand Up @@ -154,8 +162,8 @@ func TestResults_Assert(t *testing.T) {
require.Contains(t, lines[1], "FAILED: foo/bar/2:\n\t")
require.Contains(t, lines[2], "INFO: known-to-fail/1 failed (as expected):\n\t")
require.Contains(t, lines[3], "INFO: known-to-fail/2 failed (as expected):\n\t")
require.Equal(t, lines[4], "FAILED: known-to-fail/3 was expected to fail but did not\n")
require.Equal(t, lines[5], "FAILED: known-to-fail/4 was expected to fail but did not\n")
require.Equal(t, "FAILED: known-to-fail/3 was expected to fail but did not\n", lines[4])
require.Equal(t, "FAILED: known-to-fail/4 was expected to fail but did not\n", lines[5])
}

func TestResults_Assert_ReportsAllErrors(t *testing.T) {
Expand Down Expand Up @@ -675,7 +683,6 @@ func TestResults_Assert_ReportsAllErrors(t *testing.T) {
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
results := newResults(0, &testTrie{}, &testTrie{}, nil)
Expand Down Expand Up @@ -730,10 +737,12 @@ func TestResults_ServerSideband(t *testing.T) {
require.False(t, success)
lines := errorMessages(logger.Messages)
require.Len(t, lines, 4)
require.Equal(t, lines[0], "FAILED: foo/bar/2:\n\tsomething awkward in wire format; fail\n")
require.Equal(t, lines[1], "FAILED: foo/bar/3:\n\tsomething awkward in wire format\n")
require.Equal(t, lines[2], "INFO: known-to-fail/1 failed (as expected):\n\tsomething awkward in wire format\n")
require.Equal(t, lines[3], "INFO: known-to-fail/2 failed (as expected):\n\tfail\n")
require.Equal(t, []string{
"FAILED: foo/bar/2:\n\tsomething awkward in wire format; fail\n",
"FAILED: foo/bar/3:\n\tsomething awkward in wire format\n",
"INFO: known-to-fail/1 failed (as expected):\n\tsomething awkward in wire format\n",
"INFO: known-to-fail/2 failed (as expected):\n\tfail\n",
}, lines)
}

func TestResults_Report(t *testing.T) {
Expand Down Expand Up @@ -834,7 +843,6 @@ func TestCanonicalizeHeaderVals(t *testing.T) {
},
}
for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
result := canonicalizeHeaderVals(testCase.input)
Expand Down Expand Up @@ -873,7 +881,6 @@ func TestExpectedCodeString(t *testing.T) {
},
}
for _, testCase := range testCases {
testCase := testCase
t.Run(fmt.Sprintf("%d_other_codes", len(testCase.otherCodes)), func(t *testing.T) {
t.Parallel()
require.Equal(t, testCase.expectedString, expectedCodeString(testCase.expectedCode, testCase.otherCodes))
Expand Down
5 changes: 2 additions & 3 deletions internal/app/connectconformance/server_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ func TestRunTestCasesForServer(t *testing.T) {
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
results := newResults(len(requests), &testTrie{}, &testTrie{}, nil)
Expand Down Expand Up @@ -312,7 +311,7 @@ type fakeProcess struct {
}

func newFakeProcess(stdin io.Writer, stdout, stderr io.Reader) processStarter {
return func(ctx context.Context, pipeStderr bool) (*process, error) {
return func(_ context.Context, _ bool) (*process, error) {
proc := &fakeProcess{}
return &process{
processController: proc,
Expand All @@ -326,7 +325,7 @@ func newFakeProcess(stdin io.Writer, stdout, stderr io.Reader) processStarter {
}

func newStillbornProcess(stdin io.Writer, stdout, stderr io.Reader) processStarter {
return func(ctx context.Context, pipeStderr bool) (*process, error) {
return func(_ context.Context, _ bool) (*process, error) {
proc := &fakeProcess{}
stdout = &hookReader{
r: stdout,
Expand Down
6 changes: 1 addition & 5 deletions internal/app/connectconformance/test_case_library_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,6 @@ func TestNewTestCaseLibrary(t *testing.T) {
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
testCaseLib, err := newTestCaseLibrary(testSuites, testCase.config, testCase.mode)
Expand Down Expand Up @@ -437,7 +436,7 @@ func TestParseTestSuites_EmbeddedTestSuites(t *testing.T) {
assert.True(t, strings.HasPrefix(testSuiteName, "gRPC") || unicode.IsUpper(rune(testSuiteName[0])),
"test suite name %q should start with capital letter", testSuiteName)
}
assert.True(t, testSuiteName == testSuite.Name,
assert.Equal(t, testSuite.Name, testSuiteName,
"test suite name %q should not have leading or trailing spaces", testSuiteName)
assert.False(t, strings.ContainsFunc(testSuiteName, func(r rune) bool {
const allowed = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789- "
Expand Down Expand Up @@ -605,7 +604,6 @@ func TestFilter(t *testing.T) {
},
}
for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
candidates := make([]*conformancev1.TestCase, len(allTestCaseNames))
Expand Down Expand Up @@ -837,7 +835,6 @@ func TestExpandRequestData(t *testing.T) {
},
}
for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
var testCaseProto conformancev1.TestCase
Expand Down Expand Up @@ -1888,7 +1885,6 @@ func TestPopulateExpectedResponse(t *testing.T) {
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.testName, func(t *testing.T) {
t.Parallel()

Expand Down
1 change: 0 additions & 1 deletion internal/app/connectconformance/test_trie_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ func TestParsePatterns(t *testing.T) {
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.testName, func(t *testing.T) {
t.Parallel()
matched := trie.match(strings.Split(testCase.testName, "/"))
Expand Down
2 changes: 1 addition & 1 deletion internal/app/grpcclient/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ func (i *invoker) unimplemented(
func(ctx context.Context, req *conformancev1.UnimplementedRequest, opts ...grpc.CallOption) (*conformancev1.UnimplementedResponse, error) {
return i.client.Unimplemented(ctx, req, opts...)
},
func(resp *conformancev1.UnimplementedResponse) *conformancev1.ConformancePayload {
func(_ *conformancev1.UnimplementedResponse) *conformancev1.ConformancePayload {
return nil
},
)
Expand Down
2 changes: 1 addition & 1 deletion internal/app/referenceclient/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ func (i *invoker) unimplemented(
req *conformancev1.ClientCompatRequest,
) (*conformancev1.ClientResponseResult, error) {
return doUnary(ctx, req, i, i.client.Unimplemented,
func(resp *conformancev1.UnimplementedResponse) *conformancev1.ConformancePayload {
func(_ *conformancev1.UnimplementedResponse) *conformancev1.ConformancePayload {
return nil
})
}
Expand Down
Loading

0 comments on commit 8268e33

Please sign in to comment.