[jira] [Created] (FLINK-11150) Check exception messages in ValidationTest of flink-table

["Hequn Cheng (JIRA)" <ji...@apache.org>]

Hequn Cheng created FLINK-11150:
-----------------------------------

             Summary: Check exception messages in ValidationTest of flink-table
                 Key: FLINK-11150
                 URL: https://issues.apache.org/jira/browse/FLINK-11150
             Project: Flink
          Issue Type: Improvement
          Components: Table API & SQL
            Reporter: Hequn Cheng
            Assignee: Hequn Cheng


****Problem****

Currently, there are a lot of {{ValidationTests}} in flink-table. These tests are used to test whether exceptions are thrown correctly. However, the detailed messages of the exception have not been checked which makes the tests very fragile. Take the following test as an example:
{code:java}
class TableSinkValidationTest extends TableTestBase {

  @Test(expected = classOf[TableException])
  def testAppendSinkOnUpdatingTable(): Unit = {
    val env = StreamExecutionEnvironment.getExecutionEnvironment
    val tEnv = TableEnvironment.getTableEnvironment(env)

    val t = StreamTestData.get3TupleDataStream(env).toTable(tEnv, 'id, 'num, 'text)
    tEnv.registerTableSink("testSink", new TestAppendSink)

    t.groupBy('text)
    .select('text, 'id.count, 'num.sum)
    .insertInto("testSink")

    // must fail because table is not append-only
    env.execute()
  }
}
{code}
The test is used to check validation for AppendSink on updating table. The test will pass without any exceptions. If we remove the {{(expected = classOf[TableException])}}, we can see the following exception:
{code:java}
org.apache.flink.table.api.TableException: Table sink is not configured.
{code}
However, the correct exception should be:
{code:java}
org.apache.flink.table.api.TableException: AppendStreamTableSink requires that Table has only insert changes.
{code}
Since the two exceptions share the same exception class name, we also have to check the exception messages.

 

****Proposal****

To make our test more rigorous, I think it is better to use {{ExpectedException}} to check both the exception class and exception messages. So the previous test would be:
{code:java}
    @Test
  def testAppendSinkOnUpdatingTable(): Unit = {
    expectedException.expect(classOf[TableException])
    expectedException.expectMessage("AppendStreamTableSink requires that Table has only insert changes.")

    val env = StreamExecutionEnvironment.getExecutionEnvironment
    val tEnv = TableEnvironment.getTableEnvironment(env)

    val t = StreamTestData.get3TupleDataStream(env).toTable(tEnv, 'id, 'num, 'text)
    tEnv.registerTableSink("testSink", new TestAppendSink()
      .configure(Array("text", "id", "num"), Array(Types.STRING, Types.LONG, Types.LONG)))

    t.groupBy('text)
    .select('text, 'id.count, 'num.sum)
    .insertInto("testSink")

    // must fail because table is not append-only
    env.execute()
  }
{code}
which adds two more lines to the test:
{code:java}
    expectedException.expect(classOf[TableException])
    expectedException.expectMessage("AppendStreamTableSink requires that Table has only insert changes.")
{code}
 

Any suggestions are greatly appreciated!



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)