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 
🤷~
   
   ~I updated the test to use a more old school style of catching the expected 
exception and making assertions in the catch block.~
   
   Edit: Never mind, looks like TestNG also supports the `assertThrows` / 
`expectThrows` style.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to