1996fanrui commented on code in PR #23241:
URL: https://github.com/apache/flink/pull/23241#discussion_r1299380633
##########
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:
Yeah the class in jobmanager module.
I migrated it because `JobManagerProcessUtilsTest` and
`TaskExecutorProcessUtilsTest` extends `ProcessMemoryUtilsTestBase`.
I want to migrate these 3 classes to junit5 together, however
`ProcessMemoryUtilsTestBase` is migrated at
https://github.com/apache/flink/pull/23199, so I updated
`JobManagerProcessUtilsTest` and `TaskExecutorProcessUtilsTest` at this PR.
Do you think it is ok?
##########
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:
Good suggestion, it's very concise! updated
--
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]
