Skip to content

Commit a4fb5b1

Browse files
benjaminballardbenjaminballard
committed
better tests
1 parent 75b88fa commit a4fb5b1

File tree

2 files changed

+173
-42
lines changed

2 files changed

+173
-42
lines changed

‎src/main/java/org/voltdb/meshmonitor/cli/BaseInetSocketAddressConverter.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,11 @@ public InetSocketAddress convert(String value) {
4949

5050
validateHost(host);
5151
validatePort(port);
52-
return new InetSocketAddress(host, port);
52+
if (host.isEmpty() && treatPlainValueAsPort()) {
53+
return new InetSocketAddress(port);
54+
} else {
55+
return new InetSocketAddress(host, port);
56+
}
5357

5458
} else {
5559

@@ -74,7 +78,7 @@ public InetSocketAddress convert(String value) {
7478
}
7579

7680
if (lastColon == 0) {
77-
// no hostname given, but the colon indicates value is a port
81+
// no hostname given, but the colon indicates value should be the port
7882
if (requiresHostname()) {
7983
throw new IllegalArgumentException("Hostname is required. Please provide a valid FQDN or IP address.");
8084
} else {
@@ -87,7 +91,10 @@ public InetSocketAddress convert(String value) {
8791

8892
// there is one and only one colon
8993
host = value.substring(0, lastColon);
90-
port = Integer.parseInt(value.substring(lastColon + 1));
94+
String portString = value.substring(lastColon + 1);
95+
if (!portString.isEmpty()) {
96+
port = Integer.parseInt(portString);
97+
}
9198
validateHost(host);
9299
validatePort(port);
93100
return new InetSocketAddress(host, port);

‎src/test/java/org/voltdb/meshmonitor/cli/InetSocketAddressConvertersTest.java‎

Lines changed: 163 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -19,27 +19,64 @@ public class InetSocketAddressConvertersTest {
1919

2020
// This test covers both custom converter classes:
2121
private final BindInetSocketAddressConverter bconverter = new BindInetSocketAddressConverter();
22+
// Metrics Converter:
23+
// - can be port only (wildcard host)
24+
// - default port is 12223
25+
2226
private final MetricsInetSocketAddressConverter mconverter = new MetricsInetSocketAddressConverter();
23-
private final BaseInetSocketAddressConverter[] converters = {bconverter, mconverter};
27+
// Bind Converter:
28+
// - must have a hostname (it is advertised)
29+
// - defaults to port 12222
2430

25-
// The main differences:
26-
// - Metrics can be port only (wildcard host)
27-
// - Bind must have a hostname (it is advertised)
31+
private final BaseInetSocketAddressConverter[] converters = {bconverter, mconverter};
2832

33+
private void testIllegalArgument(BaseInetSocketAddressConverter converter, String input) {
34+
assertThatThrownBy(() -> converter.convert(input))
35+
.isInstanceOf(IllegalArgumentException.class);
36+
}
2937

3038
// --------- TESTS THAT ARE IDENTICAL FOR BOTH CONVERTERS ----------
3139
@Test
32-
public void shouldThrowExceptionNoClosingBracket() {
40+
public void ipv6MissingOneBracket() {
3341
String input = "[1:2:3:4";
3442

3543
for (BaseInetSocketAddressConverter c : converters) {
36-
assertThatThrownBy(() -> c.convert(input))
37-
.isInstanceOf(IllegalArgumentException.class);
44+
testIllegalArgument(c, input);
45+
}
46+
47+
input = "1:2:3:4]";
48+
49+
for (BaseInetSocketAddressConverter c : converters) {
50+
testIllegalArgument(c, input);
51+
}
52+
}
53+
54+
@Test
55+
public void ipv6NoBrackets() {
56+
String input = "1:2:3:4";
57+
58+
for (BaseInetSocketAddressConverter c : converters) {
59+
testIllegalArgument(c, input);
60+
}
61+
62+
input = "1:2:3:4:1000"; // no brackets with port
63+
64+
for (BaseInetSocketAddressConverter c : converters) {
65+
testIllegalArgument(c, input);
66+
}
67+
}
68+
69+
@Test
70+
public void ipv6NoColon() {
71+
String input = "[1:2:3:4]8080";
72+
73+
for (BaseInetSocketAddressConverter c : converters) {
74+
testIllegalArgument(c, input);
3875
}
3976
}
4077

4178
@Test
42-
public void shouldConvertWithPort() {
79+
public void hostnameWithPort() {
4380

4481
String input = "localhost:8080";
4582

@@ -50,10 +87,30 @@ public void shouldConvertWithPort() {
5087
assertThat(result.getHostName()).isEqualTo("localhost");
5188
assertThat(result.getPort()).isEqualTo(8080);
5289
}
90+
91+
input = "10.0.0.1:8080";
92+
93+
for (BaseInetSocketAddressConverter c : converters) {
94+
InetSocketAddress result = c.convert(input);
95+
96+
assertThat(result).isNotNull();
97+
assertThat(result.getHostName()).isEqualTo("10.0.0.1");
98+
assertThat(result.getPort()).isEqualTo(8080);
99+
}
100+
101+
input = "[1:2:3:4]:8080";
102+
103+
for (BaseInetSocketAddressConverter c : converters) {
104+
InetSocketAddress result = c.convert(input);
105+
106+
assertThat(result).isNotNull();
107+
assertThat(result.getHostName()).isEqualTo("1:2:3:4");
108+
assertThat(result.getPort()).isEqualTo(8080);
109+
}
53110
}
54111

55112
@Test
56-
public void shouldConvertWithoutPort() {
113+
public void hostnameOnly() {
57114

58115
String input = "localhost";
59116

@@ -68,10 +125,84 @@ public void shouldConvertWithoutPort() {
68125
assertThat(result).isNotNull();
69126
assertThat(result.getHostName()).isEqualTo("localhost");
70127
assertThat(result.getPort()).isEqualTo(mconverter.getDefaultPort());
128+
129+
input = "10.0.0.1";
130+
131+
// Bind accepts IPv4 host, adds default port
132+
result = bconverter.convert(input);
133+
assertThat(result).isNotNull();
134+
assertThat(result.getHostName()).isEqualTo(input);
135+
assertThat(result.getPort()).isEqualTo(bconverter.getDefaultPort());
136+
137+
// Metrics accepts IPv4 host, adds different default port
138+
result = mconverter.convert(input);
139+
assertThat(result).isNotNull();
140+
assertThat(result.getHostName()).isEqualTo(input);
141+
assertThat(result.getPort()).isEqualTo(mconverter.getDefaultPort());
142+
143+
input = "[1:2:3:4]";
144+
145+
// Bind accepts IPv4 host, adds default port
146+
result = bconverter.convert(input);
147+
assertThat(result).isNotNull();
148+
assertThat(result.getHostName()).isEqualTo("1:2:3:4");
149+
assertThat(result.getPort()).isEqualTo(bconverter.getDefaultPort());
150+
151+
// Metrics accepts IPv4 host, adds different default port
152+
result = mconverter.convert(input);
153+
assertThat(result).isNotNull();
154+
assertThat(result.getHostName()).isEqualTo("1:2:3:4");
155+
assertThat(result.getPort()).isEqualTo(mconverter.getDefaultPort());
156+
}
157+
158+
@Test
159+
public void hostnameColonNoPort() { // should ignore the extra colon
160+
161+
String input = "localhost:";
162+
163+
// Bind accepts localhost, adds default port
164+
InetSocketAddress result = bconverter.convert(input);
165+
assertThat(result).isNotNull();
166+
assertThat(result.getHostName()).isEqualTo("localhost");
167+
assertThat(result.getPort()).isEqualTo(bconverter.getDefaultPort());
168+
169+
// Metrics accepts localhost, adds different default port
170+
result = mconverter.convert(input);
171+
assertThat(result).isNotNull();
172+
assertThat(result.getHostName()).isEqualTo("localhost");
173+
assertThat(result.getPort()).isEqualTo(mconverter.getDefaultPort());
174+
175+
input = "10.0.0.1:";
176+
177+
// Bind accepts IPv4 host, adds default port
178+
result = bconverter.convert(input);
179+
assertThat(result).isNotNull();
180+
assertThat(result.getHostName()).isEqualTo("10.0.0.1");
181+
assertThat(result.getPort()).isEqualTo(bconverter.getDefaultPort());
182+
183+
// Metrics accepts IPv4 host, adds different default port
184+
result = mconverter.convert(input);
185+
assertThat(result).isNotNull();
186+
assertThat(result.getHostName()).isEqualTo("10.0.0.1");
187+
assertThat(result.getPort()).isEqualTo(mconverter.getDefaultPort());
188+
189+
input = "[1:2:3:4]:";
190+
191+
// Bind accepts IPv4 host, adds default port
192+
result = bconverter.convert(input);
193+
assertThat(result).isNotNull();
194+
assertThat(result.getHostName()).isEqualTo("1:2:3:4");
195+
assertThat(result.getPort()).isEqualTo(bconverter.getDefaultPort());
196+
197+
// Metrics accepts IPv4 host, adds different default port
198+
result = mconverter.convert(input);
199+
assertThat(result).isNotNull();
200+
assertThat(result.getHostName()).isEqualTo("1:2:3:4");
201+
assertThat(result.getPort()).isEqualTo(mconverter.getDefaultPort());
71202
}
72203

73204
@Test
74-
public void shouldThrowExceptionWithInvalidPort() {
205+
public void invalidPort() {
75206

76207
String input = "localhost:abc";
77208

@@ -81,24 +212,15 @@ public void shouldThrowExceptionWithInvalidPort() {
81212
}
82213
}
83214

84-
// IPv6 with brackets
85-
// empty brackets - host name empty
86-
// nothing after ] - use default port
87-
// ]: no port - use default port
88-
// ]:port - use port
89-
// [1:2:3:4]12223 - missing colon should fail
90-
91-
92215
// --------- TESTS THAT ARE DIFFERENT FOR EACH CONVERTER ----------
93216

94217
@Test
95-
public void onlyColonAndPortSpecified() throws UnknownHostException {
218+
public void colonPort() throws UnknownHostException {
96219

97220
String input = ":8080";
98221

99222
// Bind converter requires hostname
100-
assertThatThrownBy(() -> bconverter.convert(input))
101-
.isInstanceOf(IllegalArgumentException.class);
223+
testIllegalArgument(bconverter, input);
102224

103225
// Metrics converter accepts port only (uses wildcard)
104226
InetSocketAddress result = mconverter.convert(input);
@@ -108,7 +230,22 @@ public void onlyColonAndPortSpecified() throws UnknownHostException {
108230
}
109231

110232
@Test
111-
public void handleOnlyColon() {
233+
public void emptyBracketsColonPort() throws UnknownHostException {
234+
235+
String input = "[]:8080";
236+
237+
// Bind converter requires hostname
238+
testIllegalArgument(bconverter, input);
239+
240+
// Metrics converter accepts port only (uses wildcard)
241+
InetSocketAddress result = mconverter.convert(input);
242+
assertThat(result).isNotNull();
243+
assertThat(result.getHostName()).isEqualTo("0.0.0.0");
244+
assertThat(result.getPort()).isEqualTo(8080);
245+
}
246+
247+
@Test
248+
public void colonOnly() {
112249

113250
String input = ":";
114251

@@ -119,12 +256,11 @@ public void handleOnlyColon() {
119256
assertThat(result.getPort()).isEqualTo(mconverter.getDefaultPort());
120257

121258
// Bind should throw exception - hostname is required
122-
assertThatThrownBy(() -> bconverter.convert(input))
123-
.isInstanceOf(IllegalArgumentException.class);
259+
testIllegalArgument(bconverter, input);
124260
}
125-
261+
126262
@Test
127-
public void handleEmptyString() {
263+
public void emptyString() {
128264

129265
String input = "";
130266

@@ -135,18 +271,6 @@ public void handleEmptyString() {
135271
assertThat(result.getPort()).isEqualTo(mconverter.getDefaultPort());
136272

137273
// Bind should throw exception - hostname is required
138-
assertThatThrownBy(() -> bconverter.convert(input))
139-
.isInstanceOf(IllegalArgumentException.class);
140-
}
141-
142-
//@Test
143-
public void shouldConvertIPv6() {
144-
145-
String input = "[1:2:3:4]"; // no port
146-
input = "[1:2:3:4]:"; // colon but no port
147-
input = "[1:2:3:4]:1000"; // with port
148-
input = "1:2:3:4:1000"; // no brackets with port
149-
input = "1:2:3:4"; // no brackets no port
274+
testIllegalArgument(bconverter, input);
150275
}
151-
152276
}

0 commit comments

Comments
 (0)