Hi Yong, Yun's example helps, because it does point to a real failure mode: Spark's `--packages` path can catch dependency-resolution problems that a simple `--jars` or bundle-jar test might miss.
But I am not sure that leads all the way to "copy the current Docker-based regtests for Spark 4". I think there are a few different concerns mixed together here: * Is the generated POM / dependency metadata correct? * Does the bundle/shadow jar load and work in a Spark-like runtime? * Does Spark's real `--packages` path resolve and launch the client correctly? The first one can be checked by inspecting the generated metadata and resolved dependency graph. The second one seems like a good fit for the JUnit/Gradle test. The third one may still need a real Spark launcher smoke test, but I think that test should be very small and explicit. The part I am still not comfortable with is using `publishToMavenLocal` in a required PR test. It mutates global `~/.m2`, which is bad for project isolation, and Maven local is not the same as Maven Central anyway. So even if the test passes there, it is not exactly proving the same thing users will see when they resolve released artifacts. My preference would be to keep the JUnit/Gradle test for the bundle jar and Spark catalog behavior, add direct metadata/dependency checks if needed, and only keep a narrow Spark launcher test for `--packages` if we agree that this specific path needs PR coverage. I am definitely not against Spark 4 coverage. I just think the required PR tests should be matched to the specific failure modes we want to catch, rather than copying the whole Docker regtest setup by default. Robert On Sat, Jul 11, 2026 at 8:37 PM Yong Zheng <[email protected]> wrote: > 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 > > > > > >> > > > > > > >> > > > > > > > > > > > > > > > > > > > > >
