[SQL] Use the nicer formatting function for test outputs where possible#6514
Conversation
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
mythical-fred
left a comment
There was a problem hiding this comment.
LGTM — mechanical revert of the earlier qs(...) conversions back to qst(...) now that qst handles trailing-whitespace correctly. Diff is overwhelmingly qs -> qst renames (167 removed / 171 added) plus the expected-output reformatting that goes with it (column padding, (N rows) footers re-added, no \s whitespace escapes). CI green; nothing else moves.
One observation on PostgresDateTests: the EPOCH/CENTURY*/MILLENNIUM* block goes from 13 individual @Test methods to a single testExtract1 containing all of them as one multi-statement qst. Functional coverage is preserved, but if one EXTRACT case regresses the surefire report will only point at testExtract1 rather than at e.g. testCentury3 — the failing SQL is still in the diff context, so it is mild, but it is a small loss of granularity. Not a blocker; flagging in case you want to keep finer-grained names elsewhere when consolidating.
Also noticed a couple of leftover // CURRENT_DATE not supported comments got dropped along with the methods they annotated — fine since the comment was only meaningful next to the removed test, just noting it so it is intentional.
This is a purely cosmetic rewrite; I added recently a function which is more suitable for tests where strings never have trailing whitespaces. Previously I had to convert many tests, this reverts the conversions I had done and keeps the expected test outputs, as produced by Calcite/Postgres/etc.