Skip to content

Commit d382684

Browse files
gkeesh7gauravkuber
andauthored
Refine agent run validation (#513)
Co-authored-by: gauravk <gauravk@uber.com>
1 parent 0fde566 commit d382684

File tree

2 files changed

+64
-9
lines changed

2 files changed

+64
-9
lines changed

agent/cmd/cmd.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,7 @@ func WithEffect(f func()) Option {
108108

109109
// Run runs the agent.
110110
func Run(flags *Flags, opts ...Option) {
111-
if flags.PeerPort == 0 {
112-
panic("must specify non-zero peer port")
113-
}
114-
if flags.AgentServerPort == 0 {
115-
panic("must specify non-zero agent server port")
116-
}
117-
if flags.AgentRegistryPort == 0 {
118-
panic("must specify non-zero agent registry port")
119-
}
111+
validateRequiredPorts(flags)
120112

121113
var overrides options
122114
for _, o := range opts {
@@ -272,6 +264,19 @@ func Run(flags *Flags, opts ...Option) {
272264
}
273265
}
274266

267+
// validateRequiredPorts panics if any required port flags are not set.
268+
func validateRequiredPorts(flags *Flags) {
269+
if flags.PeerPort == 0 {
270+
panic("must specify non-zero peer port")
271+
}
272+
if flags.AgentServerPort == 0 {
273+
panic("must specify non-zero agent server port")
274+
}
275+
if flags.AgentRegistryPort == 0 {
276+
panic("must specify non-zero agent registry port")
277+
}
278+
}
279+
275280
// heartbeatTicker provides the minimal ticker contract required by heartbeat.
276281
type heartbeatTicker interface {
277282
Chan() <-chan time.Time

agent/cmd/cmd_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,56 @@ func TestRunValidation(t *testing.T) {
113113
})
114114
}
115115
}
116+
117+
func TestValidateRequiredPorts(t *testing.T) {
118+
tests := []struct {
119+
desc string
120+
flags Flags
121+
shouldPanic bool
122+
panicMsg string
123+
}{
124+
{
125+
desc: "valid ports",
126+
flags: Flags{
127+
PeerPort: 1,
128+
AgentServerPort: 2,
129+
AgentRegistryPort: 3,
130+
},
131+
},
132+
{
133+
desc: "missing peer port",
134+
flags: Flags{AgentServerPort: 1, AgentRegistryPort: 1},
135+
shouldPanic: true,
136+
panicMsg: "must specify non-zero peer port",
137+
},
138+
{
139+
desc: "missing agent server port",
140+
flags: Flags{PeerPort: 1, AgentRegistryPort: 1},
141+
shouldPanic: true,
142+
panicMsg: "must specify non-zero agent server port",
143+
},
144+
{
145+
desc: "missing agent registry port",
146+
flags: Flags{PeerPort: 1, AgentServerPort: 1},
147+
shouldPanic: true,
148+
panicMsg: "must specify non-zero agent registry port",
149+
},
150+
}
151+
152+
for _, test := range tests {
153+
t.Run(test.desc, func(t *testing.T) {
154+
if test.shouldPanic {
155+
assert.PanicsWithValue(t, test.panicMsg, func() {
156+
validateRequiredPorts(&test.flags)
157+
})
158+
} else {
159+
assert.NotPanics(t, func() {
160+
validateRequiredPorts(&test.flags)
161+
})
162+
}
163+
})
164+
}
165+
}
116166
func TestHeartbeatWithTicker(t *testing.T) {
117167
scope := tally.NewTestScope("", nil)
118168
mockClock := clock.NewMock()

0 commit comments

Comments
 (0)