KurtYoung commented on a change in pull request #15136:
URL: https://github.com/apache/flink/pull/15136#discussion_r592973280



##########
File path: 
flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/expressions/utils/ExpressionTestBase.scala
##########
@@ -108,95 +108,100 @@ abstract class ExpressionTestBase {
 
   @After
   def evaluateExprs(): Unit = {
-    val ctx = CodeGeneratorContext(config)
-    val inputType = if (containsLegacyTypes) {
-      fromTypeInfoToLogicalType(typeInfo)
-    } else {
-      resolvedDataType.getLogicalType
-    }
-    val exprGenerator = new ExprCodeGenerator(ctx, nullableInput = 
false).bindInput(inputType)
-
-    // cast expressions to String
-    val stringTestExprs = testExprs.map(expr => relBuilder.cast(expr._2, 
VARCHAR))
-
-    // generate code
-    val resultType = RowType.of(Seq.fill(testExprs.size)(
-      new VarCharType(VarCharType.MAX_LENGTH)): _*)
-
-    val exprs = stringTestExprs.map(exprGenerator.generateExpression)
-    val genExpr = exprGenerator.generateResultExpression(exprs, resultType, 
classOf[BinaryRowData])
-
-    val bodyCode =
-      s"""
-         |${genExpr.code}
-         |return ${genExpr.resultTerm};
-        """.stripMargin
-
-    val genFunc = FunctionCodeGenerator.generateFunction[MapFunction[RowData, 
BinaryRowData]](
-      ctx,
-      "TestFunction",
-      classOf[MapFunction[RowData, BinaryRowData]],
-      bodyCode,
-      resultType,
-      inputType)
-
-    val mapper = genFunc.newInstance(getClass.getClassLoader)
-
-    val isRichFunction = mapper.isInstanceOf[RichFunction]
-
-    // call setRuntimeContext method and open method for RichFunction
-    if (isRichFunction) {
-      val richMapper = mapper.asInstanceOf[RichMapFunction[_, _]]
-      val t = new RuntimeUDFContext(
-        new TaskInfo("ExpressionTest", 1, 0, 1, 1),
-        classOf[ExpressionTestBase].getClassLoader,
-        env.getConfig,
-        Collections.emptyMap(),
-        Collections.emptyMap(),
-        null)
-      richMapper.setRuntimeContext(t)
-      richMapper.open(new Configuration())
-    }
-
-    val testRow = if (containsLegacyTypes) {
-      val converter = DataFormatConverters
-        .getConverterForDataType(resolvedDataType)
-        .asInstanceOf[DataFormatConverter[RowData, Row]]
-      converter.toInternal(testData)
-    } else {
-      val converter = DataStructureConverters
-        .getConverter(resolvedDataType)
-        .asInstanceOf[DataStructureConverter[RowData, Row]]
-      converter.toInternalOrNull(testData)
-    }
+    try {
+      val ctx = CodeGeneratorContext(config)
+      val inputType = if (containsLegacyTypes) {
+        fromTypeInfoToLogicalType(typeInfo)
+      } else {
+        resolvedDataType.getLogicalType
+      }
+      val exprGenerator = new ExprCodeGenerator(ctx, nullableInput = 
false).bindInput(inputType)
+
+      // cast expressions to String
+      val stringTestExprs = testExprs.map(expr => relBuilder.cast(expr._2, 
VARCHAR))
+
+      // generate code
+      val resultType = RowType.of(Seq.fill(testExprs.size)(
+        new VarCharType(VarCharType.MAX_LENGTH)): _*)
+
+      val exprs = stringTestExprs.map(exprGenerator.generateExpression)
+      val genExpr = exprGenerator
+        .generateResultExpression(exprs, resultType, classOf[BinaryRowData])
+
+      val bodyCode =
+        s"""
+           |${genExpr.code}
+           |return ${genExpr.resultTerm};
+          """.stripMargin
+
+      val genFunc = 
FunctionCodeGenerator.generateFunction[MapFunction[RowData, BinaryRowData]](
+        ctx,
+        "TestFunction",
+        classOf[MapFunction[RowData, BinaryRowData]],
+        bodyCode,
+        resultType,
+        inputType)
+
+      val mapper = genFunc.newInstance(getClass.getClassLoader)
+
+      val isRichFunction = mapper.isInstanceOf[RichFunction]
+
+      // call setRuntimeContext method and open method for RichFunction
+      if (isRichFunction) {
+        val richMapper = mapper.asInstanceOf[RichMapFunction[_, _]]
+        val t = new RuntimeUDFContext(
+          new TaskInfo("ExpressionTest", 1, 0, 1, 1),
+          classOf[ExpressionTestBase].getClassLoader,
+          env.getConfig,
+          Collections.emptyMap(),
+          Collections.emptyMap(),
+          null)
+        richMapper.setRuntimeContext(t)
+        richMapper.open(new Configuration())
+      }
 
-    val result = mapper.map(testRow)
+      val testRow = if (containsLegacyTypes) {
+        val converter = DataFormatConverters
+          .getConverterForDataType(resolvedDataType)
+          .asInstanceOf[DataFormatConverter[RowData, Row]]
+        converter.toInternal(testData)
+      } else {
+        val converter = DataStructureConverters
+          .getConverter(resolvedDataType)
+          .asInstanceOf[DataStructureConverter[RowData, Row]]
+        converter.toInternalOrNull(testData)
+      }
 
-    // call close method for RichFunction
-    if (isRichFunction) {
-      mapper.asInstanceOf[RichMapFunction[_, _]].close()
-    }
+      val result = mapper.map(testRow)
 
-    // compare
-    testExprs
-      .zipWithIndex
-      .foreach {
-        case ((originalExpr, optimizedExpr, expected), index) =>
-
-          // adapt string result
-          val actual = if(!result.asInstanceOf[BinaryRowData].isNullAt(index)) 
{
-            result.asInstanceOf[BinaryRowData].getString(index).toString
-          } else {
-            null
-          }
-
-          val original = if (originalExpr == null) "" else s"for: 
[$originalExpr]"
-
-          assertEquals(
-            s"Wrong result $original optimized to: [$optimizedExpr]",
-            expected,
-            if (actual == null) "null" else actual)
+      // call close method for RichFunction
+      if (isRichFunction) {
+        mapper.asInstanceOf[RichMapFunction[_, _]].close()
       }
+
+      // compare
+      testExprs
+        .zipWithIndex
+        .foreach {
+          case ((originalExpr, optimizedExpr, expected), index) =>
+
+            // adapt string result
+            val actual = 
if(!result.asInstanceOf[BinaryRowData].isNullAt(index)) {
+              result.asInstanceOf[BinaryRowData].getString(index).toString
+            } else {
+              null
+            }
+
+            val original = if (originalExpr == null) "" else s"for: 
[$originalExpr]"
+
+            assertEquals(
+              s"Wrong result $original optimized to: [$optimizedExpr]",
+              expected,
+              if (actual == null) "null" else actual)
+        }
+    } finally {
+      testExprs.clear()

Review comment:
       I think we need to find another way to test exception. 
   `testExpectExceptionThrown` is breaking the testing loop of this test base, 
which is triggered by `@Before` and `@After`. If we write a test case mixing 
with `testExpectExceptionThrown` and some normal tests, I suspect there will be 
some weird stuff. 
   One choice is you collect all tests which expected with exception into 
another array other than `testExprs`, and also verify this exception array in 
`@After`.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to