[SPARK-15296][MLlib] Refactor All Java Tests that use SparkSession by techaddict · Pull Request #13101 · apache/spark

techaddict · 2016-05-13T16:08:46Z

What changes were proposed in this pull request?

Refactor All Java Tests that use SparkSession, to extend SharedSparkSesion

How was this patch tested?

Existing Tests

SparkQA · 2016-05-13T16:13:20Z

Test build #58581 has finished for PR 13101 at commit c181fa6.

This patch fails RAT tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-05-13T16:59:47Z

Test build #58582 has finished for PR 13101 at commit 12ba028.

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

techaddict · 2016-05-13T17:00:56Z

cc: @andrewor14 @mengxr

SparkQA · 2016-05-18T03:20:48Z

Test build #58737 has finished for PR 13101 at commit f3fa1f5.

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

andrewor14 · 2016-05-19T18:42:39Z

+ customSetUpWithException();
+ }
+
+ public void customSetUp() {}


Many of these can be protected so please mark them as such.

andrewor14 · 2016-05-19T18:44:50Z

@techaddict can you run dev/lint-java locally? Jenkins doesn't run that.

mengxr · 2016-05-19T21:45:37Z

+ spark.stop();
+ spark = null;
+
+ customTearDown();


Ditto. It is more like a pattern choice. For tearing down, users might want to insert some operations before spark is gone out of scope. So let users choose where to call super.tearDown() seems more reasonable here.

mengxr · 2016-05-19T21:46:29Z

Great to see 900 lines were removed:)

srowen · 2016-05-20T00:47:25Z

+ public void setUp() throws IOException {
+ spark = SparkSession.builder()
+ .master("local")
+ .appName("shared-spark-session")


This can set the app name to the name of the test class with getClass.getSimpleName

mengxr · 2016-05-20T01:17:52Z

+ @Before
+ public void setUp() throws IOException {
+ spark = SparkSession.builder()
+ .master("local")


Some tests depend on the default number of partitions. So it would be better to say local[2] instead of local.

SparkQA · 2016-05-20T01:41:20Z

Test build #58924 has finished for PR 13101 at commit 56de3e4.

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

techaddict · 2016-05-20T01:43:14Z

@mengxr @andrewor14 @srowen Addressed all comments. Thanks for reviewing.

mengxr · 2016-05-20T01:48:09Z

LGTM pending Jenkins

SparkQA · 2016-05-20T02:13:07Z

Test build #58923 has finished for PR 13101 at commit 65307c5.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class MultivariateGaussian @Since(\"2.0.0\") (

SparkQA · 2016-05-20T02:18:12Z

Test build #58928 has finished for PR 13101 at commit 9d7a89c.

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

SparkQA · 2016-05-20T02:26:38Z

Test build #58929 has finished for PR 13101 at commit 138818b.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- public class JavaMultilayerPerceptronClassifierSuite extends SharedSparkSession

mengxr · 2016-05-20T03:39:05Z

Merged into master and branch-2.0. Thanks!

## What changes were proposed in this pull request? Refactor All Java Tests that use SparkSession, to extend SharedSparkSesion ## How was this patch tested? Existing Tests Author: Sandeep Singh <sandeep@techaddict.me> Closes#13101 from techaddict/SPARK-15296. (cherry picked from commit 01cf649) Signed-off-by: Xiangrui Meng <meng@databricks.com>

techaddict added 18 commits May 13, 2016 21:49

ML Classification done
68cebd0

ml clustering done
e8bae89

ML Feature done
afe620a

ML Feature remove unused imports
f3a2244

ML Param Done - Remove SparkSession, esc since not used
9e41936

ml regression done
b97da5b

ml libsvm done
a96fa3a

ml tuning, util done
29a1194

mllib classification
8ad34a0

mllib clustering
940d564

mllib evaluation and feature
9d4d015

mllib fpm
e033253

mllib random
3aa61df

mllib recommendation
ddf68da

mllib regression
90b048a

mllib tree
c3f166d

fix javastyle
36ce8d2

add license to SharedSparkSession
12ba028

techaddict force-pushed the SPARK-15296 branch from 067b36c to 12ba028Compare May 13, 2016 16:19

techaddict added 2 commits May 18, 2016 07:54

merge master
39cd94d

fix import
f3fa1f5

andrewor14 reviewed May 19, 2016
View reviewed changes

Mark custom methods as protected and add override
e4117f3

mengxr reviewed May 19, 2016
View reviewed changes

techaddict added 2 commits May 20, 2016 06:08

remove customSetUp() and customTearDown()
40edaad

Merge branch 'master' into SPARK-15296
65307c5

srowen reviewed May 20, 2016
View reviewed changes

techaddict added 3 commits May 20, 2016 06:23

SparkSession appName should be class simple name
9b340fe

Merge branch 'master' into SPARK-15296
622e28d

Merge branch 'master' into SPARK-15296
56de3e4

mengxr reviewed May 20, 2016
View reviewed changes

techaddict added 4 commits May 20, 2016 07:02

SharedSparkSession should implement Serializable
c1ce08e

fix imports
2bcfdd7

Merge branch 'master' into SPARK-15296
9d7a89c

fix
138818b

asfgit closed this in 01cf649May 20, 2016

Uh oh!

Conversation

techaddict commented May 13, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented May 13, 2016

Uh oh!

SparkQA commented May 13, 2016

Uh oh!

techaddict commented May 13, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented May 18, 2016

Uh oh!

andrewor14May 19, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewor14 commented May 19, 2016

Uh oh!

mengxrMay 19, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mengxr commented May 19, 2016

Uh oh!

srowenMay 20, 2016

Choose a reason for hiding this comment

Uh oh!

techaddictMay 20, 2016

Choose a reason for hiding this comment

Uh oh!

mengxrMay 20, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 20, 2016

Uh oh!

techaddict commented May 20, 2016

Uh oh!

mengxr commented May 20, 2016

Uh oh!

SparkQA commented May 20, 2016

Uh oh!

SparkQA commented May 20, 2016

Uh oh!

SparkQA commented May 20, 2016

Uh oh!

mengxr commented May 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

techaddict commented May 13, 2016•
edited
Loading

andrewor14May 19, 2016•
edited
Loading

mengxrMay 19, 2016•
edited
Loading