yashmayya commented on code in PR #13664:
URL: https://github.com/apache/pinot/pull/13664#discussion_r1684468878


##########
pinot-query-planner/src/test/java/org/apache/pinot/query/QueryCompilationTest.java:
##########
@@ -385,6 +385,12 @@ public void testGetTableNamesForQuery() {
     assertEquals(tableNames.get(0), "a");
   }
 
+  @Test(expectedExceptions = RuntimeException.class)

Review Comment:
   I'd initially tried that with `expectedExceptionsMessageRegExp` but looks 
like that only checks the top level exception message (and not nested 
exceptions) which in our case is the `RuntimeException` with `Error composing 
query plan for:...` from 
[here](https://github.com/apache/pinot/blob/205a75debbdd2c161f6f830a71e467605fac6ec4/pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java#L140).
 ~I'm not sure why we use TestNG and not JUnit which is a lot more feature rich 
🤷~ Edit: Never mind, looks like TestNG also supports the `assertThrows` / 
`expectThrows` style.
   
   I updated the test to use a more old school style of catching the expected 
exception and making assertions in the catch block.



-- 
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: commits-unsubscr...@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to