Skip to content

Conversation

@10110346
Copy link
Contributor

@10110346 10110346 commented May 24, 2017

What changes were proposed in this pull request?

add test case to MathExpressionsSuite as #17906

How was this patch tested?

unit test cases

@10110346 10110346 changed the title [SPARK-20665][SQL][FOLLOW-UP]move test case to SQLQueryTestSuite [SPARK-20665][SQL][FOLLOW-UP]Move test case to SQLQueryTestSuite May 24, 2017
Copy link
Member

Choose a reason for hiding this comment

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

add a new line here

Copy link
Member

Choose a reason for hiding this comment

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

To -- cot

@10110346 10110346 force-pushed the wip-lx-0524 branch 2 times, most recently from 542430b to ec0fa63 Compare May 24, 2017 03:17
@10110346
Copy link
Contributor Author

Done. Should i delete the unit test case from MathFunctionsSuite.scala?
@gatorsmile

@rxin
Copy link
Contributor

rxin commented May 24, 2017

Hm I'm not sure if it is a good idea to run so many "unit test" style tests for expressions in the end to end suites. It takes a lot of time than just running unit tests.

@gatorsmile
Copy link
Member

@10110346 Could you move them to MathExpressionsSuite, based on @rxin 's comments? Thanks!

@10110346
Copy link
Contributor Author

@gatorsmile OK,i will do,thanks

@10110346 10110346 changed the title [SPARK-20665][SQL][FOLLOW-UP]Move test case to SQLQueryTestSuite [SPARK-20665][SQL][FOLLOW-UP]Move test case to MathExpressionsSuite May 26, 2017
@10110346 10110346 closed this May 27, 2017
@10110346 10110346 reopened this May 28, 2017
@10110346 10110346 force-pushed the wip-lx-0524 branch 3 times, most recently from ec8fc33 to 5b747ef Compare June 1, 2017 08:53
@10110346
Copy link
Contributor Author

10110346 commented Jun 1, 2017

@gatorsmile Please review it again,thanks

Copy link
Member

Choose a reason for hiding this comment

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

revert this back

Copy link
Member

Choose a reason for hiding this comment

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

indent issues.

@gatorsmile
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Jun 1, 2017

Test build #77647 has finished for PR 18082 at commit 5b747ef.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 2, 2017

Test build #77663 has finished for PR 18082 at commit d9d245a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@10110346
Copy link
Contributor Author

10110346 commented Jun 5, 2017

Test passed, thanks. @gatorsmile

@SparkQA
Copy link

SparkQA commented Jun 9, 2017

Test build #77834 has finished for PR 18082 at commit 0ca63be.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

Choose a reason for hiding this comment

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

After rethinking it, maybe we should keep this file untouched in this PR. The reason is we still can check the type promotion logics here.

Yeah, we still should add the test cases in MathExpressionsSuite.scala

@10110346
Copy link
Contributor Author

@gatorsmile ok,i will modify and resubmit

@SparkQA
Copy link

SparkQA commented Jun 12, 2017

Test build #77904 has finished for PR 18082 at commit 4e20839.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile
Copy link
Member

Thanks! Merging to master.

@asfgit asfgit closed this in d140918 Jun 12, 2017
dataknocker pushed a commit to dataknocker/spark that referenced this pull request Jun 16, 2017
## What changes were proposed in this pull request?

 add test case to MathExpressionsSuite as apache#17906

## How was this patch tested?

unit test cases

Author: liuxian <[email protected]>

Closes apache#18082 from 10110346/wip-lx-0524.
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.

4 participants