RyanSkraba commented on code in PR #20258:
URL: https://github.com/apache/flink/pull/20258#discussion_r1004723756
##########
flink-formats/flink-sequence-file/src/test/java/org/apache/flink/formats/sequencefile/SerializableHadoopConfigurationTest.java:
##########
@@ -19,70 +19,54 @@
package org.apache.flink.formats.sequencefile;
import org.apache.hadoop.conf.Configuration;
-import org.hamcrest.Description;
-import org.hamcrest.TypeSafeMatcher;
-import org.junit.Before;
-import org.junit.Test;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.HamcrestCondition.matching;
-
/** Tests for the {@link SerializableHadoopConfiguration}. */
-public class SerializableHadoopConfigurationTest {
+class SerializableHadoopConfigurationTest {
private static final String TEST_KEY = "test-key";
private static final String TEST_VALUE = "test-value";
private Configuration configuration;
- @Before
- public void createConfigWithCustomProperty() {
+ @BeforeEach
+ void createConfigWithCustomProperty() {
this.configuration = new Configuration();
configuration.set(TEST_KEY, TEST_VALUE);
}
@Test
- public void customPropertiesSurviveSerializationDeserialization()
+ void customPropertiesSurviveSerializationDeserialization()
throws IOException, ClassNotFoundException {
final SerializableHadoopConfiguration serializableConfigUnderTest =
new SerializableHadoopConfiguration(configuration);
final byte[] serializedConfigUnderTest =
serializeAndGetBytes(serializableConfigUnderTest);
final SerializableHadoopConfiguration deserializableConfigUnderTest =
deserializeAndGetConfiguration(serializedConfigUnderTest);
- assertThat(deserializableConfigUnderTest.get())
- .satisfies(matching(hasTheSamePropertiesAs(configuration)));
- }
-
- // ---------------------------------------- Matchers
---------------------------------------- //
-
- private static TypeSafeMatcher<Configuration> hasTheSamePropertiesAs(
- final Configuration expectedConfig) {
- return new TypeSafeMatcher<Configuration>() {
- @Override
- protected boolean matchesSafely(Configuration actualConfig) {
- final String value = actualConfig.get(TEST_KEY);
- return actualConfig != expectedConfig
- && value != null
- && expectedConfig.get(TEST_KEY).equals(value);
- }
-
- @Override
- public void describeTo(Description description) {
- description
- .appendText("a Hadoop Configuration with property:
key=")
- .appendValue(TEST_KEY)
- .appendText(" and value=")
- .appendValue(TEST_VALUE);
- }
- };
+
Assertions.<Configuration>assertThat(deserializableConfigUnderTest.get())
+ .matches(
+ actualConfig -> {
+ final String value = actualConfig.get(TEST_KEY);
+ return actualConfig !=
serializableConfigUnderTest.get()
+ && value != null
+ && serializableConfigUnderTest
+ .get()
+ .get(TEST_KEY)
+ .equals(value);
+ })
+ .describedAs(
Review Comment:
Yes, it seems that the `describedAs` needs to come before any asserts!
In this case, I changed the logic quite a bit from the original to improve
the descriptive error messages. The new logic would print the error message:
```
[ERROR] Failures:
[ERROR]
SerializableHadoopConfigurationTest.customPropertiesSurviveSerializationDeserialization:60
[a Hadoop Configuration with property: key=test-key and value=test-value] (1
failure)
-- failure 1 --
expected: "test-value"
but was: "faked-for-error-message"
```
--
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]
