Thanks for the sharing the details as well as what happened in the past. in this case, any concern with adding docker based regression test to spark 4 for now until we have a better solution?
@ Dmitri and Robert Thanks, Yong Zheng On 2026/07/07 01:36:48 yun zou wrote: > Hi, > > Thanks for laying out your reasoning. I agree with several of your goals, > particularly that every required CI test should have a clear purpose and > that we should avoid redundant coverage. However, I don't think the > proposed JUnit/Gradle test is a complete replacement for the current Spark > regression test. > > First, I think the "~2m30s" point is being interpreted a bit differently > than intended. It wasn't meant to justify keeping a heavy test simply > because it's "only" a few minutes. Rather, it was in response to the > characterization that the current Spark regression test is prohibitively > expensive. The runtime is relatively modest for the coverage it provides. > > Second, I think the current regression test is validating two different > things: > > - the Polaris service behaves correctly; and > - the Spark client can be packaged, launched, and used successfully in a > realistic environment. > > Those have different goals and different failure modes. For example, > differences in storage backends or I/O implementations are important for > the service-side tests, but they aren't really the focus of validating the > Spark client. I'd actually favor separating those concerns into two > different CI jobs rather than replacing the Spark client regression test > entirely. > > Regarding --packages, we added that coverage for a concrete reason. While > our documentation primarily demonstrates using --jars, both --jars and > --packages are well-established ways of launching Spark applications, and > users commonly use --packages as an alternative. > > Historically, we started with only the --jars regression test. We later > added --packages support because we encountered a real regression that the > --jars test did not catch: using --packages pulled in the transitive > dependencies of polaris-core, which conflicted with the version of Avro > required by Spark. That failure mode only appeared when Spark performed > dependency resolution in the same way users do. > > This is also why I'm not convinced a JUnit test is sufficient. To achieve > equivalent coverage, we'd effectively need to mimic how Spark resolves both > --jars and --packages. That means introducing another layer of test logic > that we'd have to maintain, and one that necessarily depends on Spark's > implementation. I'd rather exercise Spark's actual launcher and dependency > resolution than maintain our own approximation of that behavior. > > So I agree that we should make the responsibilities of the regression tests > clearer, and I'm supportive of separating the service-focused and Spark > client-focused tests. Where I disagree is that the Docker-based Spark > regression test is merely an end-to-end duplicate. It validates packaging > and launch behavior that has already caught at least one real regression > and does so by exercising the same code paths our users rely on. > > Best Regards, > Yun > > On Sat, Jul 4, 2026 at 4:30 PM Yong Zheng <[email protected]> wrote: > > > Hello, > > > > I think one example provided in the above reference ticket is Russell ran > > into some issue in the past with this thus the strong preference over > > docker based testing. @Russell, is this something you can provide more > > insights with? > > > > Thanks, > > Yong Zheng > > > > > > On 2026/06/30 16:55:42 Dmitri Bourlatchkov wrote: > > > Hi All, > > > > > > +1 to Robert's points. > > > > > > Testing just the "fat" client jar in CI looks sufficient to me. This jar > > > should expose the same range of class-loading issues that may occur with > > > the "thin" jar with dependencies resolved via Maven/Ivy. > > > > > > Additionally, I think Gradle-based tests are much simpler to debug and > > > evolve than Docker-based tests. > > > > > > Cheers, > > > Dmitri. > > > > > > On Tue, Jun 30, 2026 at 6:25 AM Robert Stupp <[email protected]> wrote: > > > > > > > Hi all, > > > > > > > > I think the “this only takes ~2m30s” argument is a bit distracting. > > > > > > > > The question should not be whether one CI section is currently small > > > > compared to other CI sections. > > > > The question should be: what concrete failure mode does this test catch > > > > that we cannot catch with a cheaper and more targeted test? > > > > > > > > GitHub runner time is still a shared ASF resource. > > > > Even a few minutes matter when they run on many PRs, retries, > > main/release > > > > branches, and then get copied again for Spark 4 or future Spark > > versions. > > > > So I think every required PR test should have a clear purpose and a > > clear > > > > failure mode it protects against. > > > > > > > > For the Spark plugin regtest, I am still missing that concrete > > > > justification. > > > > > > > > If the concern is the bundle jar, then I agree we should test that the > > > > bundle jar loads in an isolated Spark-like runtime and can create/use > > the > > > > Polaris catalog. > > > > That seems valuable, and the JUnit/Gradle test looks like a good fit > > for > > > > that. > > > > > > > > If the concern is `--packages` / Maven resolution, I am less convinced > > this > > > > belongs in required PR CI. > > > > Polaris appears to direct users to the packaged Spark client artifacts, > > > > especially the bundle jar, for example on the 1.5.0 downloads page. > > > > Testing Maven/Ivy resolution through `publishToMavenLocal` also has > > real > > > > costs: it mutates the developer's global `~/.m2`, interacts badly with > > > > project isolation, and is not great for build cacheability. > > > > > > > > Also, the risk of “broken generated POM metadata” seems very low. > > > > If we really care about that, we can check the publication metadata > > > > directly without launching a Docker/Spark workflow. > > > > > > > > So my concrete question is: > > > > Has the Docker-based Spark plugin regtest caught specific regressions > > that > > > > the proposed isolated JUnit/Gradle test would not have caught? > > > > > > > > Examples would help a lot here: broken dependency metadata, a real > > > > `spark-submit --packages` failure, a bundle/classpath issue, or some > > > > launcher behavior that only the Docker test exposed. > > > > Without that evidence, “it is closer to the user workflow” feels too > > broad > > > > to justify keeping it as a required PR gate. > > > > > > > > My preference would be: > > > > > > > > * keep required PR CI focused on targeted tests for the bundle jar and > > > > Spark > > > > catalog behavior; > > > > * avoid `publishToMavenLocal` and global `~/.m2` mutation in normal PR > > > > tests; > > > > * if people still want full shell/Docker coverage, run it periodically > > or > > > > as a > > > > manual workflow until we have evidence that it catches unique > > > > regressions. > > > > > > > > That gives us Spark 4 coverage without making Docker-based end-to-end > > > > testing the default answer for every Spark version. > > > > > > > > Robert > > > > > > > > On Tue, Jun 30, 2026 at 1:27 AM Yufei Gu <[email protected]> wrote: > > > > > > > > > Thanks for raising this, Yong! I agree that we need tests for Spark > > 4. > > > > > > > > > > I agree with what Yun said here. > > > > > > > > > > To add to that, the current regression tests against MinIO/RustFS > > cover > > > > > both the Spark Plugin Regression Test and the top level Regression > > Test. > > > > > These used to be separate CI workflows(merged in PR 3625), and I > > think we > > > > > should keep them separate. > > > > > > > > > > The Spark Plugin Regression Test does not need to connect to a > > storage > > > > > system such as S3, MinIO, or RustFS. It primarily serves as a smoke > > test > > > > to > > > > > verify the Polaris packaging and Spark deployment. I think we should > > > > > restore the previous setup where these workflows are separated. That > > > > would > > > > > also reduce the overall CI duration, since they can run in parallel. > > > > > > > > > > [image: Screenshot 2026-06-29 at 4.15.57 PM.png] > > > > > [image: Screenshot 2026-06-29 at 4.16.14 PM.png] > > > > > > > > > > Yufei > > > > > > > > > > > > > > > On Mon, Jun 29, 2026 at 4:07 PM yun zou <[email protected]> > > > > > wrote: > > > > > > > > > >> Hi Yong Zheng, > > > > >> > > > > >> Thanks for bringing this up! In short, I don't think it's worth the > > > > effort > > > > >> to make this conversion at the moment for the following reasons: > > > > >> > > > > >> 1. *It doesn't meaningfully improve CI time.* I think you > > mentioned > > > > >> this > > > > >> in the thread as well. Looking at one CI run as an example ( > > > > >> > > > > >> > > > > > > https://github.com/apache/polaris/actions/runs/24255532169/job/70826005994 > > > > >> ), > > > > >> the Spark Regression Test section only takes about *2m 35s*. > > Even if > > > > we > > > > >> add another Spark 4.x regression test, I don't think it would > > > > >> significantly > > > > >> increase the overall CI time—probably just another 2–3 minutes. > > The > > > > >> Runtime > > > > >> Service tests are still the slowest part of the pipeline, and > > their > > > > >> execution time is likely to continue growing. > > > > >> 2. *The regression tests provide a high level of confidence in > > > > >> correctness.* They remain the tests that most closely resemble > > our > > > > >> customers' actual environments, making them our last line of > > defense > > > > >> against regressions. That gives them significant value. Rather > > than > > > > >> spending effort trying to build simulations that provide similar > > > > >> coverage, > > > > >> I think it's better to keep these regression tests in place since > > > > they > > > > >> validate the real end-to-end behavior. > > > > >> > > > > >> Those are my thoughts, but I'm happy to discuss further if you see > > > > >> additional benefits that I'm missing. > > > > >> > > > > >> Best Regards, > > > > >> Yun > > > > >> > > > > >> On Sun, Jun 28, 2026 at 8:47 PM Yong Zheng <[email protected]> > > wrote: > > > > >> > > > > >> > Hello, > > > > >> > > > > > >> > Follow-up to the regtest thread ( > > > > >> > https://lists.apache.org/thread/4bx31cfbcqfxzgpsddvc9kcfbn9l093y) > > and > > > > >> > current PR (https://github.com/apache/polaris/pull/4588). > > > > >> > > > > > >> > Currently we support both Spark 3 ( > > > > >> > https://github.com/apache/polaris/tree/main/plugins/spark/v3.5) > > and > > > > 4 ( > > > > >> > https://github.com/apache/polaris/tree/main/plugins/spark/v4.0) > > for > > > > >> > Polaris spark client, however, only spark 3 has regtests. There > > was a > > > > >> > concern with potentially increasing CI time, however, this later > > got > > > > >> proved > > > > >> > to be not the case as "moving > > > > >> > regtests to integration tests would not necessarily save time. In > > > > fact, > > > > >> it > > > > >> > could potentially increase overall CI duration, since the longest > > > > >> running > > > > >> > workflows are currently not the regtests". > > > > >> > > > > > >> > Before we can finalize the testing strategy for Polaris spark > > client, > > > > we > > > > >> > need to decide if we want to proceed with the conversion (from > > docker > > > > >> based > > > > >> > to JUnit based). The lack of regtests for spark 4 can potentially > > > > cause > > > > >> > regression issues later. > > > > >> > > > > > >> > Also, as we are using JUnit, we can't trigger a actual > > 'spark-shell > > > > >> xxxx' > > > > >> > to simulate the actual `--packages` and `--jars`. > > > > >> > > > > > >> > However, we can kind get them working by using `URLClassLoader` > > for > > > > >> > `--jars` and `SparkSubmitUtils.resolvedMavenCoordinates` for > > > > >> `--packages`. > > > > >> > The catch here is to be able to use `--packages`, we will need to > > > > >> > `publishToMavenLocal` (which is project-isolation violation, as it > > > > will > > > > >> try > > > > >> > to modify `~/.m2`). The suggest is to drop this test and only > > handle > > > > >> bundle > > > > >> > jar via `URLClassLoader`. > > > > >> > > > > > >> > I am wondering how team would like to proceed as we can't leave > > spark > > > > 4 > > > > >> > out there without proper JUnit for a long period of time. > > > > >> > > > > > >> > Thanks, > > > > >> > Yong Zheng > > > > >> > > > > > >> > > > > > > > > > > > > > > >
