Skip to content
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

test: Add tests for Scalar and Inverval values for UnaryMinus #538

Merged
merged 10 commits into from
Jun 11, 2024

Conversation

vaibhawvipul
Copy link
Contributor

Which issue does this PR close?

Closes #508 .

Rationale for this change

Improves test coverage for various unary minus.

What changes are included in this PR?

Tests for scalar values and intervals.

How are these changes tested?

All tests pass.

@vaibhawvipul vaibhawvipul marked this pull request as ready for review June 7, 2024 07:11
Copy link
Contributor

@kazuyukitanimura kazuyukitanimura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good.
Somehow Github failed launching CI...

@kazuyukitanimura kazuyukitanimura changed the title bug: Add tests for Scalar and Inverval values for UnaryMinus test: Add tests for Scalar and Inverval values for UnaryMinus Jun 7, 2024
@kazuyukitanimura
Copy link
Contributor

pending ci

@andygrove
Copy link
Member

Thanks @vaibhawvipul and @kazuyukitanimura

@@ -1635,7 +1637,7 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper {
case (Some(sparkException), Some(cometException)) =>
assert(sparkException.getMessage.contains(dtype + " overflow"))
assert(cometException.getMessage.contains(dtype + " overflow"))
case (None, None) => assert(true) // got same outputs
case (None, None) => checkSparkAnswerAndOperator(sql(query))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we call checkSparkMaybeThrows,, and if that doesn't fail, then we call checkSparkAnswerAndOperator. I think this could all be streamlined, but this is beyond the scope of this PR

@andygrove andygrove merged commit a4e268c into apache:main Jun 11, 2024
43 checks passed
himadripal pushed a commit to himadripal/datafusion-comet that referenced this pull request Sep 7, 2024
…#538)

* adding scalar tests

* refactor and test for interval

* ci checks fixed

* running operator checks when no error

* removing redundant sqlconf

* fix ci errorsg

* moving interval test to array section

* ci fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for Scalar and Inverval values for UnaryMinus
3 participants