-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable resultset validation of Drill tests #15096
Conversation
This reverts commit baddb89.
List<Object[]> expectedResults = parseResults(currentRowSignature, expectedResultsText); | ||
try { | ||
Assert.assertEquals(StringUtils.format("result count: %s", sql), expectedResultsText.size(), results.size()); | ||
if (!isOrdered(sql)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be flipped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the logic here is kinda like: if the resultset is not ordered - lets order both of them by the same comparator and expect the same (ordered) resultsets after that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok. Because there is no guarantee about what order the results will arrive in. can you please add a comment here in the code itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be done in a follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed a few more things - and made SqlToRelConverter#isOrdered
do the decision here - but these changes become dependendt on #15086 ; I can add it after that
sql/src/test/java/org/apache/druid/sql/calcite/DrillWindowQueryTest.java
Show resolved
Hide resolved
- introduces a test_X method for every testcase (995 testcases) - added a resultset parser which reads the expected resultset based on the result schema - loaded a few more datasets - added a testcase to ensure that all files have a corresponding testcase - renamed DecoupledIgnore to NegativeTest - categorized the failing 268 tests
- introduces a test_X method for every testcase (995 testcases) - added a resultset parser which reads the expected resultset based on the result schema - loaded a few more datasets - added a testcase to ensure that all files have a corresponding testcase - renamed DecoupledIgnore to NegativeTest - categorized the failing 268 tests
- introduces a test_X method for every testcase (995 testcases) - added a resultset parser which reads the expected resultset based on the result schema - loaded a few more datasets - added a testcase to ensure that all files have a corresponding testcase - renamed DecoupledIgnore to NegativeTest - categorized the failing 268 tests
- introduces a test_X method for every testcase (995 testcases) - added a resultset parser which reads the expected resultset based on the result schema - loaded a few more datasets - added a testcase to ensure that all files have a corresponding testcase - renamed DecoupledIgnore to NegativeTest - categorized the failing 268 tests
- introduces a test_X method for every testcase (995 testcases) - added a resultset parser which reads the expected resultset based on the result schema - loaded a few more datasets - added a testcase to ensure that all files have a corresponding testcase - renamed DecoupledIgnore to NegativeTest - categorized the failing 268 tests
test_X
method for every testcase (995 testcases)DecoupledIgnore
toNegativeTest
current histogram of failures:
This PR has: