Run unit tests as part of CI builds

[mccheah]

Currently we're relying on integration tests with Minikube to verify the correctness of our work. We should also include lower-level unit tests to harden our current behavior and prevent regressions of it.

This will be far easier once we do #109.

[kimoonkim]

+1. Do we think we should run unit tests only in kubernetes maven modules first? I find other spark module unit tests flaky and take too much time.

[mccheah]

I feel pretty comfortable with running core and kubernetes. We may make some changes to core down the line but it should be better than running streaming, etc.

[kimoonkim]

Makes sense. Maybe try them in Travis CI even before #109? If yes, I can write a PR.

[mccheah]

Sure, I think we can unit test KubernetesClusterSchedulerBackend pretty sanely right now. Client is the class that probably needs some work to make it easier to test.

[kimoonkim]

Sounds good. I'll see what I can do.

[kimoonkim]

Thinking about using the following:

$ build/mvn -Pmesos -Pyarn -Phadoop-2.3 -Pkubernetes  \
   -pl core,resource-managers/kubernetes/core -am clean test

, which includes core and resource-managers/kubernetes/core. (A few other modules they depend on will also run, but they are fast) From my Macbook:

[INFO] Reactor Summary:
[INFO]
[INFO] Spark Project Parent POM ........................... SUCCESS [  6.051 s]
[INFO] Spark Project Tags ................................. SUCCESS [  8.760 s]
[INFO] Spark Project Networking ........................... SUCCESS [01:00 min]
[INFO] Spark Project Shuffle Streaming Service ............ SUCCESS [ 15.120 s]
[INFO] Spark Project Unsafe ............................... SUCCESS [ 14.343 s]
[INFO] Spark Project Launcher ............................. SUCCESS [ 14.200 s]
[INFO] Spark Project Core ................................. SUCCESS [21:36 min]
[INFO] Spark Project Kubernetes ........................... SUCCESS [ 14.804 s]

I also explored using -DwildcardSuites with package name filters. Hard to find good filters for core because too many spark modules use overlapping package names.

@mccheah Does this make sense? I imagine I can put this in .travis.yml and run it in parallel with the existing basic maven command.

[ash211]

This looks reasonable to enable -- we don't have many (any?) unit tests right now but starting to run them now makes sense.

@mccheah would you support adding the command @kimoonkim pasted above to the travis config? Looks like it would run just the tests. I'd want to make one change: hadoop 2.7 instead of 2.3, but that won't be a large change.

[kimoonkim]

@ash211 @mccheah I started writing a .travis.yml change assuming we want to do this. I'll switch to hadoop 2.7 there since it's quite trivial.

[kimoonkim]

Played with Travis on my personal fork a bit. Turned out the spark core unit tests are flaky. Hangs and times out too often causing build failures. I am thinking about dropping spark core all together or filtering even more so only a subset related with kubernetes client code runs.

[mccheah]

We're going to want to test core since we make changes to it - #116 comes to mind.

[kimoonkim]

@mccheah Thanks for the input. It seems most of our changes to core so far are made on a small set of packages, like org.apache.spark.deploy.

$ wget -qO- https://github.com/apache-spark-on-k8s/spark/pull/1.diff | grep "^+++ b/" | grep "src\/main\/scala" | sort | uniq
+++ b/core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/rest/RestSubmissionServer.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/kubernetes/Client.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/kubernetes/KubernetesClientBuilder.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/kubernetes/LoggingPodStatusWatcher.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/kubernetes/config.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/kubernetes/constants.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/rest/KubernetesRestProtocolMessages.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/rest/kubernetes/CompressionUtils.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/rest/kubernetes/HttpClientUtil.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/rest/kubernetes/KubernetesFileUtils.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/rest/kubernetes/KubernetesSparkRestApi.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/rest/kubernetes/KubernetesSparkRestServer.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/rest/kubernetes/MultiServerFeignTarget.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/kubernetes/KubernetesClusterManager.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/kubernetes/KubernetesClusterSchedulerBackend.scala
+++ b/resource-managers/kubernetes/integration-tests-spark-jobs/src/main/scala/org/apache/spark/deploy/kubernetes/integrationtest/jobs/FileExistenceTest.scala
+++ b/resource-managers/kubernetes/integration-tests-spark-jobs/src/main/scala/org/apache/spark/deploy/kubernetes/integrationtest/jobs/SparkPiWithInfiniteWait.scala

Maybe we can start by including only these packages, using -DwildcardSuite in tandem with -pl? And add more as needed (This needs some maintenance work, I admit)

$ build/mvn test -Pmesos -Pyarn -Phadoop-2.7 -Pkubernetes
-pl core,resource-managers/kubernetes/core -am
-Dtest=none -DwildcardSuites=org.apache.spark.deploy.,org.apache.spark.scheduler.cluster.kubernetes.

I tried this in my Travis (an example build). This covers ClientSuite, SparkSubmmitSuite, etc. It seems more stable than running all unit tests in core.

The point is to avoid flaky unit test builds, which slows down everyone. But, I am open to other alternatives. Any suggestions?

[mccheah]

What about running the full suite for now and then blacklisting the specific ones we find to be flaky as we go along? This is a more conservative stance where we try to test as much as we can and have minimal risk of omitting a test that would have caught something we introduced. But I don't know how difficult it would be to specify specific tests to omit from the Maven build; we wouldn't want to use annotations on the test classes themselves.

[kimoonkim]

We can try that. Yes, we don't want to annotate source code. It all boils down to what kind of negative filters are supported by the scalatest maven plugin. I couldn't find good options to use so far, but we can dig more. Also hunting down flaky tests can be time-consuming in general. But if we're lucky, maybe it's just one or two.

Another thing is whether we can drop JDK7 builds. With the unit tests config added, Travis will run four parallel builds (Unit tests and full builds for JDK7. Same for JDK8). I have a hunch running all of them adds to flakiness. I'd rather drop JDK7 and buy more stability and coverage instead if I can. What do you think?