You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "GOODBOY008 (via GitHub)" <gi...@apache.org> on 2024/04/03 08:50:29 UTC
[PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
GOODBOY008 opened a new pull request, #24612:
URL: https://github.com/apache/flink/pull/24612
Changes:
- Module: flink-core with,Package: Configuration migrate to junit5
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on code in PR #24612:
URL: https://github.com/apache/flink/pull/24612#discussion_r1565149231
##########
flink-core/src/test/java/org/apache/flink/configuration/FilesystemSchemeConfigTest.java:
##########
@@ -21,83 +21,79 @@
import org.apache.flink.core.fs.FileSystem;
import org.apache.flink.core.fs.UnsupportedFileSystemSchemeException;
import org.apache.flink.core.fs.local.LocalFileSystem;
-import org.apache.flink.util.TestLogger;
-import org.junit.After;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import java.io.File;
import java.io.IOException;
import java.net.URI;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
/** Tests for the configuration of the default file system scheme. */
-public class FilesystemSchemeConfigTest extends TestLogger {
+public class FilesystemSchemeConfigTest {
- @Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
+ @TempDir public File tempFolder;
- @After
- public void clearFsSettings() throws IOException {
+ @AfterEach
+ void clearFsSettings() throws IOException {
FileSystem.initialize(new Configuration());
}
// ------------------------------------------------------------------------
@Test
- public void testDefaultsToLocal() throws Exception {
- URI justPath = new URI(tempFolder.newFile().toURI().getPath());
- assertNull(justPath.getScheme());
+ void testDefaultsToLocal() throws Exception {
+ URI justPath = new URI(File.createTempFile("junit", null, tempFolder).toURI().getPath());
+ assertThat(justPath.getScheme()).isNull();
FileSystem fs = FileSystem.get(justPath);
- assertEquals("file", fs.getUri().getScheme());
+ assertThat(fs.getUri().getScheme()).isEqualTo("file");
}
@Test
- public void testExplicitlySetToLocal() throws Exception {
+ void testExplicitlySetToLocal() throws Exception {
final Configuration conf = new Configuration();
conf.set(CoreOptions.DEFAULT_FILESYSTEM_SCHEME, LocalFileSystem.getLocalFsURI().toString());
FileSystem.initialize(conf);
- URI justPath = new URI(tempFolder.newFile().toURI().getPath());
- assertNull(justPath.getScheme());
+ URI justPath = new URI(File.createTempFile("junit", null, tempFolder).toURI().getPath());
+ assertThat(justPath.getScheme()).isNull();
FileSystem fs = FileSystem.get(justPath);
- assertEquals("file", fs.getUri().getScheme());
+ assertThat(fs.getUri().getScheme()).isEqualTo("file");
}
@Test
- public void testExplicitlySetToOther() throws Exception {
+ void testExplicitlySetToOther() throws Exception {
final Configuration conf = new Configuration();
conf.set(CoreOptions.DEFAULT_FILESYSTEM_SCHEME, "otherFS://localhost:1234/");
FileSystem.initialize(conf);
- URI justPath = new URI(tempFolder.newFile().toURI().getPath());
- assertNull(justPath.getScheme());
+ URI justPath = new URI(File.createTempFile("junit", null, tempFolder).toURI().getPath());
+ assertThat(justPath.getScheme()).isNull();
try {
FileSystem.get(justPath);
fail("should have failed with an exception");
} catch (UnsupportedFileSystemSchemeException e) {
- assertTrue(e.getMessage().contains("otherFS"));
+ assertThat(e.getMessage()).contains("otherFS");
Review Comment:
It's better to use `assertThatThrownBy` instead.
##########
flink-core/src/test/java/org/apache/flink/configuration/MemorySizeTest.java:
##########
@@ -20,243 +20,232 @@
import org.apache.flink.core.testutils.CommonTestUtils;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
import java.io.IOException;
import static org.apache.flink.configuration.MemorySize.MemoryUnit.MEGA_BYTES;
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
-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.assertThatThrownBy;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
/** Tests for the {@link MemorySize} class. */
-public class MemorySizeTest {
+class MemorySizeTest {
@Test
- public void testUnitConversion() {
+ void testUnitConversion() {
final MemorySize zero = MemorySize.ZERO;
- assertEquals(0, zero.getBytes());
- assertEquals(0, zero.getKibiBytes());
- assertEquals(0, zero.getMebiBytes());
- assertEquals(0, zero.getGibiBytes());
- assertEquals(0, zero.getTebiBytes());
+ assertThat(zero.getBytes()).isZero();
+ assertThat(zero.getKibiBytes()).isZero();
+ assertThat(zero.getMebiBytes()).isZero();
+ assertThat(zero.getGibiBytes()).isZero();
+ assertThat(zero.getTebiBytes()).isZero();
final MemorySize bytes = new MemorySize(955);
- assertEquals(955, bytes.getBytes());
- assertEquals(0, bytes.getKibiBytes());
- assertEquals(0, bytes.getMebiBytes());
- assertEquals(0, bytes.getGibiBytes());
- assertEquals(0, bytes.getTebiBytes());
+ assertThat(bytes.getBytes()).isEqualTo(955);
+ assertThat(bytes.getKibiBytes()).isZero();
+ assertThat(bytes.getMebiBytes()).isZero();
+ assertThat(bytes.getGibiBytes()).isZero();
+ assertThat(bytes.getTebiBytes()).isZero();
final MemorySize kilos = new MemorySize(18500);
- assertEquals(18500, kilos.getBytes());
- assertEquals(18, kilos.getKibiBytes());
- assertEquals(0, kilos.getMebiBytes());
- assertEquals(0, kilos.getGibiBytes());
- assertEquals(0, kilos.getTebiBytes());
+ assertThat(kilos.getBytes()).isEqualTo(18500);
+ assertThat(kilos.getKibiBytes()).isEqualTo(18);
+ assertThat(kilos.getMebiBytes()).isZero();
+ assertThat(kilos.getGibiBytes()).isZero();
+ assertThat(kilos.getTebiBytes()).isZero();
final MemorySize megas = new MemorySize(15 * 1024 * 1024);
- assertEquals(15_728_640, megas.getBytes());
- assertEquals(15_360, megas.getKibiBytes());
- assertEquals(15, megas.getMebiBytes());
- assertEquals(0, megas.getGibiBytes());
- assertEquals(0, megas.getTebiBytes());
+ assertThat(megas.getBytes()).isEqualTo(15_728_640);
+ assertThat(megas.getKibiBytes()).isEqualTo(15_360);
+ assertThat(megas.getMebiBytes()).isEqualTo(15);
+ assertThat(megas.getGibiBytes()).isZero();
+ assertThat(megas.getTebiBytes()).isZero();
final MemorySize teras = new MemorySize(2L * 1024 * 1024 * 1024 * 1024 + 10);
- assertEquals(2199023255562L, teras.getBytes());
- assertEquals(2147483648L, teras.getKibiBytes());
- assertEquals(2097152, teras.getMebiBytes());
- assertEquals(2048, teras.getGibiBytes());
- assertEquals(2, teras.getTebiBytes());
+ assertThat(teras.getBytes()).isEqualTo(2199023255562L);
+ assertThat(teras.getKibiBytes()).isEqualTo(2147483648L);
+ assertThat(teras.getMebiBytes()).isEqualTo(2097152);
+ assertThat(teras.getGibiBytes()).isEqualTo(2048);
+ assertThat(teras.getTebiBytes()).isEqualTo(2);
}
- @Test(expected = IllegalArgumentException.class)
- public void testInvalid() {
- new MemorySize(-1);
+ @Test
+ void testInvalid() {
+ assertThatExceptionOfType(IllegalArgumentException.class)
+ .isThrownBy(
+ () -> {
+ new MemorySize(-1);
+ });
}
@Test
- public void testStandardUtils() throws IOException {
+ void testStandardUtils() throws IOException {
final MemorySize size = new MemorySize(1234567890L);
final MemorySize cloned = CommonTestUtils.createCopySerializable(size);
- assertEquals(size, cloned);
- assertEquals(size.hashCode(), cloned.hashCode());
- assertEquals(size.toString(), cloned.toString());
+ assertThat(cloned).isEqualTo(size);
+ assertThat(cloned.hashCode()).isEqualTo(size.hashCode());
+ assertThat(cloned.toString()).isEqualTo(size.toString());
}
@Test
- public void testParseBytes() {
- assertEquals(1234, MemorySize.parseBytes("1234"));
- assertEquals(1234, MemorySize.parseBytes("1234b"));
- assertEquals(1234, MemorySize.parseBytes("1234 b"));
- assertEquals(1234, MemorySize.parseBytes("1234bytes"));
- assertEquals(1234, MemorySize.parseBytes("1234 bytes"));
+ void testParseBytes() {
+ assertThat(MemorySize.parseBytes("1234")).isEqualTo(1234);
+ assertThat(MemorySize.parseBytes("1234b")).isEqualTo(1234);
+ assertThat(MemorySize.parseBytes("1234 b")).isEqualTo(1234);
+ assertThat(MemorySize.parseBytes("1234bytes")).isEqualTo(1234);
+ assertThat(MemorySize.parseBytes("1234 bytes")).isEqualTo(1234);
}
@Test
- public void testParseKibiBytes() {
- assertEquals(667766, MemorySize.parse("667766k").getKibiBytes());
- assertEquals(667766, MemorySize.parse("667766 k").getKibiBytes());
- assertEquals(667766, MemorySize.parse("667766kb").getKibiBytes());
- assertEquals(667766, MemorySize.parse("667766 kb").getKibiBytes());
- assertEquals(667766, MemorySize.parse("667766kibibytes").getKibiBytes());
- assertEquals(667766, MemorySize.parse("667766 kibibytes").getKibiBytes());
+ void testParseKibiBytes() {
+ assertThat(MemorySize.parse("667766k").getKibiBytes()).isEqualTo(667766);
+ assertThat(MemorySize.parse("667766 k").getKibiBytes()).isEqualTo(667766);
+ assertThat(MemorySize.parse("667766kb").getKibiBytes()).isEqualTo(667766);
+ assertThat(MemorySize.parse("667766 kb").getKibiBytes()).isEqualTo(667766);
+ assertThat(MemorySize.parse("667766kibibytes").getKibiBytes()).isEqualTo(667766);
+ assertThat(MemorySize.parse("667766 kibibytes").getKibiBytes()).isEqualTo(667766);
}
@Test
- public void testParseMebiBytes() {
- assertEquals(7657623, MemorySize.parse("7657623m").getMebiBytes());
- assertEquals(7657623, MemorySize.parse("7657623 m").getMebiBytes());
- assertEquals(7657623, MemorySize.parse("7657623mb").getMebiBytes());
- assertEquals(7657623, MemorySize.parse("7657623 mb").getMebiBytes());
- assertEquals(7657623, MemorySize.parse("7657623mebibytes").getMebiBytes());
- assertEquals(7657623, MemorySize.parse("7657623 mebibytes").getMebiBytes());
+ void testParseMebiBytes() {
+ assertThat(MemorySize.parse("7657623m").getMebiBytes()).isEqualTo(7657623);
+ assertThat(MemorySize.parse("7657623 m").getMebiBytes()).isEqualTo(7657623);
+ assertThat(MemorySize.parse("7657623mb").getMebiBytes()).isEqualTo(7657623);
+ assertThat(MemorySize.parse("7657623 mb").getMebiBytes()).isEqualTo(7657623);
+ assertThat(MemorySize.parse("7657623mebibytes").getMebiBytes()).isEqualTo(7657623);
+ assertThat(MemorySize.parse("7657623 mebibytes").getMebiBytes()).isEqualTo(7657623);
}
@Test
- public void testParseGibiBytes() {
- assertEquals(987654, MemorySize.parse("987654g").getGibiBytes());
- assertEquals(987654, MemorySize.parse("987654 g").getGibiBytes());
- assertEquals(987654, MemorySize.parse("987654gb").getGibiBytes());
- assertEquals(987654, MemorySize.parse("987654 gb").getGibiBytes());
- assertEquals(987654, MemorySize.parse("987654gibibytes").getGibiBytes());
- assertEquals(987654, MemorySize.parse("987654 gibibytes").getGibiBytes());
+ void testParseGibiBytes() {
+ assertThat(MemorySize.parse("987654g").getGibiBytes()).isEqualTo(987654);
+ assertThat(MemorySize.parse("987654 g").getGibiBytes()).isEqualTo(987654);
+ assertThat(MemorySize.parse("987654gb").getGibiBytes()).isEqualTo(987654);
+ assertThat(MemorySize.parse("987654 gb").getGibiBytes()).isEqualTo(987654);
+ assertThat(MemorySize.parse("987654gibibytes").getGibiBytes()).isEqualTo(987654);
+ assertThat(MemorySize.parse("987654 gibibytes").getGibiBytes()).isEqualTo(987654);
}
@Test
- public void testParseTebiBytes() {
- assertEquals(1234567, MemorySize.parse("1234567t").getTebiBytes());
- assertEquals(1234567, MemorySize.parse("1234567 t").getTebiBytes());
- assertEquals(1234567, MemorySize.parse("1234567tb").getTebiBytes());
- assertEquals(1234567, MemorySize.parse("1234567 tb").getTebiBytes());
- assertEquals(1234567, MemorySize.parse("1234567tebibytes").getTebiBytes());
- assertEquals(1234567, MemorySize.parse("1234567 tebibytes").getTebiBytes());
+ void testParseTebiBytes() {
+ assertThat(MemorySize.parse("1234567t").getTebiBytes()).isEqualTo(1234567);
+ assertThat(MemorySize.parse("1234567 t").getTebiBytes()).isEqualTo(1234567);
+ assertThat(MemorySize.parse("1234567tb").getTebiBytes()).isEqualTo(1234567);
+ assertThat(MemorySize.parse("1234567 tb").getTebiBytes()).isEqualTo(1234567);
+ assertThat(MemorySize.parse("1234567tebibytes").getTebiBytes()).isEqualTo(1234567);
+ assertThat(MemorySize.parse("1234567 tebibytes").getTebiBytes()).isEqualTo(1234567);
}
@Test
- public void testUpperCase() {
- assertEquals(1L, MemorySize.parse("1 B").getBytes());
- assertEquals(1L, MemorySize.parse("1 K").getKibiBytes());
- assertEquals(1L, MemorySize.parse("1 M").getMebiBytes());
- assertEquals(1L, MemorySize.parse("1 G").getGibiBytes());
- assertEquals(1L, MemorySize.parse("1 T").getTebiBytes());
+ void testUpperCase() {
+ assertThat(MemorySize.parse("1 B").getBytes()).isEqualTo(1L);
+ assertThat(MemorySize.parse("1 K").getKibiBytes()).isEqualTo(1L);
+ assertThat(MemorySize.parse("1 M").getMebiBytes()).isEqualTo(1L);
+ assertThat(MemorySize.parse("1 G").getGibiBytes()).isEqualTo(1L);
+ assertThat(MemorySize.parse("1 T").getTebiBytes()).isEqualTo(1L);
Review Comment:
isOne
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
Posted by "GOODBOY008 (via GitHub)" <gi...@apache.org>.
GOODBOY008 commented on PR #24612:
URL: https://github.com/apache/flink/pull/24612#issuecomment-2033951840
@Jiabao-Sun PTAL
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #24612:
URL: https://github.com/apache/flink/pull/24612#issuecomment-2036688897
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "94ff81023ed7f0fe9e9a2afd45ba73bd0006c134",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "94ff81023ed7f0fe9e9a2afd45ba73bd0006c134",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 94ff81023ed7f0fe9e9a2afd45ba73bd0006c134 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
Posted by "GOODBOY008 (via GitHub)" <gi...@apache.org>.
GOODBOY008 commented on code in PR #24612:
URL: https://github.com/apache/flink/pull/24612#discussion_r1565637869
##########
flink-core/src/test/java/org/apache/flink/configuration/ReadableWritableConfigurationTest.java:
##########
@@ -169,7 +170,7 @@ void testGetOptionalFromObject() {
testSpec.setValue(configuration);
Optional<?> optional = configuration.getOptional(testSpec.getOption());
- assertThat(optional.get(), equalTo(testSpec.getValue()));
+ assertThat(optional.get()).isEqualTo(testSpec.getValue());
Review Comment:
Compile error with suggestion
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
Posted by "GOODBOY008 (via GitHub)" <gi...@apache.org>.
GOODBOY008 commented on code in PR #24612:
URL: https://github.com/apache/flink/pull/24612#discussion_r1565642234
##########
flink-core/src/test/java/org/apache/flink/configuration/GlobalConfigurationTest.java:
##########
@@ -42,57 +42,48 @@ class GlobalConfigurationTest {
@TempDir private File tmpDir;
@Test
- void testConfigurationWithLegacyYAML() {
+ void testConfigurationWithLegacyYAML() throws FileNotFoundException {
File confFile = new File(tmpDir, GlobalConfiguration.LEGACY_FLINK_CONF_FILENAME);
-
- try {
- try (final PrintWriter pw = new PrintWriter(confFile)) {
-
- pw.println("###########################"); // should be skipped
- pw.println("# Some : comments : to skip"); // should be skipped
- pw.println("###########################"); // should be skipped
- pw.println("mykey1: myvalue1"); // OK, simple correct case
- pw.println("mykey2 : myvalue2"); // OK, whitespace before colon is correct
- pw.println("mykey3:myvalue3"); // SKIP, missing white space after colon
- pw.println(" some nonsense without colon and whitespace separator"); // SKIP
- pw.println(" : "); // SKIP
- pw.println(" "); // SKIP (silently)
- pw.println(" "); // SKIP (silently)
- pw.println("mykey4: myvalue4# some comments"); // OK, skip comments only
- pw.println(" mykey5 : myvalue5 "); // OK, trim unnecessary whitespace
- pw.println("mykey6: my: value6"); // OK, only use first ': ' as separator
- pw.println("mykey7: "); // SKIP, no value provided
- pw.println(": myvalue8"); // SKIP, no key provided
-
- pw.println("mykey9: myvalue9"); // OK
- pw.println("mykey9: myvalue10"); // OK, overwrite last value
-
- } catch (FileNotFoundException e) {
- e.printStackTrace();
- }
-
- Configuration conf = GlobalConfiguration.loadConfiguration(tmpDir.getAbsolutePath());
-
- // all distinct keys from confFile1 + confFile2 key
- assertThat(conf.keySet()).hasSize(6);
-
- // keys 1, 2, 4, 5, 6, 7, 8 should be OK and match the expected values
- assertThat(conf.getString("mykey1", null)).isEqualTo("myvalue1");
- assertThat(conf.getString("mykey1", null)).isEqualTo("myvalue1");
- assertThat(conf.getString("mykey2", null)).isEqualTo("myvalue2");
- assertThat(conf.getString("mykey3", "null")).isEqualTo("null");
- assertThat(conf.getString("mykey4", null)).isEqualTo("myvalue4");
- assertThat(conf.getString("mykey5", null)).isEqualTo("myvalue5");
- assertThat(conf.getString("mykey6", null)).isEqualTo("my: value6");
- assertThat(conf.getString("mykey7", "null")).isEqualTo("null");
- assertThat(conf.getString("mykey8", "null")).isEqualTo("null");
- assertThat(conf.getString("mykey9", null)).isEqualTo("myvalue10");
- } finally {
- // Clear the standard yaml flag to avoid impact to other cases.
- GlobalConfiguration.setStandardYaml(true);
- confFile.delete();
- tmpDir.delete();
+ try (PrintWriter pw = new PrintWriter(confFile)) {
+ pw.println("###########################"); // should be skipped
+ pw.println("# Some : comments : to skip"); // should be skipped
+ pw.println("###########################"); // should be skipped
+ pw.println("mykey1: myvalue1"); // OK, simple correct case
+ pw.println("mykey2 : myvalue2"); // OK, whitespace before colon is correct
+ pw.println("mykey3:myvalue3"); // SKIP, missing white space after colon
+ pw.println(" some nonsense without colon and whitespace separator"); // SKIP
+ pw.println(" : "); // SKIP
+ pw.println(" "); // SKIP (silently)
+ pw.println(" "); // SKIP (silently)
+ pw.println("mykey4: myvalue4# some comments"); // OK, skip comments only
+ pw.println(" mykey5 : myvalue5 "); // OK, trim unnecessary whitespace
+ pw.println("mykey6: my: value6"); // OK, only use first ': ' as separator
+ pw.println("mykey7: "); // SKIP, no value provided
+ pw.println(": myvalue8"); // SKIP, no key provided
+
+ pw.println("mykey9: myvalue9"); // OK
+ pw.println("mykey9: myvalue10"); // OK, overwrite last value
}
+ Configuration conf = GlobalConfiguration.loadConfiguration(tmpDir.getAbsolutePath());
+
+ // all distinct keys from confFile1 + confFile2 key
+ assertThat(conf.keySet()).hasSize(6);
+
+ // keys 1, 2, 4, 5, 6, 7, 8 should be OK and match the expected values
+ assertThat(conf.getString("mykey1", null)).isEqualTo("myvalue1");
+ assertThat(conf.getString("mykey1", null)).isEqualTo("myvalue1");
+ assertThat(conf.getString("mykey2", null)).isEqualTo("myvalue2");
+ assertThat(conf.getString("mykey3", "null")).isEqualTo("null");
+ assertThat(conf.getString("mykey4", null)).isEqualTo("myvalue4");
+ assertThat(conf.getString("mykey5", null)).isEqualTo("myvalue5");
+ assertThat(conf.getString("mykey6", null)).isEqualTo("my: value6");
+ assertThat(conf.getString("mykey7", "null")).isEqualTo("null");
+ assertThat(conf.getString("mykey8", "null")).isEqualTo("null");
+ assertThat(conf.getString("mykey9", null)).isEqualTo("myvalue10");
+ // Clear the standard yaml flag to avoid impact to other cases.
+ GlobalConfiguration.setStandardYaml(true);
+ confFile.delete();
+ tmpDir.delete();
Review Comment:
There is no need to clean after test completed.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
Posted by "Jiabao-Sun (via GitHub)" <gi...@apache.org>.
Jiabao-Sun commented on code in PR #24612:
URL: https://github.com/apache/flink/pull/24612#discussion_r1565548513
##########
flink-core/src/test/java/org/apache/flink/configuration/MemorySizePrettyPrintingTest.java:
##########
@@ -45,13 +46,13 @@ public static Object[][] parameters() {
};
}
- @Parameterized.Parameter public MemorySize memorySize;
+ @Parameter private MemorySize memorySize;
- @Parameterized.Parameter(1)
+ @Parameter(value = 1)
Review Comment:
value can be ignored and expectedString can be private.
##########
flink-core/src/test/java/org/apache/flink/configuration/ReadableWritableConfigurationTest.java:
##########
@@ -169,7 +170,7 @@ void testGetOptionalFromObject() {
testSpec.setValue(configuration);
Optional<?> optional = configuration.getOptional(testSpec.getOption());
- assertThat(optional.get(), equalTo(testSpec.getValue()));
+ assertThat(optional.get()).isEqualTo(testSpec.getValue());
Review Comment:
```suggestion
assertThat(optional).hasValue(testSpec.getValue());
```
##########
flink-core/src/test/java/org/apache/flink/configuration/GlobalConfigurationTest.java:
##########
@@ -42,57 +42,48 @@ class GlobalConfigurationTest {
@TempDir private File tmpDir;
@Test
- void testConfigurationWithLegacyYAML() {
+ void testConfigurationWithLegacyYAML() throws FileNotFoundException {
File confFile = new File(tmpDir, GlobalConfiguration.LEGACY_FLINK_CONF_FILENAME);
-
- try {
- try (final PrintWriter pw = new PrintWriter(confFile)) {
-
- pw.println("###########################"); // should be skipped
- pw.println("# Some : comments : to skip"); // should be skipped
- pw.println("###########################"); // should be skipped
- pw.println("mykey1: myvalue1"); // OK, simple correct case
- pw.println("mykey2 : myvalue2"); // OK, whitespace before colon is correct
- pw.println("mykey3:myvalue3"); // SKIP, missing white space after colon
- pw.println(" some nonsense without colon and whitespace separator"); // SKIP
- pw.println(" : "); // SKIP
- pw.println(" "); // SKIP (silently)
- pw.println(" "); // SKIP (silently)
- pw.println("mykey4: myvalue4# some comments"); // OK, skip comments only
- pw.println(" mykey5 : myvalue5 "); // OK, trim unnecessary whitespace
- pw.println("mykey6: my: value6"); // OK, only use first ': ' as separator
- pw.println("mykey7: "); // SKIP, no value provided
- pw.println(": myvalue8"); // SKIP, no key provided
-
- pw.println("mykey9: myvalue9"); // OK
- pw.println("mykey9: myvalue10"); // OK, overwrite last value
-
- } catch (FileNotFoundException e) {
- e.printStackTrace();
- }
-
- Configuration conf = GlobalConfiguration.loadConfiguration(tmpDir.getAbsolutePath());
-
- // all distinct keys from confFile1 + confFile2 key
- assertThat(conf.keySet()).hasSize(6);
-
- // keys 1, 2, 4, 5, 6, 7, 8 should be OK and match the expected values
- assertThat(conf.getString("mykey1", null)).isEqualTo("myvalue1");
- assertThat(conf.getString("mykey1", null)).isEqualTo("myvalue1");
- assertThat(conf.getString("mykey2", null)).isEqualTo("myvalue2");
- assertThat(conf.getString("mykey3", "null")).isEqualTo("null");
- assertThat(conf.getString("mykey4", null)).isEqualTo("myvalue4");
- assertThat(conf.getString("mykey5", null)).isEqualTo("myvalue5");
- assertThat(conf.getString("mykey6", null)).isEqualTo("my: value6");
- assertThat(conf.getString("mykey7", "null")).isEqualTo("null");
- assertThat(conf.getString("mykey8", "null")).isEqualTo("null");
- assertThat(conf.getString("mykey9", null)).isEqualTo("myvalue10");
- } finally {
- // Clear the standard yaml flag to avoid impact to other cases.
- GlobalConfiguration.setStandardYaml(true);
- confFile.delete();
- tmpDir.delete();
+ try (PrintWriter pw = new PrintWriter(confFile)) {
+ pw.println("###########################"); // should be skipped
+ pw.println("# Some : comments : to skip"); // should be skipped
+ pw.println("###########################"); // should be skipped
+ pw.println("mykey1: myvalue1"); // OK, simple correct case
+ pw.println("mykey2 : myvalue2"); // OK, whitespace before colon is correct
+ pw.println("mykey3:myvalue3"); // SKIP, missing white space after colon
+ pw.println(" some nonsense without colon and whitespace separator"); // SKIP
+ pw.println(" : "); // SKIP
+ pw.println(" "); // SKIP (silently)
+ pw.println(" "); // SKIP (silently)
+ pw.println("mykey4: myvalue4# some comments"); // OK, skip comments only
+ pw.println(" mykey5 : myvalue5 "); // OK, trim unnecessary whitespace
+ pw.println("mykey6: my: value6"); // OK, only use first ': ' as separator
+ pw.println("mykey7: "); // SKIP, no value provided
+ pw.println(": myvalue8"); // SKIP, no key provided
+
+ pw.println("mykey9: myvalue9"); // OK
+ pw.println("mykey9: myvalue10"); // OK, overwrite last value
}
+ Configuration conf = GlobalConfiguration.loadConfiguration(tmpDir.getAbsolutePath());
+
+ // all distinct keys from confFile1 + confFile2 key
+ assertThat(conf.keySet()).hasSize(6);
+
+ // keys 1, 2, 4, 5, 6, 7, 8 should be OK and match the expected values
+ assertThat(conf.getString("mykey1", null)).isEqualTo("myvalue1");
+ assertThat(conf.getString("mykey1", null)).isEqualTo("myvalue1");
+ assertThat(conf.getString("mykey2", null)).isEqualTo("myvalue2");
+ assertThat(conf.getString("mykey3", "null")).isEqualTo("null");
+ assertThat(conf.getString("mykey4", null)).isEqualTo("myvalue4");
+ assertThat(conf.getString("mykey5", null)).isEqualTo("myvalue5");
+ assertThat(conf.getString("mykey6", null)).isEqualTo("my: value6");
+ assertThat(conf.getString("mykey7", "null")).isEqualTo("null");
+ assertThat(conf.getString("mykey8", "null")).isEqualTo("null");
+ assertThat(conf.getString("mykey9", null)).isEqualTo("myvalue10");
+ // Clear the standard yaml flag to avoid impact to other cases.
+ GlobalConfiguration.setStandardYaml(true);
+ confFile.delete();
+ tmpDir.delete();
Review Comment:
In exceptional scenarios, will the deletion of temporary directories be ignored?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
Posted by "GOODBOY008 (via GitHub)" <gi...@apache.org>.
GOODBOY008 commented on PR #24612:
URL: https://github.com/apache/flink/pull/24612#issuecomment-2044061547
@Jiabao-Sun @1996fanrui PTAL
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
Posted by "Jiabao-Sun (via GitHub)" <gi...@apache.org>.
Jiabao-Sun commented on code in PR #24612:
URL: https://github.com/apache/flink/pull/24612#discussion_r1565204190
##########
flink-core/src/test/java/org/apache/flink/configuration/ConfigurationConversionsTest.java:
##########
@@ -52,7 +51,9 @@ public class ConfigurationConversionsTest {
private Configuration pc;
- @Before
+ @Parameter private TestSpec testSpec;
+
+ @BeforeEach
public void init() {
Review Comment:
The visibility of class and methods can be package default.
##########
flink-core/src/test/java/org/apache/flink/configuration/FilesystemSchemeConfigTest.java:
##########
@@ -21,83 +21,75 @@
import org.apache.flink.core.fs.FileSystem;
import org.apache.flink.core.fs.UnsupportedFileSystemSchemeException;
import org.apache.flink.core.fs.local.LocalFileSystem;
-import org.apache.flink.util.TestLogger;
-import org.junit.After;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
-import java.io.IOException;
+import java.io.File;
import java.net.URI;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
/** Tests for the configuration of the default file system scheme. */
-public class FilesystemSchemeConfigTest extends TestLogger {
+public class FilesystemSchemeConfigTest {
- @Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
+ @TempDir public File tempFolder;
Review Comment:
private
##########
flink-core/src/test/java/org/apache/flink/configuration/ConfigurationParsingInvalidFormatsTest.java:
##########
@@ -64,35 +66,35 @@ public static Object[][] getSpecs() {
};
}
- @Parameterized.Parameter public ConfigOption<?> option;
+ @Parameter public ConfigOption<?> option;
Review Comment:
visibility can be private.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
Posted by "GOODBOY008 (via GitHub)" <gi...@apache.org>.
GOODBOY008 commented on code in PR #24612:
URL: https://github.com/apache/flink/pull/24612#discussion_r1565637869
##########
flink-core/src/test/java/org/apache/flink/configuration/ReadableWritableConfigurationTest.java:
##########
@@ -169,7 +170,7 @@ void testGetOptionalFromObject() {
testSpec.setValue(configuration);
Optional<?> optional = configuration.getOptional(testSpec.getOption());
- assertThat(optional.get(), equalTo(testSpec.getValue()));
+ assertThat(optional.get()).isEqualTo(testSpec.getValue());
Review Comment:
There is no need to clean after test completed.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-25537][JUnit5 Migration] Module: flink-core with,Package: Configuration [flink]
Posted by "Jiabao-Sun (via GitHub)" <gi...@apache.org>.
Jiabao-Sun merged PR #24612:
URL: https://github.com/apache/flink/pull/24612
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org