static client mode e2e test

[sudiptob2]

closes G-Research/spark#153

[GeorgeJahad]

Still not quite what I'm looking for.

Take a look at this code for a "more complete" baseSparkPiTest(), (which is meant to be used with all spark pi tests.):

https://github.com/GeorgeJahad/armada-spark/blob/04aaed7b7f33b7db966bc5957bf560b88047c9f1/src/test/scala/org/apache/spark/deploy/armada/e2e/ArmadaSparkE2E.scala#L184-L203

and this is my version of baseSparkPiGangTest():

https://github.com/GeorgeJahad/armada-spark/blob/04aaed7b7f33b7db966bc5957bf560b88047c9f1/src/test/scala/org/apache/spark/deploy/armada/e2e/ArmadaSparkE2E.scala#L208-L220

With those methods defined, the deployMode specific gang tests become very simple:

  test("Basic SparkPi job with gang scheduling - staticCluster", E2ETest) {
    baseSparkPiGangTest("cluster")
      .assertGangJob("armada-spark", 3) // 1 driver + 2 executors
      .run()
  }

  test("Basic SparkPi job with gang scheduling - staticClient", E2ETest) {
    baseSparkPiGangTest("client")
      .assertExecutorGangJob("armada-spark", 2) // Only 2 executors, no driver
      .run()
  }

All the boilerplate is in the moreCompleteBaseSparkPiTest(). The baseSparkPiGangTest() contains just the interesting parts of the test that are common to both. The deployMode specific tests just contain the parts that apply to that deployMode. Doesn't that seem clearer?

FYI I have run my version of the gang tests and they seem to work as expected:
https://github.com/GeorgeJahad/armada-spark/actions/runs/20840130283

[sudiptob2]

@GeorgeJahad updated now. While consolidating under a common base method, there was at least one test which was doing something different, I refactored them slightly to make use of the common base methods.

[GeorgeJahad]

does this comment need to be moved?

[sudiptob2]

yes lets move it before the base method

[GeorgeJahad]

Everything looks good. The last question I have is that the ingressAnnotations for the cli test now include the "kubernetes.io/ingress.class", but didn't before: https://github.com/armadaproject/armada-spark/blob/master/src/test/scala/org/apache/spark/deploy/armada/e2e/ArmadaSparkE2E.scala#L232-L235

Why the change?

[sudiptob2]

I added it to make it similar, I far as I understood, it doesn't affect the intention of the test. Let me know if you know of any specific reason that might affect the test.

[GeorgeJahad]

lgtm, thanks @sudiptob2 !