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

[SPARK-38036][SQL][TESTS] Refactor VersionsSuite to HiveClientSuite and make it a subclass of HiveVersionSuite #35335

Closed
wants to merge 5 commits into from

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Jan 26, 2022

What changes were proposed in this pull request?

There is a TODO in VersionsSuite:

  • TODO: Refactor this to HiveClientSuite and make it a subclass of HiveVersionSuite

this pr completed this TODO, the main change as follows:

  • copy all test cases in versions.foreach scope of VersionsSuite to HiveClientSuite
  • override nestedSuites function in HiveClientSuites to use each hive version to test the cases in HiveClientSuite similar as HiveClientUserNameSuites and HivePartitionFilteringSuites
  • move other cases to HiveClientSuites

Why are the changes needed?

Make VersionsSuite as a subclass of HiveVersionSuite to unify the test mode of multi version hive

Does this PR introduce any user-facing change?

No.

How was this patch tested?

  • Pass GA
  • Manual test:

Before

mvn clean install -DskipTests -pl sql/hive -am 
mvn test -pl sql/hive -Dtest=none -DwildcardSuites=org.apache.spark.sql.hive.client.HiveClientSuites

Run completed in 13 minutes, 10 seconds.
Total number of tests run: 867
Suites: completed 2, aborted 0
Tests: succeeded 867, failed 0, canceled 0, ignored 1, pending 0
All tests passed.

After

mvn clean install -DskipTests -pl sql/hive -am 
mvn test -pl sql/hive -Dtest=none -DwildcardSuites=org.apache.spark.sql.hive.client.HiveClientSuites

Run completed in 3 minutes, 8 seconds.
Total number of tests run: 867
Suites: completed 14, aborted 0
Tests: succeeded 867, failed 0, canceled 0, ignored 1, pending 0
All tests passed

The number of test cases is the same, and Suites changed from 2 to 14

@github-actions github-actions bot added the SQL label Jan 26, 2022
@LuciferYang LuciferYang changed the title [SPARK-38036][TESTS] Refactor VersionsSuite to HiveClientSuite and make it a subclass of HiveVersionSuite [SPARK-38036][SQL][TESTS] Refactor VersionsSuite to HiveClientSuite and make it a subclass of HiveVersionSuite Jan 26, 2022
private var versionSpark: TestHiveVersion = null

versions.foreach { version =>
test(s"$version: create client") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test cases in versions.foreach have been moved to HiveClientSuite.scala

// Since Hive 3.0, HIVE-19310 skipped `ensureDbInit` if `hive.in.test=false`.
hadoopConf.set("hive.in.test", "true")
// Since HIVE-17626(Hive 3.0.0), need to set hive.query.reexecution.enabled=false.
hadoopConf.set("hive.query.reexecution.enabled", "false")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

hadoopConf.set("hive.query.reexecution.enabled", "false") move to HiveVersionSuite.scala as default configuration.

}
}

test("success sanity check") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

success sanity check, hadoop configuration preserved, override useless and side-effect hive configurations and failure sanity check move to HiveClientSuites

}
}

test("create client") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The $version: prefix is removed because the subclass of HiveVersionSuite will print this prefix by default, other cases in this file are similar

@LuciferYang
Copy link
Contributor Author

cc @cloud-fan @HyukjinKwon

Copy link
Contributor

@cloud-fan cloud-fan left a comment

Choose a reason for hiding this comment

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

thanks for finishing this TODO!

@cloud-fan
Copy link
Contributor

merging to master!

@cloud-fan cloud-fan closed this in e81f611 Feb 11, 2022
@LuciferYang
Copy link
Contributor Author

thanks @cloud-fan

@LuciferYang LuciferYang deleted the SPARK-38036 branch October 22, 2023 07:44
dongjoon-hyun added a commit that referenced this pull request Apr 1, 2024
### What changes were proposed in this pull request?

This PR aims to clean up the removed `VersionsSuite` reference.

### Why are the changes needed?

At Apache Spark 3.3.0, `VersionsSuite` is removed via SPARK-38036 .
- #35335

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #45800 from dongjoon-hyun/SPARK-47676.

Authored-by: Dongjoon Hyun <dhyun@apple.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
dongjoon-hyun added a commit that referenced this pull request Apr 1, 2024
### What changes were proposed in this pull request?

This PR aims to clean up the removed `VersionsSuite` reference.

### Why are the changes needed?

At Apache Spark 3.3.0, `VersionsSuite` is removed via SPARK-38036 .
- #35335

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #45800 from dongjoon-hyun/SPARK-47676.

Authored-by: Dongjoon Hyun <dhyun@apple.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
(cherry picked from commit 128f74b)
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
dongjoon-hyun added a commit that referenced this pull request Apr 1, 2024
### What changes were proposed in this pull request?

This PR aims to clean up the removed `VersionsSuite` reference.

### Why are the changes needed?

At Apache Spark 3.3.0, `VersionsSuite` is removed via SPARK-38036 .
- #35335

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #45800 from dongjoon-hyun/SPARK-47676.

Authored-by: Dongjoon Hyun <dhyun@apple.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
(cherry picked from commit 128f74b)
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants