Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Dec 25, 2020

What changes were proposed in this pull request?

Add comments for the unified datasource tests, describe what kind of tests they contain, and put refs to other test suits.

Why are the changes needed?

To improve code maintenance.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By running ./dev/scalastyle.

@SparkQA
Copy link

SparkQA commented Dec 25, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37975/

@SparkQA
Copy link

SparkQA commented Dec 25, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37975/

@SparkQA
Copy link

SparkQA commented Dec 25, 2020

Test build #133385 has finished for PR 30929 at commit 0a8a0b8.

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

@SparkQA
Copy link

SparkQA commented Dec 26, 2020

Test build #133394 has finished for PR 30929 at commit 0b557b2.

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

@MaxGekk
Copy link
Member Author

MaxGekk commented Dec 26, 2020

Not clear how my changes are related to the error:

[error] /home/runner/work/spark/spark/mllib/target/java/org/apache/spark/mllib/util/MLlibTestSparkContext.java:10:1:  error: illegal combination of modifiers: public and protected
[error]   protected  class testImplicits {
[error]              ^

@SparkQA
Copy link

SparkQA commented Dec 26, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37988/

@SparkQA
Copy link

SparkQA commented Dec 26, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37989/

@SparkQA
Copy link

SparkQA commented Dec 26, 2020

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37989/

@SparkQA
Copy link

SparkQA commented Dec 26, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37988/

@SparkQA
Copy link

SparkQA commented Dec 26, 2020

Test build #133397 has finished for PR 30929 at commit af4d508.

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

@SparkQA
Copy link

SparkQA commented Dec 26, 2020

Test build #133398 has finished for PR 30929 at commit ac74991.

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

* - V2 table catalog tests:
* [[org.apache.spark.sql.execution.command.v2.AlterTableDropPartitionSuite]]
* - V1 table catalog tests:
* [[org.apache.spark.sql.execution.command.v1.AlterTableDropPartitionSuiteBase]]
Copy link
Member

Choose a reason for hiding this comment

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

org.apache.spark.execution.command.AlterTableDropPartitionSuiteBase ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I think the ref is correct, see

trait AlterTableDropPartitionSuiteBase extends command.AlterTableDropPartitionSuiteBase {

Copy link
Member

Choose a reason for hiding this comment

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

My bad~

* - V2 table catalog tests:
* [[org.apache.spark.sql.execution.command.v2.AlterTableAddPartitionSuite]]
* - V1 table catalog tests:
* [[org.apache.spark.sql.execution.command.v1.AlterTableAddPartitionSuiteBase]]
Copy link
Member

Choose a reason for hiding this comment

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

org.apache.spark.sql.execution.command.AlterTableAddPartitionSuiteBase?

Copy link
Member Author

Choose a reason for hiding this comment

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

You pointed out the current trait which contains tests for v1 and v2. org.apache.spark.sql.execution.command.v1.AlterTableAddPartitionSuiteBase is the base trait for v1 In-Memory and Hive catalogs.

@HyukjinKwon
Copy link
Member

+1 I like this comments!

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 4a61fc1 Dec 28, 2020
@dongjoon-hyun
Copy link
Member

+1, late LGTM. Thank you.

@MaxGekk MaxGekk deleted the doc-unified-tests branch February 19, 2021 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants