You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by HyukjinKwon <gi...@git.apache.org> on 2016/12/29 09:06:38 UTC
[GitHub] spark pull request #16433: [SPARK-19022][TESTS] Fix tests dependent on OS du...
GitHub user HyukjinKwon opened a pull request:
https://github.com/apache/spark/pull/16433
[SPARK-19022][TESTS] Fix tests dependent on OS due to different newline characters
## What changes were proposed in this pull request?
There are two tests failing on Windows due to the different newlines.
```
- StreamingQueryProgress - prettyJson *** FAILED *** (0 milliseconds)
"{
"id" : "39788670-6722-48b7-a248-df6ba08722ac",
"runId" : "422282f1-3b81-4b47-a15d-82dda7e69390",
"name" : "myName",
...
}" did not equal "{
"id" : "39788670-6722-48b7-a248-df6ba08722ac",
"runId" : "422282f1-3b81-4b47-a15d-82dda7e69390",
"name" : "myName",
...
}"
...
```
```
- StreamingQueryStatus - prettyJson *** FAILED *** (0 milliseconds)
"{
"message" : "active",
"isDataAvailable" : true,
"isTriggerActive" : false
}" did not equal "{
"message" : "active",
"isDataAvailable" : true,
"isTriggerActive" : false
}"
...
```
The reason is, `pretty` in `org.json4s.pretty` writes OS-dependent newlines but the string defined in the tests are `\n`. This ends up with test failures.
This PR proposes to compare these regardless of newline concerns.
## How was this patch tested?
Manually tested via AppVeyor.
**Before**
https://ci.appveyor.com/project/spark-test/spark/build/417-newlines-fix-before
**After**
https://ci.appveyor.com/project/spark-test/spark/build/418-newlines-fix
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HyukjinKwon/spark tests-StreamingQueryStatusAndProgressSuite
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/16433.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #16433
----
commit 15f821cadd39027cfd8860309e32d6b06be92833
Author: hyukjinkwon <gu...@gmail.com>
Date: 2016-12-29T05:27:05Z
Fix newline comparison issues
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/16433
I just double checked. It seems `org.json4s.pretty` is being used in several places but they look for debugging purpose, printing purpose and making a request body (e.g., `StandaloneRestSubmitSuite`). So, for `org.json4s.pretty`, it seems these are only tests being failed due to this problem.
About OS-dependent newline tests, I just checked the rest of them. I skimmed again the failed tests at my best and it seems these are only the tests being failed due to this problem.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16433
Merged to master
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16433: [SPARK-19022][TESTS] Fix tests dependent on OS du...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/16433#discussion_r94174880
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamingQueryStatusAndProgressSuite.scala ---
@@ -30,10 +30,16 @@ import org.apache.spark.sql.streaming.StreamingQueryStatusAndProgressSuite._
class StreamingQueryStatusAndProgressSuite extends StreamTest {
+ implicit class EqualsIgnoreCRLF(source: String) {
+ def equalsIgnoreCRLF(target: String): Boolean = {
+ source.stripMargin.replaceAll("\r\n|\r|\n", System.lineSeparator) ===
--- End diff --
not need to call `stripMargin` in this method
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16433
Maybe a dumb question but why is this only a problem in this test and not for many other test suites? i'd sort of imagine the newline difference affects a lot of comparisons.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/16433
Hm, is it because this is the only test for prettyJson? that could make sense then. I want to avoid repeating this logic in tests, but it seems like there are no other instances of it. I also want to make sure that, say, we _do_ want the output of prettyJson to vary by platform. Hm, I guess that's reasonable here as it's meant for display on a terminal I guess.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/16433
Yes, I hesitated to submit this PR for a while due to similar concerns..
> is it because this is the only test for prettyJson?
I believe so. Let me double check again.
> I also make sure that, say, we do want the output of prettyJson to vary by platform. Hm, I guess that's reasonable here as it's meant for display on a terminal I guess.
^
@zsxwing could you confirm this please if possible?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16433
**[Test build #70718 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70718/testReport)** for PR 16433 at commit [`15f821c`](https://github.com/apache/spark/commit/15f821cadd39027cfd8860309e32d6b06be92833).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` implicit class EqualsIgnoreCRLF(source: String) `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16433
**[Test build #70718 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70718/testReport)** for PR 16433 at commit [`15f821c`](https://github.com/apache/spark/commit/15f821cadd39027cfd8860309e32d6b06be92833).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16433
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16433
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70735/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/16433
In most cases, it seems they explicitly write `\n` (e.g. writing CSV and JSON). _Apparently_, these seem only tests being failed due to this problem
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16433
**[Test build #70735 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70735/testReport)** for PR 16433 at commit [`f3f78b8`](https://github.com/apache/spark/commit/f3f78b8522e3eb0bba5cd6927f49d199d5bd9f92).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` implicit class EqualsIgnoreCRLF(source: String) `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/16433
Build started: [TESTS] `org.apache.spark.sql.streaming.StreamingQueryStatusAndProgressSuite` [](https://ci.appveyor.com/project/spark-test/spark/branch/AE40452F-D970-407C-92EB-C8079EC86A06)
Diff: https://github.com/apache/spark/compare/master...spark-test:AE40452F-D970-407C-92EB-C8079EC86A06
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16433
**[Test build #70735 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70735/testReport)** for PR 16433 at commit [`f3f78b8`](https://github.com/apache/spark/commit/f3f78b8522e3eb0bba5cd6927f49d199d5bd9f92).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16433: [SPARK-19022][TESTS] Fix tests dependent on OS du...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/16433#discussion_r94200602
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamingQueryStatusAndProgressSuite.scala ---
@@ -30,10 +30,16 @@ import org.apache.spark.sql.streaming.StreamingQueryStatusAndProgressSuite._
class StreamingQueryStatusAndProgressSuite extends StreamTest {
+ implicit class EqualsIgnoreCRLF(source: String) {
+ def equalsIgnoreCRLF(target: String): Boolean = {
+ source.stripMargin.replaceAll("\r\n|\r|\n", System.lineSeparator) ===
--- End diff --
Oh, sure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/16433
Build started: [TESTS] `org.apache.spark.sql.streaming.StreamingQueryStatusAndProgressSuite` [](https://ci.appveyor.com/project/spark-test/spark/branch/D1A3B54F-82B5-481D-ADE8-7CC273C97303)
Diff: https://github.com/apache/spark/compare/master...spark-test:D1A3B54F-82B5-481D-ADE8-7CC273C97303
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16433
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #16433: [SPARK-19022][TESTS] Fix tests dependent on OS due to di...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16433
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70718/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #16433: [SPARK-19022][TESTS] Fix tests dependent on OS du...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/16433
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org