You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by attilapiros <gi...@git.apache.org> on 2018/03/18 05:08:44 UTC
[GitHub] spark pull request #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expec...
GitHub user attilapiros opened a pull request:
https://github.com/apache/spark/pull/20852
[SPARK-23728][BRANCH-2.3] Fix ML tests with expected exceptions running streaming tests
## What changes were proposed in this pull request?
The testTransformerByInterceptingException failed to catch the expected message on 2.3 during streaming tests as the feature generated message is not at the direct caused by exception but even one level deeper.
## How was this patch tested?
Running the unit tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/attilapiros/spark SPARK-23728
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20852.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 #20852
----
commit 7544cb427cbbb1a0186ad5e16cf4f09fee0c0dbf
Author: “attilapiros” <pi...@...>
Date: 2018-03-18T04:56:08Z
fix testTransformerByInterceptingException
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20852
Thank you, @jkbradley and @attilapiros .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expec...
Posted by WeichenXu123 <gi...@git.apache.org>.
Github user WeichenXu123 commented on a diff in the pull request:
https://github.com/apache/spark/pull/20852#discussion_r175380424
--- Diff: mllib/src/test/scala/org/apache/spark/ml/util/MLTest.scala ---
@@ -119,9 +119,15 @@ trait MLTest extends StreamTest with TempDirectory { self: Suite =>
expectedMessagePart : String,
firstResultCol: String) {
+ def hasExpectedMessageDirectly(exception: Throwable): Boolean =
+ exception.getMessage.contains(expectedMessagePart)
+
def hasExpectedMessage(exception: Throwable): Boolean =
- exception.getMessage.contains(expectedMessagePart) ||
- (exception.getCause != null && exception.getCause.getMessage.contains(expectedMessagePart))
+ hasExpectedMessageDirectly(exception) || (
+ exception.getCause != null && (
+ hasExpectedMessageDirectly(exception.getCause) || (
+ exception.getCause.getCause != null &&
+ hasExpectedMessageDirectly(exception.getCause.getCause))))
--- End diff --
Why not use a loop to check inner exception ? So it can check any deep level exception msg.
Maybe we cannot make sure the possible root exception max level (even in master version)
@jkbradley What do you think ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expec...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:
https://github.com/apache/spark/pull/20852#discussion_r175524288
--- Diff: mllib/src/test/scala/org/apache/spark/ml/util/MLTest.scala ---
@@ -119,9 +119,15 @@ trait MLTest extends StreamTest with TempDirectory { self: Suite =>
expectedMessagePart : String,
firstResultCol: String) {
+ def hasExpectedMessageDirectly(exception: Throwable): Boolean =
+ exception.getMessage.contains(expectedMessagePart)
+
def hasExpectedMessage(exception: Throwable): Boolean =
- exception.getMessage.contains(expectedMessagePart) ||
- (exception.getCause != null && exception.getCause.getMessage.contains(expectedMessagePart))
+ hasExpectedMessageDirectly(exception) || (
+ exception.getCause != null && (
+ hasExpectedMessageDirectly(exception.getCause) || (
+ exception.getCause.getCause != null &&
+ hasExpectedMessageDirectly(exception.getCause.getCause))))
--- End diff --
I don't think we need to loop further. If the real message is buried this deep, that could be considered a problem in and of itself.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20852
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88345/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:
https://github.com/apache/spark/pull/20852
Apologies for breaking it!
This LGTM
I'll go ahead and merge it to fix the build, but please comment further on this PR as needed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by attilapiros <gi...@git.apache.org>.
Github user attilapiros commented on the issue:
https://github.com/apache/spark/pull/20852
Closing manually as it was merged to branch-2.3.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20852
(@attilapiros, just in case it should be manually closed)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20852
**[Test build #88345 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88345/testReport)** for PR 20852 at commit [`7544cb4`](https://github.com/apache/spark/commit/7544cb427cbbb1a0186ad5e16cf4f09fee0c0dbf).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:
https://github.com/apache/spark/pull/20852
Merging with branch-2.3
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/20852
Hi, @yanboliang and @jkbradley .
`branch-2.3` is broken. Could you review this PR?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20852
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expec...
Posted by attilapiros <gi...@git.apache.org>.
Github user attilapiros closed the pull request at:
https://github.com/apache/spark/pull/20852
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20852
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20852: [SPARK-23728][BRANCH-2.3] Fix ML tests with expected exc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20852
**[Test build #88345 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88345/testReport)** for PR 20852 at commit [`7544cb4`](https://github.com/apache/spark/commit/7544cb427cbbb1a0186ad5e16cf4f09fee0c0dbf).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org