You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/02/04 03:48:51 UTC
[GitHub] [spark] beliefer opened a new pull request #31466: [WIP][SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
beliefer opened a new pull request #31466:
URL: https://github.com/apache/spark/pull/31466
### What changes were proposed in this pull request?
The current implement of `SQLQueryTestSuite` cannot run on windows system.
Becasue the code below will fail on windows system:
`assume(TestUtils.testCommandAvailable("/bin/bash"))`
For operation system that cannot support `/bin/bash`, we just skip some tests.
### Why are the changes needed?
SQLQueryTestSuite has a bug on windows system.
### Does this PR introduce _any_ user-facing change?
'No'.
### How was this patch tested?
Jenkins test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570808552
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Yes. the origin code will skip the whole `SQLQueryTestSuite`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570699497
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
`assume` makes `SQLQueryTestSuite` exit
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570176457
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -260,9 +260,6 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
newLine.startsWith("--") && !newLine.startsWith("--QUERY-DELIMITER")
}
- // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
- // here we need to check command available
- assume(TestUtils.testCommandAvailable("/bin/bash"))
Review comment:
Does it fail or skip? I think it skips, which is fine?
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Yeah, can we call `assume` to trigger skipping the test instead of filtering out here?
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -260,9 +260,6 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
newLine.startsWith("--") && !newLine.startsWith("--QUERY-DELIMITER")
}
- // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
- // here we need to check command available
- assume(TestUtils.testCommandAvailable("/bin/bash"))
Review comment:
Can we conditionally skip here instead of filtering the tests out?
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Can you double check and investigate to make it working @beliefer? `assume` under `test` should work and skip only that single test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773146059
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134852/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775180765
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39608/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571528231
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
This skipping behavior not happen in `SparkScriptTransformationSuite` and `PipedRDDSuite`. Because `SparkScriptTransformationSuite` and `PipedRDDSuite` write each test case in single `SQLTestUtils.test`.
But `SQLQueryTestSuite` calls `TestUtils.testCommandAvailable("/bin/bash")` in common place.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571820552
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
I meant the fix such as:
```scala
assume(
!testCase.inputFile.endsWith("transform.sql") ||
TestUtils.testCommandAvailable("/bin/bash"))
```
I tested that it only skips transform.sql when `/bin/bash` is not available:
```
[info] - transform.sql !!! CANCELED !!! (36 milliseconds)
[info] "/.../spark/sql/core/src/test/resources/sql-tests/inputs/transform.sql" ended with "transform.sql", and org.apache.spark.TestUtils.testCommandAvailable("/bin/bas") was false (SQLQueryTestSuite.scala:265)
[info] org.scalatest.exceptions.TestCanceledException:
[info] at org.scalatest.Assertions.newTestCanceledException(Assertions.scala:475)
[info] at org.scalatest.Assertions.newTestCanceledException$(Assertions.scala:474)
[info] at org.scalatest.Assertions$.newTestCanceledException(Assertions.scala:1231)
[info] at org.scalatest.Assertions$AssertionsHelper.macroAssume(Assertions.scala:1310)
[info] at org.apache.spark.sql.SQLQueryTestSuite.runTest(SQLQueryTestSuite.scala:265)
[info] at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$createScalaTestCase$5(SQLQueryTestSuite.scala:247)
[info] at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
[info] at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
[info] at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
[info] at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
[info] at org.scalatest.Transformer.apply(Transformer.scala:22)
[info] at org.scalatest.Transformer.apply(Transformer.scala:20)
[info] at org.scalatest.funsuite.AnyFunSuiteLike$$anon$1.apply(AnyFunSuiteLike.scala:190)
[info] at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:176)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571721038
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Can we conditionally call `assume` based on the name of test?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570176457
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -260,9 +260,6 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
newLine.startsWith("--") && !newLine.startsWith("--QUERY-DELIMITER")
}
- // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
- // here we need to check command available
- assume(TestUtils.testCommandAvailable("/bin/bash"))
Review comment:
Does it fail or skip? I think it skips, which is fine?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773773261
**[Test build #134907 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134907/testReport)** for PR 31466 at commit [`82c0782`](https://github.com/apache/spark/commit/82c0782f6416716d394b7149cc1fe857b7b86a9e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775042017
**[Test build #135024 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135024/testReport)** for PR 31466 at commit [`08a06fc`](https://github.com/apache/spark/commit/08a06fc8a6228a516caeb3496861a693fe1c68c8).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [WIP][SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773027882
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39439/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775657073
@HyukjinKwon Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570699101
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -260,9 +260,6 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
newLine.startsWith("--") && !newLine.startsWith("--QUERY-DELIMITER")
}
- // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
- // here we need to check command available
- assume(TestUtils.testCommandAvailable("/bin/bash"))
Review comment:
If fail here, `SQLQueryTestSuite` exit.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
`assume` makes `SQLQueryTestSuite` exit
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Yes. the origin code will skip the whole `SQLQueryTestSuite`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571829295
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
`TestUtils.testCommandAvailable("/bin/bash")` won't be executed via short circuiting
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570740350
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Can you double check and investigate to make it working @beliefer? `assume` under `test` should work and skip only that single test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775624131
Thanks @zzcclp. I reverted it back from branch-3.1.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon closed pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #31466:
URL: https://github.com/apache/spark/pull/31466
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571721121
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
e.g.) `assume(... test name contains transform ... || TestUtils.testCommandAvailable("/bin/bash"))`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571911907
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -155,9 +155,13 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
.set(SQLConf.SHUFFLE_PARTITIONS, 4)
/** List of test cases to ignore, in lower cases. */
- protected def ignoreList: Set[String] = Set(
- "ignored.sql" // Do NOT remove this one. It is here to test the ignore functionality.
- )
+ protected val ignoreList: Set[String] = if (TestUtils.testCommandAvailable("/bin/bash")) {
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to ignore it.
+ Set("ignored.sql", "transform.sql")
+ } else {
+ Set("ignored.sql") // Do NOT remove this one. It is here to test the ignore functionality.
+ }
Review comment:
```suggestion
protected def ignoreList: Set[String] = Set(
"ignored.sql" // Do NOT remove this one. It is here to test the ignore functionality.
) ++ {
// SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
// here we need to ignore it.
if (TestUtils.testCommandAvailable("/bin/bash")) Set("transform.sql") else Nil
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775046285
**[Test build #135023 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135023/testReport)** for PR 31466 at commit [`612063c`](https://github.com/apache/spark/commit/612063cf003fcddc811dc06539e0b3c2ab40eb56).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775221969
**[Test build #135025 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135025/testReport)** for PR 31466 at commit [`01caf85`](https://github.com/apache/spark/commit/01caf85014a7733402a0abcb091fa739eeafa32d).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571024361
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Does that skip all tests only in `SQLQueryTestSuite`? or does it happen in other suites too? e.g.) `SparkScriptTransformationSuite` or `PipedRDDSuite`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773146059
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134852/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571859352
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
If the file is `transform.sql`, `TestUtils.testCommandAvailable("/bin/bash")` return false. Then `assume` will break the test cases behind.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775223018
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135025/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775162561
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39608/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571859352
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
If the file is `transform.sql`, `TestUtils.testCommandAvailable("/bin/bash")` return false. Then `assume` will break the follow test cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571811985
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
`SQLQueryTestSuite` must contains `transform.sql`. Why we need to judge `test name contains transform`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570699101
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -260,9 +260,6 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
newLine.startsWith("--") && !newLine.startsWith("--QUERY-DELIMITER")
}
- // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
- // here we need to check command available
- assume(TestUtils.testCommandAvailable("/bin/bash"))
Review comment:
If fail here, `SQLQueryTestSuite` exit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r572513892
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -154,10 +154,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Fewer shuffle partitions to speed up testing.
.set(SQLConf.SHUFFLE_PARTITIONS, 4)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
Review comment:
When there are other PRs in the future, change them by the way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570210410
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -260,9 +260,6 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
newLine.startsWith("--") && !newLine.startsWith("--QUERY-DELIMITER")
}
- // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
- // here we need to check command available
- assume(TestUtils.testCommandAvailable("/bin/bash"))
Review comment:
Seems it skips all tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775599266
@HyukjinKwon Thanks for your work! @cloud-fan @maropu Thanks for your review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773146059
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571828443
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
If so, `SQLQueryTestSuite` will execute `TestUtils.testCommandAvailable("/bin/bash")` many times.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773878017
**[Test build #134907 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134907/testReport)** for PR 31466 at commit [`82c0782`](https://github.com/apache/spark/commit/82c0782f6416716d394b7149cc1fe857b7b86a9e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775071593
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775104234
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39608/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775643637
I think there's a logical conflict in branch-3.1 when we backport, see https://github.com/apache/spark/runs/1859823085. I think it doesn't matter much whether to backport or not in any event.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31466: [WIP][SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773029416
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39439/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [WIP][SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773122521
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [WIP][SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773122521
**[Test build #134852 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134852/testReport)** for PR 31466 at commit [`5087499`](https://github.com/apache/spark/commit/50874995e27f00536b0494d8b73d67f391704aa6).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31466: [WIP][SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773016444
**[Test build #134852 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134852/testReport)** for PR 31466 at commit [`5087499`](https://github.com/apache/spark/commit/50874995e27f00536b0494d8b73d67f391704aa6).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570210410
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -260,9 +260,6 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
newLine.startsWith("--") && !newLine.startsWith("--QUERY-DELIMITER")
}
- // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
- // here we need to check command available
- assume(TestUtils.testCommandAvailable("/bin/bash"))
Review comment:
Seems it skips all tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775180765
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39608/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zzcclp commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
zzcclp commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775618187
> Merged to master and branch-3.1.
@HyukjinKwon after merged this pr into branch-3.1, there is an 'not found: value TestUtils' error in SQLQueryTestSuite, root cause is missing import 'org.apache.spark.TestUtils'.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571868026
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
The case I tested above is when the file is `transform.sql` and `TestUtils.testCommandAvailable("/bin/bash")` returns `false`. I tested it by manually changing `/bin/bash` to an arbitrary executable such as `/bin/bas`.
Does it fail and stop all other test cases? Then maybe it is a Windows specific behaviour. If that's the case, I am fine with the current fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570650670
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -260,9 +260,6 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
newLine.startsWith("--") && !newLine.startsWith("--QUERY-DELIMITER")
}
- // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
- // here we need to check command available
- assume(TestUtils.testCommandAvailable("/bin/bash"))
Review comment:
Can we conditionally skip here instead of filtering the tests out?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570644961
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Could you use `ignore` instead of filtering out it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775080723
**[Test build #135025 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135025/testReport)** for PR 31466 at commit [`01caf85`](https://github.com/apache/spark/commit/01caf85014a7733402a0abcb091fa739eeafa32d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775042017
**[Test build #135024 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135024/testReport)** for PR 31466 at commit [`08a06fc`](https://github.com/apache/spark/commit/08a06fc8a6228a516caeb3496861a693fe1c68c8).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775046285
**[Test build #135023 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135023/testReport)** for PR 31466 at commit [`612063c`](https://github.com/apache/spark/commit/612063cf003fcddc811dc06539e0b3c2ab40eb56).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775597566
Merged to master and branch-3.1.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [WIP][SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773026451
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39439/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775223018
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135025/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773140157
cc @cloud-fan @wangyum @maropu
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773146059
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773140157
cc @cloud-fan @wangyum @maropu
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773785184
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39489/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773897289
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134907/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773794573
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39489/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571876655
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Another reason is each test case will execute `assume`, if we adopt `ignoreList` to fix this issue, it seems simpler.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773794596
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39489/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773897289
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134907/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775071593
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773773261
**[Test build #134907 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134907/testReport)** for PR 31466 at commit [`82c0782`](https://github.com/apache/spark/commit/82c0782f6416716d394b7149cc1fe857b7b86a9e).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775080723
**[Test build #135025 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135025/testReport)** for PR 31466 at commit [`01caf85`](https://github.com/apache/spark/commit/01caf85014a7733402a0abcb091fa739eeafa32d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775051064
**[Test build #135023 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135023/testReport)** for PR 31466 at commit [`612063c`](https://github.com/apache/spark/commit/612063cf003fcddc811dc06539e0b3c2ab40eb56).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r571911907
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -155,9 +155,13 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
.set(SQLConf.SHUFFLE_PARTITIONS, 4)
/** List of test cases to ignore, in lower cases. */
- protected def ignoreList: Set[String] = Set(
- "ignored.sql" // Do NOT remove this one. It is here to test the ignore functionality.
- )
+ protected val ignoreList: Set[String] = if (TestUtils.testCommandAvailable("/bin/bash")) {
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to ignore it.
+ Set("ignored.sql", "transform.sql")
+ } else {
+ Set("ignored.sql") // Do NOT remove this one. It is here to test the ignore functionality.
+ }
Review comment:
```suggestion
protected val ignoreList: Set[String] = Set(
"ignored.sql" // Do NOT remove this one. It is here to test the ignore functionality.
) ++ {
// SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
// here we need to ignore it.
if (TestUtils.testCommandAvailable("/bin/bash")) Set("transform.sql") else Nil
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31466: [WIP][SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773029416
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39439/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570644961
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Could you use `ignore` instead of filtering out it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775630900
Why missing import 'org.apache.spark.TestUtils' ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31466: [WIP][SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773016444
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r570650364
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -566,7 +563,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Filter out test files with invalid extensions such as temp files created
// by vi (.swp), Mac (.DS_Store) etc.
val filteredFiles = files.filter(_.getName.endsWith(validFileExtensions))
- filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ val allFiles = filteredFiles ++ dirs.flatMap(listFilesRecursively)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
+ // here we need to check command available
+ if (TestUtils.testCommandAvailable("/bin/bash")) {
+ allFiles
+ } else {
+ allFiles.filterNot(_.getName == "transform.sql")
Review comment:
Yeah, can we call `assume` to trigger skipping the test instead of filtering out here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-775047390
**[Test build #135024 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135024/testReport)** for PR 31466 at commit [`08a06fc`](https://github.com/apache/spark/commit/08a06fc8a6228a516caeb3496861a693fe1c68c8).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31466: [WIP][SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31466:
URL: https://github.com/apache/spark/pull/31466#issuecomment-773016444
**[Test build #134852 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134852/testReport)** for PR 31466 at commit [`5087499`](https://github.com/apache/spark/commit/50874995e27f00536b0494d8b73d67f391704aa6).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31466: [SPARK-34352][SQL] Improve SQLQueryTestSuite so as could run on windows system
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31466:
URL: https://github.com/apache/spark/pull/31466#discussion_r572038364
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
##########
@@ -154,10 +154,14 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
// Fewer shuffle partitions to speed up testing.
.set(SQLConf.SHUFFLE_PARTITIONS, 4)
+ // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
Review comment:
`SQL test 'transform.sql' will use` -> `SQL test 'transform.sql' which uses`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org