SPARK-9487][Tests] Begin the work of robustifying unit tests , start with ContextCleanerSuite

[skanjila]

…tests

What changes were proposed in this pull request?

This is a work in progress pull request whose goal will be to robustify the unit tests, to kick this off I will focus on one module at a time, for now I will focus on robustifying all unit tests around ContextCleanerSuite
(Please fill in changes proposed in this fix)
Changed ContextCleanerSuite to use local[4] instead of local[2]

How was this patch tested?

I ran unit tests locally for this suite and everything passed locally, of course this doesnt mean anything till we see how Jenkins deals with these changes
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Please review http://spark.apache.org/contributing.html before opening a pull request.

[srowen]

How does this relate to #15848 ?
As I understand you already hit problems and were not able to debug them. What's different here?
I was going to close the JIRA, to set your expectation, because I'm not seeing it's easy to make this change, and this is taking a lot of time to discuss, and it doesn't really solve a problem.

[skanjila]

Whats different is that this time around I am only making one change at a time (in this case ContextCleanerSuite), fixing the unit tests by backtracking from the Jenkins failures. In this strategy I can focus my efforts on rewriting unit tests around one module or class as opposed to dealing with 3 or 4 failures at a time. If you don't feel like this approach is worth pursuing you can close this JIRA but I at least thought I would purse this and see how far I get. By the way this approach is a result of our discussions where you had mentioned creating a WIP pull request where we fix issues one at a time for each class or module.

[SparkQA]

Test build #71021 has finished for PR 16498 at commit b338ca9.

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

[rxin]

Can you explain why this makes it more robust?

[skanjila]

@rxin long story here, the goal of this jira is to create robust unit tests where appropriate and in doing so use the same number of local instances in the spark config both in the Scala and Python code which is local[4]. I am making this change individually and creating pull requests, in some cases the tests will fail in Jenkins but work locally, in these cases I will be working backwards from Jenkins and trying to make the unit tests more robust and in doing so independent of the number of local instances chosen. Make sense?

[srowen]

To be clear, we're not going to commit this one test case at a time. The best strategy is to get a logical subset working, at the least, like for a whole module. It may entail some tricky debugging if you can't reproduce the Jenkins failures locally, though I think I flagged some potential fixes in the last PRs already.

To be honest I don't know that this is worth our time. It's not making the tests better, just changing them to be more consistent -- which, as a side effect, reveals a few tests which are a bit dependent on local[2] vs local[4] for exact results. This isn't such a big deal.

[skanjila]

@srowen I'm ok closing this given the time/effort and energy I've put in, I am more than willing to do challenging debugging for every module that we wish to make unit tests better but if you don't feel like its worth the effort I can pick a different JIRA to tackle. Let me know how you'd like to proceed.

[srowen]

As before, I think this is around in circles. This PR just changes 1 file, now, not even a module's worth.

If you aren't progressing work on this and haven't been able to resolve why your previous change failed, and there's no strong need to do this anyway, I think we should not keep repeating this discussion.