Skip to content

Commit cd7a12f

Browse files
[chore] Replace Fatal calls with stretch/assert (#14)
Use a proper assertion framework, refactor all calls throughout the codebase Co-authored-by: sergeyb <sergeyb@uber.com>
1 parent 0beff58 commit cd7a12f

18 files changed

Lines changed: 121 additions & 214 deletions

File tree

MODULE.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ go_deps.from_file(go_mod = "//:go.mod")
3131
use_repo(
3232
go_deps,
3333
"com_github_gogo_protobuf",
34+
"com_github_stretchr_testify",
3435
"com_github_uber_go_tally_v4",
3536
"org_golang_google_grpc",
3637
"org_golang_google_protobuf",

gateway/controller/BUILD.bazel

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,11 @@ go_test(
1616
name = "controller_test",
1717
srcs = ["ping_test.go", "land_test.go"],
1818
embed = [":controller"],
19-
deps = ["//gateway/protopb"],
19+
deps = [
20+
"//gateway/protopb",
21+
"@com_github_stretchr_testify//assert",
22+
"@com_github_stretchr_testify//require",
23+
"@com_github_uber_go_tally_v4//:tally",
24+
"@org_uber_go_zap//:zap",
25+
],
2026
)

gateway/controller/land_test.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,13 @@ import (
44
"context"
55
"testing"
66

7+
"github.com/stretchr/testify/require"
78
pb "github.com/uber/submitqueue/gateway/protopb"
89
)
910

1011
func TestNewLandController(t *testing.T) {
1112
controller := NewLandController(nil, nil)
12-
if controller == nil {
13-
t.Fatal("NewLandController() returned nil")
14-
}
13+
require.NotNil(t, controller)
1514
}
1615

1716
func TestLand_ReturnsSqid(t *testing.T) {
@@ -24,11 +23,6 @@ func TestLand_ReturnsSqid(t *testing.T) {
2423
}
2524
resp, err := controller.Land(ctx, req)
2625

27-
if err != nil {
28-
t.Fatalf("Land() returned error: %v", err)
29-
}
30-
31-
if resp.Sqid == "" {
32-
t.Fatal("Expected sqid to be set, got empty string")
33-
}
26+
require.NoError(t, err)
27+
require.NotEmpty(t, resp.Sqid)
3428
}

gateway/controller/ping_test.go

Lines changed: 14 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ import (
55
"testing"
66
"time"
77

8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
810
pb "github.com/uber/submitqueue/gateway/protopb"
911
)
1012

1113
func TestNewPingController(t *testing.T) {
1214
controller := NewPingController(nil, nil)
13-
if controller == nil {
14-
t.Fatal("NewPingController() returned nil")
15-
}
15+
require.NotNil(t, controller)
1616
}
1717

1818
func TestPing_DefaultMessage(t *testing.T) {
@@ -22,13 +22,8 @@ func TestPing_DefaultMessage(t *testing.T) {
2222
req := &pb.PingRequest{}
2323
resp, err := controller.Ping(ctx, req)
2424

25-
if err != nil {
26-
t.Fatalf("Ping() returned error: %v", err)
27-
}
28-
29-
if resp.Message != "pong" {
30-
t.Errorf("Expected message 'pong', got '%s'", resp.Message)
31-
}
25+
require.NoError(t, err)
26+
assert.Equal(t, "pong", resp.Message)
3227
}
3328

3429
func TestPing_CustomMessage(t *testing.T) {
@@ -50,13 +45,8 @@ func TestPing_CustomMessage(t *testing.T) {
5045
req := &pb.PingRequest{Message: tc.input}
5146
resp, err := controller.Ping(ctx, req)
5247

53-
if err != nil {
54-
t.Fatalf("Ping() returned error: %v", err)
55-
}
56-
57-
if resp.Message != tc.expected {
58-
t.Errorf("Expected message '%s', got '%s'", tc.expected, resp.Message)
59-
}
48+
require.NoError(t, err)
49+
assert.Equal(t, tc.expected, resp.Message)
6050
})
6151
}
6252
}
@@ -68,13 +58,8 @@ func TestPing_ServiceName(t *testing.T) {
6858
req := &pb.PingRequest{}
6959
resp, err := controller.Ping(ctx, req)
7060

71-
if err != nil {
72-
t.Fatalf("Ping() returned error: %v", err)
73-
}
74-
75-
if resp.ServiceName != "gateway" {
76-
t.Errorf("Expected service name 'gateway', got '%s'", resp.ServiceName)
77-
}
61+
require.NoError(t, err)
62+
assert.Equal(t, "gateway", resp.ServiceName)
7863
}
7964

8065
func TestPing_Timestamp(t *testing.T) {
@@ -86,13 +71,9 @@ func TestPing_Timestamp(t *testing.T) {
8671
resp, err := controller.Ping(ctx, req)
8772
after := time.Now().Unix()
8873

89-
if err != nil {
90-
t.Fatalf("Ping() returned error: %v", err)
91-
}
92-
93-
if resp.Timestamp < before || resp.Timestamp > after {
94-
t.Errorf("Timestamp %d is not within expected range [%d, %d]", resp.Timestamp, before, after)
95-
}
74+
require.NoError(t, err)
75+
assert.GreaterOrEqual(t, resp.Timestamp, before)
76+
assert.LessOrEqual(t, resp.Timestamp, after)
9677
}
9778

9879
func TestPing_Hostname(t *testing.T) {
@@ -102,13 +83,6 @@ func TestPing_Hostname(t *testing.T) {
10283
req := &pb.PingRequest{}
10384
resp, err := controller.Ping(ctx, req)
10485

105-
if err != nil {
106-
t.Fatalf("Ping() returned error: %v", err)
107-
}
108-
109-
// Hostname should be set (non-empty string)
110-
// We don't check the exact value as it depends on the environment
111-
if resp.Hostname == "" {
112-
t.Error("Expected hostname to be set, got empty string")
113-
}
86+
require.NoError(t, err)
87+
assert.NotEmpty(t, resp.Hostname)
11488
}

gateway/integration_tests/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ go_test(
1111
],
1212
deps = [
1313
"//gateway/protopb",
14+
"@com_github_stretchr_testify//assert",
15+
"@com_github_stretchr_testify//require",
1416
"@org_golang_google_grpc//:grpc",
1517
"@org_golang_google_grpc//credentials/insecure",
1618
],

gateway/integration_tests/ping_test.go

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"testing"
88
"time"
99

10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
1012
pb "github.com/uber/submitqueue/gateway/protopb"
1113
"google.golang.org/grpc"
1214
"google.golang.org/grpc/credentials/insecure"
@@ -24,9 +26,7 @@ func TestPingAPI(t *testing.T) {
2426

2527
// Wait for server to be ready
2628
conn, err := waitForServer(t, addr, serverReadyTimeout)
27-
if err != nil {
28-
t.Fatalf("Gateway server not ready: %v", err)
29-
}
29+
require.NoError(t, err, "Gateway server not ready")
3030
defer conn.Close()
3131

3232
client := pb.NewSubmitQueueGatewayClient(conn)
@@ -39,23 +39,13 @@ func TestPingAPI(t *testing.T) {
3939
}
4040

4141
resp, err := client.Ping(ctx, req)
42-
if err != nil {
43-
t.Fatalf("Ping failed: %v", err)
44-
}
42+
require.NoError(t, err, "Ping failed")
4543

4644
// Validate response
47-
if resp.Message == "" {
48-
t.Error("Response message is empty")
49-
}
50-
if resp.ServiceName != "gateway" {
51-
t.Errorf("Expected service name 'gateway', got '%s'", resp.ServiceName)
52-
}
53-
if resp.Timestamp == 0 {
54-
t.Error("Timestamp is zero")
55-
}
56-
if resp.Hostname == "" {
57-
t.Error("Hostname is empty")
58-
}
45+
assert.NotEmpty(t, resp.Message, "Response message should not be empty")
46+
assert.Equal(t, "gateway", resp.ServiceName)
47+
assert.NotZero(t, resp.Timestamp, "Timestamp should not be zero")
48+
assert.NotEmpty(t, resp.Hostname, "Hostname should not be empty")
5949

6050
t.Logf("Gateway Ping test passed:")
6151
t.Logf(" Message: %s", resp.Message)

go.mod

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ go 1.24.5
44

55
require (
66
github.com/gogo/protobuf v1.3.2
7+
github.com/stretchr/testify v1.9.0
78
github.com/uber-go/tally/v4 v4.1.17
89
go.uber.org/fx v1.22.0
910
go.uber.org/yarpc v1.81.0
@@ -16,12 +17,14 @@ require (
1617
github.com/BurntSushi/toml v1.2.1 // indirect
1718
github.com/beorn7/perks v1.0.1 // indirect
1819
github.com/cespare/xxhash/v2 v2.3.0 // indirect
20+
github.com/davecgh/go-spew v1.1.1 // indirect
1921
github.com/gogo/googleapis v1.4.1 // indirect
2022
github.com/gogo/status v1.1.0 // indirect
2123
github.com/golang/mock v1.7.0-rc.1 // indirect
2224
github.com/golang/protobuf v1.5.4 // indirect
2325
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
2426
github.com/opentracing/opentracing-go v1.2.0 // indirect
27+
github.com/pmezard/go-difflib v1.0.0 // indirect
2528
github.com/prometheus/client_golang v1.11.1 // indirect
2629
github.com/prometheus/client_model v0.2.0 // indirect
2730
github.com/prometheus/common v0.26.0 // indirect
@@ -43,5 +46,6 @@ require (
4346
golang.org/x/text v0.23.0 // indirect
4447
golang.org/x/tools v0.22.0 // indirect
4548
google.golang.org/genproto/googleapis/rpc v0.0.0-20241230172942-26aa7a208def // indirect
49+
gopkg.in/yaml.v3 v3.0.1 // indirect
4650
honnef.co/go/tools v0.4.3 // indirect
4751
)

go.sum

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFB
8282
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
8383
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
8484
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
85+
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
86+
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
8587
github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
8688
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 h1:I0XW9+e1XWDxdcEniV4rQAIOPUGDq67JSCiRCgGCZLI=
8789
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=
@@ -91,6 +93,8 @@ github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lN
9193
github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
9294
github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
9395
github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
96+
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
97+
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
9498
github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs=
9599
github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
96100
github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
@@ -283,6 +287,8 @@ gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLks
283287
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
284288
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
285289
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
290+
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f h1:BLraFXnmrev5lT+xlilqcH8XK9/i0At2xKjWk4p6zsU=
291+
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
286292
gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
287293
gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
288294
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=

integration_tests/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ go_test(
1616
"//gateway/protopb",
1717
"//orchestrator/protopb",
1818
"//speculator/protopb",
19+
"@com_github_stretchr_testify//assert",
20+
"@com_github_stretchr_testify//require",
1921
"@org_golang_google_grpc//:grpc",
2022
"@org_golang_google_grpc//credentials/insecure",
2123
],

integration_tests/ping_test.go

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"testing"
88
"time"
99

10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
1012
gatewaypb "github.com/uber/submitqueue/gateway/protopb"
1113
orchestratorpb "github.com/uber/submitqueue/orchestrator/protopb"
1214
speculatorpb "github.com/uber/submitqueue/speculator/protopb"
@@ -27,68 +29,50 @@ func TestPingForAllServices(t *testing.T) {
2729
t.Run("Gateway", func(t *testing.T) {
2830
addr := getEnvOrDefault("GATEWAY_ADDR", "localhost:8081")
2931
conn, err := waitForServer(t, addr, serverReadyTimeout)
30-
if err != nil {
31-
t.Fatalf("Gateway server not ready: %v", err)
32-
}
32+
require.NoError(t, err, "Gateway server not ready")
3333
defer conn.Close()
3434

3535
client := gatewaypb.NewSubmitQueueGatewayClient(conn)
3636
ctx, cancel := context.WithTimeout(context.Background(), defaultTimeout)
3737
defer cancel()
3838

3939
resp, err := client.Ping(ctx, &gatewaypb.PingRequest{Message: "e2e test"})
40-
if err != nil {
41-
t.Fatalf("Gateway Ping failed: %v", err)
42-
}
43-
if resp.ServiceName != "gateway" {
44-
t.Errorf("Expected service name 'gateway', got '%s'", resp.ServiceName)
45-
}
40+
require.NoError(t, err, "Gateway Ping failed")
41+
assert.Equal(t, "gateway", resp.ServiceName)
4642
t.Logf("Gateway is healthy: %s", resp.Message)
4743
})
4844

4945
// Test Orchestrator
5046
t.Run("Orchestrator", func(t *testing.T) {
5147
addr := getEnvOrDefault("ORCHESTRATOR_ADDR", "localhost:8082")
5248
conn, err := waitForServer(t, addr, serverReadyTimeout)
53-
if err != nil {
54-
t.Fatalf("Orchestrator server not ready: %v", err)
55-
}
49+
require.NoError(t, err, "Orchestrator server not ready")
5650
defer conn.Close()
5751

5852
client := orchestratorpb.NewSubmitQueueOrchestratorClient(conn)
5953
ctx, cancel := context.WithTimeout(context.Background(), defaultTimeout)
6054
defer cancel()
6155

6256
resp, err := client.Ping(ctx, &orchestratorpb.PingRequest{Message: "e2e test"})
63-
if err != nil {
64-
t.Fatalf("Orchestrator Ping failed: %v", err)
65-
}
66-
if resp.ServiceName != "orchestrator" {
67-
t.Errorf("Expected service name 'orchestrator', got '%s'", resp.ServiceName)
68-
}
57+
require.NoError(t, err, "Orchestrator Ping failed")
58+
assert.Equal(t, "orchestrator", resp.ServiceName)
6959
t.Logf("Orchestrator is healthy: %s", resp.Message)
7060
})
7161

7262
// Test Speculator
7363
t.Run("Speculator", func(t *testing.T) {
7464
addr := getEnvOrDefault("SPECULATOR_ADDR", "localhost:8083")
7565
conn, err := waitForServer(t, addr, serverReadyTimeout)
76-
if err != nil {
77-
t.Fatalf("Speculator server not ready: %v", err)
78-
}
66+
require.NoError(t, err, "Speculator server not ready")
7967
defer conn.Close()
8068

8169
client := speculatorpb.NewSubmitQueueSpeculatorClient(conn)
8270
ctx, cancel := context.WithTimeout(context.Background(), defaultTimeout)
8371
defer cancel()
8472

8573
resp, err := client.Ping(ctx, &speculatorpb.PingRequest{Message: "e2e test"})
86-
if err != nil {
87-
t.Fatalf("Speculator Ping failed: %v", err)
88-
}
89-
if resp.ServiceName != "speculator" {
90-
t.Errorf("Expected service name 'speculator', got '%s'", resp.ServiceName)
91-
}
74+
require.NoError(t, err, "Speculator Ping failed")
75+
assert.Equal(t, "speculator", resp.ServiceName)
9276
t.Logf("Speculator is healthy: %s", resp.Message)
9377
})
9478

0 commit comments

Comments
 (0)