[GitHub] [druid] kfaraz commented on pull request #13106: Query test case definitions

[GitBox <gi...@apache.org>]

kfaraz commented on PR #13106:
URL: https://github.com/apache/druid/pull/13106#issuecomment-1260415405

   Haven't reviewed the PR thoroughly yet but it seems pretty useful and would make test cases much more readable.
   
   A couple of first glance comments:
   - I suppose the only use of `TestCaseWriter` would be to convert existing tests as new tests would always be written in the new format. I suppose you would call this writer from somewhere inside `BaseCalciteQueryTest.testQuery()`, run all the tests and print out all the test cases in one go. Please correct me if I am missing something.
   - I would advise using yaml rather than having to define a whole new syntax. Yaml is meant to tackle such requirements, you can easily have multi-line strings, even embed json data, without compromising on the format or having to unnecessarily quote things.
   - I would also advise removing the suffix `...Section` from different objects that make up a test case. I feel that the decision to write things in a "section" or otherwise is really an impl detail. It would be cognitively simpler if a test case just specifies `Resources`, `Context`, `ExpectedResults` etc., being completely agnostic of the underlying file format.
   


-- 
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-unsubscribe@druid.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org