ferenc-csaky commented on code in PR #19870:
URL: https://github.com/apache/flink/pull/19870#discussion_r1339298186
##########
flink-scala/src/test/scala/org/apache/flink/api/scala/typeutils/EnumValueSerializerCompatibilityTest.scala:
##########
@@ -19,26 +19,25 @@ package org.apache.flink.api.scala.typeutils
import
org.apache.flink.api.common.typeutils.{TypeSerializerSchemaCompatibility,
TypeSerializerSnapshotSerializationUtil}
import org.apache.flink.core.memory.{DataInputViewStreamWrapper,
DataOutputViewStreamWrapper}
-import org.apache.flink.util.TestLogger
-import org.junit.{Rule, Test}
-import org.junit.Assert._
-import org.junit.rules.TemporaryFolder
-import org.scalatest.junit.JUnitSuiteLike
+import org.assertj.core.api.Assertions.{assertThat, fail}
+import org.junit.Test
Review Comment:
This was left on JUnit4.
##########
flink-scala/src/test/scala/org/apache/flink/api/scala/typeutils/EnumValueSerializerCompatibilityTest.scala:
##########
@@ -19,26 +19,25 @@ package org.apache.flink.api.scala.typeutils
import
org.apache.flink.api.common.typeutils.{TypeSerializerSchemaCompatibility,
TypeSerializerSnapshotSerializationUtil}
import org.apache.flink.core.memory.{DataInputViewStreamWrapper,
DataOutputViewStreamWrapper}
-import org.apache.flink.util.TestLogger
-import org.junit.{Rule, Test}
-import org.junit.Assert._
-import org.junit.rules.TemporaryFolder
-import org.scalatest.junit.JUnitSuiteLike
+import org.assertj.core.api.Assertions.{assertThat, fail}
+import org.junit.Test
+import org.junit.jupiter.api.io.TempDir
import java.io._
import java.net.{URL, URLClassLoader}
+import java.nio.file.Files
import scala.reflect.NameTransformer
import scala.tools.nsc.{Global, Settings}
import scala.tools.nsc.reporters.ConsoleReporter
-class EnumValueSerializerCompatibilityTest extends TestLogger with
JUnitSuiteLike {
+class EnumValueSerializerCompatibilityTest {
- private val _tempFolder = new TemporaryFolder()
+ private val _tempFolder = Files.createTempDirectory("")
- @Rule
- def tempFolder = _tempFolder
+ @TempDir
+ private def tempFolder = _tempFolder
Review Comment:
Is `@TempDir` utilized in any way like this? IMO we could leverage
`@TempDir` as a test method param approach here so it is not necessary to deal
with the required initialization:
```java
@Test
def checkIdenticalEnums(@TempDir tempFolder: Path): Unit = {
assertThat(checkCompatibility(enumA, enumA,
tempFolder).isCompatibleAsIs).isTrue
}
```
It has to be added to every method, but IMO it will be cleaner overall. Or
maybe init the class level field with `TestFileUtils.createTempDir()`, which
will be cleaned up.
##########
flink-scala/src/test/scala/org/apache/flink/api/scala/typeutils/TypeInfoFactoryTest.scala:
##########
@@ -25,74 +25,73 @@ import org.apache.flink.api.java.typeutils.{EitherTypeInfo
=> JavaEitherTypeInfo
import org.apache.flink.api.java.typeutils.TypeInfoFactoryTest._
import org.apache.flink.api.scala._
import org.apache.flink.api.scala.typeutils.TypeInfoFactoryTest._
-import org.apache.flink.util.TestLogger
-import org.junit.Assert._
-import org.junit.Test
-import org.scalatest.junit.JUnitSuiteLike
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.Test
import java.lang.reflect.Type
import java.util
-class TypeInfoFactoryTest extends TestLogger with JUnitSuiteLike {
+class TypeInfoFactoryTest {
@Test
def testSimpleType(): Unit = {
val ti = createTypeInformation[ScalaIntLike]
- assertEquals(INT_TYPE_INFO, ti)
+ assertThat(ti).isEqualTo(INT_TYPE_INFO)
}
@Test
def testMyTuple(): Unit = {
val ti = createTypeInformation[MyTuple[Double, String]]
- assertTrue(ti.isInstanceOf[MyTupleTypeInfo[_, _]])
+ assertThat(ti).isInstanceOf(classOf[MyTupleTypeInfo[_, _]])
val mtti = ti.asInstanceOf[MyTupleTypeInfo[_, _]]
- assertEquals(DOUBLE_TYPE_INFO, mtti.getField0)
- assertEquals(STRING_TYPE_INFO, mtti.getField1)
+ assertThat(mtti.getField0).isEqualTo(DOUBLE_TYPE_INFO)
+ assertThat(mtti.getField1).isEqualTo(STRING_TYPE_INFO)
}
@Test
- def testMyTupleHierarchy() {
+ def testMyTupleHierarchy(): Unit = {
val ti = createTypeInformation[MyTuple2]
- assertTrue(ti.isInstanceOf[MyTupleTypeInfo[_, _]])
+ assertThat(ti).isInstanceOf(classOf[MyTupleTypeInfo[_, _]])
val mtti = ti.asInstanceOf[MyTupleTypeInfo[_, _]]
- assertEquals(STRING_TYPE_INFO, mtti.getField0)
- assertEquals(BOOLEAN_TYPE_INFO, mtti.getField1)
+ assertThat(mtti.getField0).isEqualTo(STRING_TYPE_INFO)
+ assertThat(mtti.getField1).isEqualTo(BOOLEAN_TYPE_INFO)
val ti2 = createTypeInformation[MyScalaTupleClass]
- assertTrue(ti2.isInstanceOf[MyTupleTypeInfo[_, _]])
+ assertThat(ti2).isInstanceOf(classOf[MyTupleTypeInfo[_, _]])
val mtti2 = ti2.asInstanceOf[MyTupleTypeInfo[_, _]]
- assertEquals(STRING_TYPE_INFO, mtti.getField0)
- assertEquals(BOOLEAN_TYPE_INFO, mtti.getField1)
+ // TODO: should it be mtti2?
Review Comment:
I definitely think so.
##########
flink-scala/src/test/java/org/apache/flink/runtime/misc/KryoSerializerRegistrationsTest.java:
##########
@@ -91,41 +84,4 @@ public void testDefaultKryoRegisteredClassesDidNotChange()
throws Exception {
}
}
}
-
- /**
- * Creates a Kryo serializer and writes the default registrations out to a
comma separated file
- * with one entry per line:
- *
- * <pre>
- * id,class
- * </pre>
- *
- * <p>The produced file is used to check that the registered IDs don't
change in future Flink
- * versions.
- *
- * <p>This method is not used in the tests, but documents how the test
file has been created and
- * can be used to re-create it if needed.
- *
- * @param filePath File path to write registrations to
- */
- private void writeDefaultKryoRegistrations(String filePath) throws
IOException {
Review Comment:
Was this unused, I guess?
##########
flink-scala/pom.xml:
##########
@@ -272,6 +272,12 @@ under the License.
<goals>
<goal>test-jar</goal>
</goals>
+ <configuration>
+ <excludes>
+ <!-- test-jar
is still used by JUnit4 modules -->
+
<exclude>META-INF/services/org.junit.jupiter.api.extension.Extension</exclude>
Review Comment:
Maybe highlight this with a TODO to remove this exclude after the migration
is complete and JUnit4 is not necessary anymore?
##########
flink-scala/src/test/scala/org/apache/flink/api/scala/types/TypeInformationGenTest.scala:
##########
@@ -132,207 +133,196 @@ class TypeInformationGenTest {
createTypeInformation[Array[T]]
}
- Assert.assertEquals(
- PrimitiveArrayTypeInfo.BOOLEAN_PRIMITIVE_ARRAY_TYPE_INFO,
- getType(boolArray))
+ assertThat(getType(boolArray)).isEqualTo(
+ PrimitiveArrayTypeInfo.BOOLEAN_PRIMITIVE_ARRAY_TYPE_INFO
+ )
- Assert.assertEquals(PrimitiveArrayTypeInfo.BYTE_PRIMITIVE_ARRAY_TYPE_INFO,
getType(byteArray))
+
assertThat(getType(byteArray)).isEqualTo(PrimitiveArrayTypeInfo.BYTE_PRIMITIVE_ARRAY_TYPE_INFO)
- Assert.assertEquals(PrimitiveArrayTypeInfo.CHAR_PRIMITIVE_ARRAY_TYPE_INFO,
getType(charArray))
+
assertThat(getType(charArray)).isEqualTo(PrimitiveArrayTypeInfo.CHAR_PRIMITIVE_ARRAY_TYPE_INFO)
-
Assert.assertEquals(PrimitiveArrayTypeInfo.SHORT_PRIMITIVE_ARRAY_TYPE_INFO,
getType(shortArray))
+ assertThat(getType(shortArray)).isEqualTo(
+ PrimitiveArrayTypeInfo.SHORT_PRIMITIVE_ARRAY_TYPE_INFO)
- Assert.assertEquals(PrimitiveArrayTypeInfo.INT_PRIMITIVE_ARRAY_TYPE_INFO,
getType(intArray))
+
assertThat(getType(intArray)).isEqualTo(PrimitiveArrayTypeInfo.INT_PRIMITIVE_ARRAY_TYPE_INFO)
- Assert.assertEquals(PrimitiveArrayTypeInfo.LONG_PRIMITIVE_ARRAY_TYPE_INFO,
getType(longArray))
+
assertThat(getType(longArray)).isEqualTo(PrimitiveArrayTypeInfo.LONG_PRIMITIVE_ARRAY_TYPE_INFO)
-
Assert.assertEquals(PrimitiveArrayTypeInfo.FLOAT_PRIMITIVE_ARRAY_TYPE_INFO,
getType(floatArray))
+ assertThat(getType(floatArray)).isEqualTo(
+ PrimitiveArrayTypeInfo.FLOAT_PRIMITIVE_ARRAY_TYPE_INFO)
- Assert.assertEquals(
- PrimitiveArrayTypeInfo.DOUBLE_PRIMITIVE_ARRAY_TYPE_INFO,
- getType(doubleArray))
+ assertThat(getType(doubleArray)).isEqualTo(
+ PrimitiveArrayTypeInfo.DOUBLE_PRIMITIVE_ARRAY_TYPE_INFO)
- Assert.assertEquals(BasicArrayTypeInfo.STRING_ARRAY_TYPE_INFO,
getType(stringArray))
+
assertThat(getType(stringArray)).isEqualTo(BasicArrayTypeInfo.STRING_ARRAY_TYPE_INFO)
- Assert.assertTrue(getType(objectArray).isInstanceOf[ObjectArrayTypeInfo[_,
_]])
- Assert.assertTrue(
+
assertThat(getType(objectArray)).isInstanceOf(classOf[ObjectArrayTypeInfo[_,
_]])
+ assertThat(
getType(objectArray)
.asInstanceOf[ObjectArrayTypeInfo[_, _]]
.getComponentInfo
- .isInstanceOf[PojoTypeInfo[_]])
+ .isInstanceOf[PojoTypeInfo[_]]).isTrue
}
@Test
def testTupleWithBasicTypes(): Unit = {
val ti = createTypeInformation[(Int, Long, Double, Float, Boolean, String,
Char, Short, Byte)]
- Assert.assertTrue(ti.isTupleType)
- Assert.assertEquals(9, ti.getArity)
- Assert.assertTrue(ti.isInstanceOf[TupleTypeInfoBase[_]])
+ assertThat(ti.isTupleType).isTrue
+ assertThat(ti.getArity).isEqualTo(9)
+ assertThat(ti).isInstanceOf(classOf[TupleTypeInfoBase[_]])
val tti = ti.asInstanceOf[TupleTypeInfoBase[_]]
- Assert.assertEquals(classOf[Tuple9[_, _, _, _, _, _, _, _, _]],
tti.getTypeClass)
+ assertThat(tti.getTypeClass).isEqualTo(classOf[(_, _, _, _, _, _, _, _,
_)])
for (i <- 0 until 9) {
- Assert.assertTrue(tti.getTypeAt(i).isInstanceOf[BasicTypeInfo[_]])
+ assertThat(tti.getTypeAt(i)).isInstanceOf(classOf[BasicTypeInfo[_]])
}
- Assert.assertEquals(BasicTypeInfo.INT_TYPE_INFO, tti.getTypeAt(0))
- Assert.assertEquals(BasicTypeInfo.LONG_TYPE_INFO, tti.getTypeAt(1))
- Assert.assertEquals(BasicTypeInfo.DOUBLE_TYPE_INFO, tti.getTypeAt(2))
- Assert.assertEquals(BasicTypeInfo.FLOAT_TYPE_INFO, tti.getTypeAt(3))
- Assert.assertEquals(BasicTypeInfo.BOOLEAN_TYPE_INFO, tti.getTypeAt(4))
- Assert.assertEquals(BasicTypeInfo.STRING_TYPE_INFO, tti.getTypeAt(5))
- Assert.assertEquals(BasicTypeInfo.CHAR_TYPE_INFO, tti.getTypeAt(6))
- Assert.assertEquals(BasicTypeInfo.SHORT_TYPE_INFO, tti.getTypeAt(7))
- Assert.assertEquals(BasicTypeInfo.BYTE_TYPE_INFO, tti.getTypeAt(8))
+ assertThat(tti.getTypeAt(0)).isEqualTo(BasicTypeInfo.INT_TYPE_INFO)
+ assertThat(tti.getTypeAt(1)).isEqualTo(BasicTypeInfo.LONG_TYPE_INFO)
+ assertThat(tti.getTypeAt(2)).isEqualTo(BasicTypeInfo.DOUBLE_TYPE_INFO)
+ assertThat(tti.getTypeAt(3)).isEqualTo(BasicTypeInfo.FLOAT_TYPE_INFO)
+ assertThat(tti.getTypeAt(4)).isEqualTo(BasicTypeInfo.BOOLEAN_TYPE_INFO)
+ assertThat(tti.getTypeAt(5)).isEqualTo(BasicTypeInfo.STRING_TYPE_INFO)
+ assertThat(tti.getTypeAt(6)).isEqualTo(BasicTypeInfo.CHAR_TYPE_INFO)
+ assertThat(tti.getTypeAt(7)).isEqualTo(BasicTypeInfo.SHORT_TYPE_INFO)
+ assertThat(tti.getTypeAt(8)).isEqualTo(BasicTypeInfo.BYTE_TYPE_INFO)
}
@Test
def testTupleWithTuples(): Unit = {
val ti = createTypeInformation[(Tuple1[String], Tuple1[Int], Tuple2[Long,
Long])]
- Assert.assertTrue(ti.isTupleType())
- Assert.assertEquals(3, ti.getArity)
- Assert.assertTrue(ti.isInstanceOf[TupleTypeInfoBase[_]])
+ assertThat(ti.isTupleType).isTrue
+ assertThat(ti.getArity).isEqualTo(3)
+ assertThat(ti).isInstanceOf(classOf[TupleTypeInfoBase[_]])
val tti = ti.asInstanceOf[TupleTypeInfoBase[_]]
- Assert.assertEquals(classOf[Tuple3[_, _, _]], tti.getTypeClass)
- Assert.assertTrue(tti.getTypeAt(0).isTupleType())
- Assert.assertTrue(tti.getTypeAt(1).isTupleType())
- Assert.assertTrue(tti.getTypeAt(2).isTupleType())
- Assert.assertEquals(classOf[Tuple1[_]], tti.getTypeAt(0).getTypeClass)
- Assert.assertEquals(classOf[Tuple1[_]], tti.getTypeAt(1).getTypeClass)
- Assert.assertEquals(classOf[Tuple2[_, _]], tti.getTypeAt(2).getTypeClass)
- Assert.assertEquals(1, tti.getTypeAt(0).getArity)
- Assert.assertEquals(1, tti.getTypeAt(1).getArity)
- Assert.assertEquals(2, tti.getTypeAt(2).getArity)
- Assert.assertEquals(
- BasicTypeInfo.STRING_TYPE_INFO,
- tti.getTypeAt(0).asInstanceOf[TupleTypeInfoBase[_]].getTypeAt(0))
- Assert.assertEquals(
- BasicTypeInfo.INT_TYPE_INFO,
- tti.getTypeAt(1).asInstanceOf[TupleTypeInfoBase[_]].getTypeAt(0))
- Assert.assertEquals(
- BasicTypeInfo.LONG_TYPE_INFO,
- tti.getTypeAt(2).asInstanceOf[TupleTypeInfoBase[_]].getTypeAt(0))
- Assert.assertEquals(
- BasicTypeInfo.LONG_TYPE_INFO,
- tti.getTypeAt(2).asInstanceOf[TupleTypeInfoBase[_]].getTypeAt(1))
+ assertThat(tti.getTypeClass).isEqualTo(classOf[(_, _, _)])
+ assertThat(tti.getTypeAt(0).isTupleType)
+ assertThat(tti.getTypeAt(1).isTupleType)
+ assertThat(tti.getTypeAt(2).isTupleType)
Review Comment:
Missing `.isTrue`.
--
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]
