Jiabao-Sun commented on code in PR #23241:
URL: https://github.com/apache/flink/pull/23241#discussion_r1299315926
##########
flink-runtime/src/test/java/org/apache/flink/runtime/jobmanager/JobManagerProcessUtilsTest.java:
##########
@@ -37,23 +38,21 @@
import static
org.apache.flink.runtime.jobmanager.JobManagerProcessUtils.JM_LEGACY_HEAP_OPTIONS;
import static
org.apache.flink.runtime.jobmanager.JobManagerProcessUtils.JM_PROCESS_MEMORY_OPTIONS;
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.hasItem;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.collection.IsArrayWithSize.arrayWithSize;
-import static org.hamcrest.collection.IsMapContaining.hasKey;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
/** Tests for {@link JobManagerProcessUtils}. */
-public class JobManagerProcessUtilsTest extends
ProcessMemoryUtilsTestBase<JobManagerProcessSpec> {
+class JobManagerProcessUtilsTest extends
ProcessMemoryUtilsTestBase<JobManagerProcessSpec> {
Review Comment:
This test class seems in module jobmanager.
Is there any dependence?
##########
flink-runtime/src/test/java/org/apache/flink/runtime/clusterframework/BootstrapToolsTest.java:
##########
@@ -590,75 +595,79 @@ public void testGetDynamicPropertiesAsString() {
final String dynamicProperties =
BootstrapTools.getDynamicPropertiesAsString(baseConfig,
targetConfig);
if (OperatingSystem.isWindows()) {
- assertEquals("-Dkey.b=\"b2\" -Dkey.c=\"c\"", dynamicProperties);
+ assertThat(dynamicProperties).isEqualTo("-Dkey.b=\"b2\"
-Dkey.c=\"c\"");
} else {
- assertEquals("-Dkey.b='b2' -Dkey.c='c'", dynamicProperties);
+ assertThat(dynamicProperties).isEqualTo("-Dkey.b='b2'
-Dkey.c='c'");
}
}
@Test
- public void testEscapeDynamicPropertyValueWithSingleQuote() {
+ void testEscapeDynamicPropertyValueWithSingleQuote() {
final String value1 = "#a,b&c^d*e@f(g!h";
- assertEquals("'" + value1 + "'",
BootstrapTools.escapeWithSingleQuote(value1));
+ assertThat(BootstrapTools.escapeWithSingleQuote(value1)).isEqualTo("'"
+ value1 + "'");
final String value2 = "'foobar";
- assertEquals("''\\''foobar'",
BootstrapTools.escapeWithSingleQuote(value2));
+
assertThat(BootstrapTools.escapeWithSingleQuote(value2)).isEqualTo("''\\''foobar'");
final String value3 = "foo''bar";
- assertEquals("'foo'\\'''\\''bar'",
BootstrapTools.escapeWithSingleQuote(value3));
+
assertThat(BootstrapTools.escapeWithSingleQuote(value3)).isEqualTo("'foo'\\'''\\''bar'");
final String value4 = "'foo' 'bar'";
- assertEquals("''\\''foo'\\'' '\\''bar'\\'''",
BootstrapTools.escapeWithSingleQuote(value4));
+ assertThat(BootstrapTools.escapeWithSingleQuote(value4))
+ .isEqualTo("''\\''foo'\\'' '\\''bar'\\'''");
}
@Test
- public void testEscapeDynamicPropertyValueWithDoubleQuote() {
+ void testEscapeDynamicPropertyValueWithDoubleQuote() {
final String value1 = "#a,b&c^d*e@f(g!h";
- assertEquals("\"#a,b&c\"^^\"d*e@f(g!h\"",
BootstrapTools.escapeWithDoubleQuote(value1));
+ assertThat(BootstrapTools.escapeWithDoubleQuote(value1))
+ .isEqualTo("\"#a,b&c\"^^\"d*e@f(g!h\"");
final String value2 = "foo\"bar'";
- assertEquals("\"foo\\\"bar'\"",
BootstrapTools.escapeWithDoubleQuote(value2));
+
assertThat(BootstrapTools.escapeWithDoubleQuote(value2)).isEqualTo("\"foo\\\"bar'\"");
final String value3 = "\"foo\" \"bar\"";
- assertEquals("\"\\\"foo\\\" \\\"bar\\\"\"",
BootstrapTools.escapeWithDoubleQuote(value3));
+ assertThat(BootstrapTools.escapeWithDoubleQuote(value3))
+ .isEqualTo("\"\\\"foo\\\" \\\"bar\\\"\"");
}
@Test
- public void testGetEnvironmentVariables() {
+ void testGetEnvironmentVariables() {
Configuration testConf = new Configuration();
testConf.setString("containerized.master.env.LD_LIBRARY_PATH",
"/usr/lib/native");
Map<String, String> res =
ConfigurationUtils.getPrefixedKeyValuePairs("containerized.master.env.",
testConf);
- Assert.assertEquals(1, res.size());
+ assertThat(res).hasSize(1);
Map.Entry<String, String> entry = res.entrySet().iterator().next();
- Assert.assertEquals("LD_LIBRARY_PATH", entry.getKey());
- Assert.assertEquals("/usr/lib/native", entry.getValue());
+ assertThat(entry.getKey()).isEqualTo("LD_LIBRARY_PATH");
+ assertThat(entry.getValue()).isEqualTo("/usr/lib/native");
Review Comment:
Maybe it can be simplified as
```java
assertThat(res).hasSize(1).containsEntry("LD_LIBRARY_PATH",
"/usr/lib/native")
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/clusterframework/BootstrapToolsTest.java:
##########
@@ -681,27 +689,22 @@ public void testWriteConfigurationAndReload() throws
IOException {
map.put("key4", "AB\"C\"D");
map.put("key5", "AB'\"D:B");
flinkConfig.set(mapConfigOption, map);
- assertThat(
- flinkConfig.get(mapConfigOption).entrySet(),
- containsInAnyOrder(map.entrySet().toArray()));
+ assertThat(flinkConfig.get(mapConfigOption)).containsAllEntriesOf(map);
final ConfigOption<Duration> durationConfigOption =
ConfigOptions.key("test-duration-key").durationType().noDefaultValue();
final Duration duration = Duration.ofMillis(3000);
flinkConfig.set(durationConfigOption, duration);
- assertEquals(duration, flinkConfig.get(durationConfigOption));
+ assertThat(flinkConfig.get(durationConfigOption)).isEqualTo(duration);
BootstrapTools.writeConfiguration(flinkConfig, new File(flinkConfDir,
FLINK_CONF_FILENAME));
final Configuration loadedFlinkConfig =
GlobalConfiguration.loadConfiguration(flinkConfDir.getAbsolutePath());
- assertThat(
- loadedFlinkConfig.get(listStringConfigOption),
containsInAnyOrder(list.toArray()));
- assertThat(
- loadedFlinkConfig.get(listDurationConfigOption),
- containsInAnyOrder(durationList.toArray()));
- assertThat(
- loadedFlinkConfig.get(mapConfigOption).entrySet(),
- containsInAnyOrder(map.entrySet().toArray()));
- assertEquals(duration, loadedFlinkConfig.get(durationConfigOption));
+ assertThat(loadedFlinkConfig.get(listStringConfigOption))
+ .contains(list.toArray((new String[0])));
+ assertThat(loadedFlinkConfig.get(listDurationConfigOption))
+ .contains(durationList.toArray((new Duration[0])));
Review Comment:
```suggestion
assertThat(loadedFlinkConfig.get(listStringConfigOption))
.containsExactlyInAnyOrderElementsOf(list);
assertThat(loadedFlinkConfig.get(listDurationConfigOption))
.containsExactlyInAnyOrderElementsOf(durationList);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/clusterframework/BootstrapToolsTest.java:
##########
@@ -668,9 +677,8 @@ public void testWriteConfigurationAndReload() throws
IOException {
final List<Duration> durationList =
Arrays.asList(Duration.ofSeconds(3), Duration.ofMinutes(1));
flinkConfig.set(listDurationConfigOption, durationList);
- assertThat(
- flinkConfig.get(listDurationConfigOption),
- containsInAnyOrder(durationList.toArray()));
+ assertThat(flinkConfig.get(listDurationConfigOption))
+ .containsAnyOf(durationList.toArray(new Duration[0]));
Review Comment:
```suggestion
assertThat(flinkConfig.get(listDurationConfigOption))
.containsExactlyInAnyOrderElementsOf(durationList);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/clusterframework/BootstrapToolsTest.java:
##########
@@ -590,75 +595,79 @@ public void testGetDynamicPropertiesAsString() {
final String dynamicProperties =
BootstrapTools.getDynamicPropertiesAsString(baseConfig,
targetConfig);
if (OperatingSystem.isWindows()) {
- assertEquals("-Dkey.b=\"b2\" -Dkey.c=\"c\"", dynamicProperties);
+ assertThat(dynamicProperties).isEqualTo("-Dkey.b=\"b2\"
-Dkey.c=\"c\"");
} else {
- assertEquals("-Dkey.b='b2' -Dkey.c='c'", dynamicProperties);
+ assertThat(dynamicProperties).isEqualTo("-Dkey.b='b2'
-Dkey.c='c'");
}
}
@Test
- public void testEscapeDynamicPropertyValueWithSingleQuote() {
+ void testEscapeDynamicPropertyValueWithSingleQuote() {
final String value1 = "#a,b&c^d*e@f(g!h";
- assertEquals("'" + value1 + "'",
BootstrapTools.escapeWithSingleQuote(value1));
+ assertThat(BootstrapTools.escapeWithSingleQuote(value1)).isEqualTo("'"
+ value1 + "'");
final String value2 = "'foobar";
- assertEquals("''\\''foobar'",
BootstrapTools.escapeWithSingleQuote(value2));
+
assertThat(BootstrapTools.escapeWithSingleQuote(value2)).isEqualTo("''\\''foobar'");
final String value3 = "foo''bar";
- assertEquals("'foo'\\'''\\''bar'",
BootstrapTools.escapeWithSingleQuote(value3));
+
assertThat(BootstrapTools.escapeWithSingleQuote(value3)).isEqualTo("'foo'\\'''\\''bar'");
final String value4 = "'foo' 'bar'";
- assertEquals("''\\''foo'\\'' '\\''bar'\\'''",
BootstrapTools.escapeWithSingleQuote(value4));
+ assertThat(BootstrapTools.escapeWithSingleQuote(value4))
+ .isEqualTo("''\\''foo'\\'' '\\''bar'\\'''");
}
@Test
- public void testEscapeDynamicPropertyValueWithDoubleQuote() {
+ void testEscapeDynamicPropertyValueWithDoubleQuote() {
final String value1 = "#a,b&c^d*e@f(g!h";
- assertEquals("\"#a,b&c\"^^\"d*e@f(g!h\"",
BootstrapTools.escapeWithDoubleQuote(value1));
+ assertThat(BootstrapTools.escapeWithDoubleQuote(value1))
+ .isEqualTo("\"#a,b&c\"^^\"d*e@f(g!h\"");
final String value2 = "foo\"bar'";
- assertEquals("\"foo\\\"bar'\"",
BootstrapTools.escapeWithDoubleQuote(value2));
+
assertThat(BootstrapTools.escapeWithDoubleQuote(value2)).isEqualTo("\"foo\\\"bar'\"");
final String value3 = "\"foo\" \"bar\"";
- assertEquals("\"\\\"foo\\\" \\\"bar\\\"\"",
BootstrapTools.escapeWithDoubleQuote(value3));
+ assertThat(BootstrapTools.escapeWithDoubleQuote(value3))
+ .isEqualTo("\"\\\"foo\\\" \\\"bar\\\"\"");
}
@Test
- public void testGetEnvironmentVariables() {
+ void testGetEnvironmentVariables() {
Configuration testConf = new Configuration();
testConf.setString("containerized.master.env.LD_LIBRARY_PATH",
"/usr/lib/native");
Map<String, String> res =
ConfigurationUtils.getPrefixedKeyValuePairs("containerized.master.env.",
testConf);
- Assert.assertEquals(1, res.size());
+ assertThat(res).hasSize(1);
Map.Entry<String, String> entry = res.entrySet().iterator().next();
- Assert.assertEquals("LD_LIBRARY_PATH", entry.getKey());
- Assert.assertEquals("/usr/lib/native", entry.getValue());
+ assertThat(entry.getKey()).isEqualTo("LD_LIBRARY_PATH");
+ assertThat(entry.getValue()).isEqualTo("/usr/lib/native");
}
@Test
- public void testGetEnvironmentVariablesErroneous() {
+ void testGetEnvironmentVariablesErroneous() {
Configuration testConf = new Configuration();
testConf.setString("containerized.master.env.", "/usr/lib/native");
Map<String, String> res =
ConfigurationUtils.getPrefixedKeyValuePairs("containerized.master.env.",
testConf);
- Assert.assertEquals(0, res.size());
+ assertThat(res).isEmpty();
}
@Test
- public void testWriteConfigurationAndReload() throws IOException {
- final File flinkConfDir =
temporaryFolder.newFolder().getAbsoluteFile();
+ void testWriteConfigurationAndReload() throws IOException {
+ final File flinkConfDir =
TempDirUtils.newFolder(temporaryFolder).getAbsoluteFile();
final Configuration flinkConfig = new Configuration();
final ConfigOption<List<String>> listStringConfigOption =
ConfigOptions.key("test-list-string-key").stringType().asList().noDefaultValue();
final List<String> list =
Arrays.asList("A,B,C,D", "A'B'C'D", "A;BCD", "AB\"C\"D",
"AB'\"D:B");
flinkConfig.set(listStringConfigOption, list);
- assertThat(flinkConfig.get(listStringConfigOption),
containsInAnyOrder(list.toArray()));
+ assertThat(flinkConfig.get(listStringConfigOption))
+ .containsAnyOf(list.toArray(new String[0]));
Review Comment:
```suggestion
assertThat(flinkConfig.get(listStringConfigOption))
.containsExactlyInAnyOrderElementsOf(list);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
