-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
move array function unit_tests to sqllogictest #8332
Conversation
cc @jayzhan211 @alamb PTAL |
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.
Thank you very much @Veeupup
Let me know if this looks good to you @jayzhan211 |
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.
Sure! But I found that |
I agree this is a good plan (to keep |
Signed-off-by: veeupup <code@tanweime.com>
8d983fa
to
8f84321
Compare
OK, we can keep it. I am just afraid that people will create test in array_expression.rs if there is still rust test (any kind of test) inside it. |
Maybe we should leave some comments for it 🤔 |
I think comments help. |
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.
Thank you @Veeupup @jayzhan211 and @Weijun-H
Which issue does this PR close?
Closes #8324 .
I have checked the unit tests in
array_expressions.rs
and found that almost all test cases are covered bysqllogictest
.So this PR is almost about removing the duplicate unit tests
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?