[ 
https://issues.apache.org/jira/browse/SPARK-19592?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15869985#comment-15869985
 ] 

Armin Braun commented on SPARK-19592:
-------------------------------------

I see your point on these two:

{quote}
Isn't this going to mean changing every single test suite?
That is to say I could kind of imagine a broader cleanup and refactoring of 
test state. A big change just to remove a few lines of config doesn't seem 
worth it.
{quote}

Yea it will obviously require wider changes (but see below).  Looks to me like 
this would be a valid start for cleaning up tests state in general.

{quote}
 Ideally that's cleaned up all in one go or not.
{quote}

I mean you could go testsuite by testsuite and eventually drop the properties 
being injected by the build system. Doing this all in one go would admittedly 
be a big change.
Even an incremental approach (doing this step by step and having the test setup 
inside ScalaTest be redundant) would be worth it in my opinion though ... would 
already make the test env more readable (and less importantly but nice to have 
... runnable from the IDE) wouldn't it?

> Duplication in Test Configuration Relating to SparkConf Settings Should be 
> Removed
> ----------------------------------------------------------------------------------
>
>                 Key: SPARK-19592
>                 URL: https://issues.apache.org/jira/browse/SPARK-19592
>             Project: Spark
>          Issue Type: Improvement
>          Components: Tests
>    Affects Versions: 2.1.0, 2.2.0
>         Environment: Applies to all Environments
>            Reporter: Armin Braun
>            Priority: Minor
>
> This configuration for Surefire, Scalatest is duplicated in the parent POM as 
> well as the SBT build.
> While this duplication cannot be removed in general it can at least be 
> removed for all system properties that simply result in a SparkConf setting I 
> think.
> Instead of having lines like 
> {code}
> <spark.ui.enabled>false</spark.ui.enabled>
> {code}
> twice in the pom.xml
> and once in SBT as
> {code}
> javaOptions in Test += "-Dspark.ui.enabled=false",
> {code}
> it would be a lot cleaner to simply have a 
> {code}
> var conf: SparkConf 
> {code}
> field in 
> {code}
> org.apache.spark.SparkFunSuite
> {code}
>  that has SparkConf set up with all the shared configuration that 
> `systemProperties` currently provide. Obviously this cannot be done straight 
> away given that
> many subclasses of the parent suit do this, so I think it would be best to 
> simply add a method to the parent that provides this configuration for now
> and start refactoring away duplication in other suit setups from there step 
> by step until the sys properties can be removed from the pom and sbt.build.
> This makes the build a lot easier to maintain and makes tests more readable 
> by making the environment setup more explicit in the code.
> (also it would allow running more tests straight from the IDE which is always 
> a nice thing imo)



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to