Skip to content

Conversation

@NiharS
Copy link
Contributor

@NiharS NiharS commented Jul 13, 2018

What changes were proposed in this pull request?

While looking through the codebase, it appeared that the scala code for RDD.cartesian does not have any tests for correctness. This adds a couple basic tests to verify cartesian yields correct values. While the implementation for RDD.cartesian is pretty simple, it always helps to have a few tests!

How was this patch tested?

The new test cases pass, and the scala style tests from running dev/run-tests all pass.

Please review http://spark.apache.org/contributing.html before opening a pull request.

@SparkQA
Copy link

SparkQA commented Jul 15, 2018

Test build #4213 has finished for PR 21765 at commit 9df4c3b.

  • This patch fails Spark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 16, 2018

Test build #4215 has finished for PR 21765 at commit 9df4c3b.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 16, 2018

Test build #4216 has finished for PR 21765 at commit 9df4c3b.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

The scala code for RDD.cartesian does not have any tests for correctness. This adds a couple basic tests to verify cartesian yields correct values.

Passes the added test cases, and passes the scala style tests.

Author: Nihar Sheth <[email protected]>
@NiharS
Copy link
Contributor Author

NiharS commented Jul 17, 2018

Rebased onto SPARK-24813 so hopefully tests will work now

@srowen
Copy link
Member

srowen commented Jul 17, 2018

Yup it will always apply your PR to master anyway, so would have picked it up in this test run.

@SparkQA
Copy link

SparkQA commented Jul 17, 2018

Test build #4217 has finished for PR 21765 at commit 9df4c3b.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 18, 2018

Test build #4218 has finished for PR 21765 at commit e5f469a.

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

Copy link
Contributor

@jiangxb1987 jiangxb1987 left a comment

Choose a reason for hiding this comment

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

LGTM

@srowen
Copy link
Member

srowen commented Jul 18, 2018

Merged to master

@asfgit asfgit closed this in 2694dd2 Jul 18, 2018
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