kgyrtkirk commented on code in PR #15086:
URL: https://github.com/apache/druid/pull/15086#discussion_r1347892185


##########
sql/src/test/java/org/apache/druid/sql/calcite/QueryVerification.java:
##########
@@ -47,7 +47,12 @@ public QueryTestRunner.QueryVerifyStep 
make(QueryTestRunner.BaseExecuteQuery exe
     {
       return () -> {
         for (QueryTestRunner.QueryResults queryResults : execStep.results()) {
-          verifier.verifyResults(queryResults);
+          try {
+            verifier.verifyResults(queryResults);
+          }
+          catch (Exception e) {
+            throw new RuntimeException("Exception during verification!", e);

Review Comment:
   I was looking around what else to use - but since this is part of the test 
framework and `DruidException` wants "Persona" and "Category" which is clearly 
unrelated as an `Exception` here means that the test most likely have crashed...
   ```
               throw DruidException.forPersona(Persona.DEVELOPER)
                   .ofCategory(Category.UNCATEGORIZED)
                   .build(e, "Exception during verification!");
   ```
   I could probably use  `Assert.fail`  here - but the junit4 `Assert.fail` 
does not accept an `Exception` so it would not retain the stacktrace... junit5 
has a method like that - but we don't yet have that here (...and since the test 
most likely crashed; and not failed using that would kinda miss its usecase)
   
   Letting out the `Exception` at this point kinda leads to a shotgun surgery  
(although I do thing that every test method should be allowed to just let 
checked `Exception`-s out)
   
   I was looking around what other exceptions are out there...but didn't found 
a better match than retaining to wrap the checked `Exception` into a plain 
`RuntimeException` ; to let it out.
   
   



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