[Spark-22967][TESTS]Fix VersionSuite's unit tests by change Windows path into URI path #20199

Ngone51 · 2018-01-09T08:20:54Z

What changes were proposed in this pull request?

Two unit test will fail due to Windows format path:

1.test(s"$version: read avro file containing decimal")

org.apache.hadoop.hive.ql.metadata.HiveException: MetaException(message:java.lang.IllegalArgumentException: Can not create a Path from an empty string);

2.test(s"$version: SPARK-17920: Insert into/overwrite avro table")

Unable to infer the schema. The schema specification is required to create the table `default`.`tab2`.;
org.apache.spark.sql.AnalysisException: Unable to infer the schema. The schema specification is required to create the table `default`.`tab2`.;

This pr fix these two unit test by change Windows path into URI path.

How was this patch tested?

Existed.

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

Ngone51 · 2018-01-09T08:38:03Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala

@@ -58,7 +58,7 @@ class VersionsSuite extends SparkFunSuite with Logging {
   */
  protected def withTempDir(f: File => Unit): Unit = {
    val dir = Utils.createTempDir().getCanonicalFile
-    try f(dir) finally Utils.deleteRecursively(dir)
+    f(dir)


Leave deletion work to ShutdownHookManager to avoid delete IOException caused by 'file occupation in other program' error on Windows. (SEE SPARK-22967)
And temp dirs will be cleaned up after unit test completed, but this is only guaranteed for test(s"$version: SPARK-17920: Insert into/overwrite avro table"). And a lot of temp dirs produced by some other unit tests will still remains on Windows for unclear reason, maybe 'file occupation in other program ' too.

Ngone51 · 2018-01-09T08:38:37Z

cc @HyukjinKwon

HyukjinKwon · 2018-01-09T08:51:13Z

ok to test

HyukjinKwon · 2018-01-09T08:51:23Z

Will take a look soon.

SparkQA · 2018-01-09T11:05:32Z

Test build #85847 has finished for PR 20199 at commit 22669d1.

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

HyukjinKwon · 2018-01-09T12:41:43Z

Let's fix the PR title to [SPARK-22967][TESTS] ....

HyukjinKwon · 2018-01-09T12:44:21Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala

@@ -58,7 +58,7 @@ class VersionsSuite extends SparkFunSuite with Logging {
   */
  protected def withTempDir(f: File => Unit): Unit = {
    val dir = Utils.createTempDir().getCanonicalFile
-    try f(dir) finally Utils.deleteRecursively(dir)
+    f(dir)


Hm, actually, shall we just skip the failing tests for now (assume(!Utils.isWindows))? Given what we talked, only few tests are failed in some few conditions when Hive version is 0.12?

Actually, only one(2nd test mentioned above). Probably, assume(!(Utils.isWindows && version == "0.12")) would be ok. WDYT?

Yea, sounds good.

HyukjinKwon · 2018-01-09T12:51:23Z

I met the similar case when I fixed some tests related with Kafka before - https://issues-test.apache.org/jira/browse/KAFKA-1194. This took me a while to debug and find this issue ..

To me, I am fine if we fail to find the issue that causes the failure in Hive but might be worth to give a try to find the issue causing this failure and leave the link here.

HyukjinKwon · 2018-01-09T12:53:42Z

I can trigger a test on Windows. Let me leave the build:

Build started: [SQL] org.apache.spark.sql.hive.client.VersionsSuite
Diff: master...spark-test:68230874-FD64-41B8-82FD-527386DEDE69

Ngone51 · 2018-01-09T13:34:36Z

Ok, will try.

HyukjinKwon · 2018-01-10T01:49:58Z

Build started: [SQL] org.apache.spark.sql.hive.client.VersionsSuite
Diff: master...spark-test:A43BBFC9-F966-40FD-BA66-98D175F432E2

SparkQA · 2018-01-10T04:09:16Z

Test build #85894 has finished for PR 20199 at commit 04b6628.

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

HyukjinKwon · 2018-01-10T13:51:40Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala

@@ -842,6 +842,7 @@ class VersionsSuite extends SparkFunSuite with Logging {
    }

    test(s"$version: SPARK-17920: Insert into/overwrite avro table") {
+      assume(!(Utils.isWindows && version == "0.12"))


@Ngone51 Have you had a chance to check out the issue? If you can't find, let's leave a comment here saying like ... it's intendedly skipped because it fails on Windows.

@HyukjinKwon Ok, will comment.

And I have find serval issues which exactly describe the same problem:

https://issues.apache.org/jira/browse/SPARK-12216

https://issues.apache.org/jira/browse/SPARK-8333

https://issues.apache.org/jira/browse/SPARK-18979

It seems this is a common issue exists on Windows and haven't been resolved yet. And I didn't find a accurate cause for the error from those issues. And now, I doubt this problem maybe related to URLClassLoader. And, I have reproduce the same issue after I did a experiment with URLClassLoader. Though, I'm not sure this is the real cause, yet. Working on this.

We can merge this pr to master after I add comment. And open a new pr of that issue(if fixed) or communicate under one of those issues. WDYT?

Nice. Yea, let's merge this one after adding a comment saying it's skipped because it's failed in the condition on Windows.

Then, see if we can fix the root cause, and then re-enable this test in the PR if everything goes well.

HyukjinKwon

LGTM

SparkQA · 2018-01-11T12:40:54Z

Test build #85960 has finished for PR 20199 at commit c3d93c3.

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

HyukjinKwon · 2018-01-11T13:17:49Z

Merged to master and branch-2.3.

…path into URI path ## What changes were proposed in this pull request? Two unit test will fail due to Windows format path: 1.test(s"$version: read avro file containing decimal") ``` org.apache.hadoop.hive.ql.metadata.HiveException: MetaException(message:java.lang.IllegalArgumentException: Can not create a Path from an empty string); ``` 2.test(s"$version: SPARK-17920: Insert into/overwrite avro table") ``` Unable to infer the schema. The schema specification is required to create the table `default`.`tab2`.; org.apache.spark.sql.AnalysisException: Unable to infer the schema. The schema specification is required to create the table `default`.`tab2`.; ``` This pr fix these two unit test by change Windows path into URI path. ## How was this patch tested? Existed. Please review http://spark.apache.org/contributing.html before opening a pull request. Author: wuyi5 <ngone_5451@163.com> Closes #20199 from Ngone51/SPARK-22967. (cherry picked from commit 0552c36) Signed-off-by: hyukjinkwon <gurwls223@gmail.com>

Ngone51 added 2 commits January 9, 2018 16:01

change Windows path into URI format path

3d1cafa

leave deletion work to ShutdownHookManager

22669d1

Ngone51 commented Jan 9, 2018

View reviewed changes

HyukjinKwon reviewed Jan 9, 2018

View reviewed changes

Ngone51 changed the title ~~[Spark-22967][Hive]Fix VersionSuite's unit tests by change Windows path into URI path~~ [Spark-22967][TESTS]Fix VersionSuite's unit tests by change Windows path into URI path Jan 9, 2018

skip test(0.12: 'SPARK-17920: Insert into/overwrite avro table')

04b6628

HyukjinKwon reviewed Jan 10, 2018

View reviewed changes

add comment

c3d93c3

HyukjinKwon approved these changes Jan 11, 2018

View reviewed changes

asfgit closed this in 0552c36 Jan 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spark-22967][TESTS]Fix VersionSuite's unit tests by change Windows path into URI path #20199

[Spark-22967][TESTS]Fix VersionSuite's unit tests by change Windows path into URI path #20199

Ngone51 commented Jan 9, 2018

Ngone51 Jan 9, 2018

Ngone51 commented Jan 9, 2018

HyukjinKwon commented Jan 9, 2018

HyukjinKwon commented Jan 9, 2018

SparkQA commented Jan 9, 2018

HyukjinKwon commented Jan 9, 2018 •

edited

HyukjinKwon Jan 9, 2018 •

edited

Ngone51 Jan 9, 2018

HyukjinKwon Jan 9, 2018

HyukjinKwon commented Jan 9, 2018

HyukjinKwon commented Jan 9, 2018

Ngone51 commented Jan 9, 2018

HyukjinKwon commented Jan 10, 2018

SparkQA commented Jan 10, 2018

HyukjinKwon Jan 10, 2018

Ngone51 Jan 10, 2018

HyukjinKwon Jan 11, 2018

HyukjinKwon left a comment

SparkQA commented Jan 11, 2018

HyukjinKwon commented Jan 11, 2018

[Spark-22967][TESTS]Fix VersionSuite's unit tests by change Windows path into URI path #20199

[Spark-22967][TESTS]Fix VersionSuite's unit tests by change Windows path into URI path #20199

Conversation

Ngone51 commented Jan 9, 2018

What changes were proposed in this pull request?

How was this patch tested?

Ngone51 Jan 9, 2018

Choose a reason for hiding this comment

Ngone51 commented Jan 9, 2018

HyukjinKwon commented Jan 9, 2018

HyukjinKwon commented Jan 9, 2018

SparkQA commented Jan 9, 2018

HyukjinKwon commented Jan 9, 2018 • edited

HyukjinKwon Jan 9, 2018 • edited

Choose a reason for hiding this comment

Ngone51 Jan 9, 2018

Choose a reason for hiding this comment

HyukjinKwon Jan 9, 2018

Choose a reason for hiding this comment

HyukjinKwon commented Jan 9, 2018

HyukjinKwon commented Jan 9, 2018

Ngone51 commented Jan 9, 2018

HyukjinKwon commented Jan 10, 2018

SparkQA commented Jan 10, 2018

HyukjinKwon Jan 10, 2018

Choose a reason for hiding this comment

Ngone51 Jan 10, 2018

Choose a reason for hiding this comment

HyukjinKwon Jan 11, 2018

Choose a reason for hiding this comment

HyukjinKwon left a comment

Choose a reason for hiding this comment

SparkQA commented Jan 11, 2018

HyukjinKwon commented Jan 11, 2018

HyukjinKwon commented Jan 9, 2018 •

edited

HyukjinKwon Jan 9, 2018 •

edited