Hi All, Robert's proposal looks good to me... except that I'm not sure what "narrow Spark launcher test for `--packages`" means :) but I'd be fine with just reviewing that part in a PR.
Cheers, Dmitri. On Mon, Jul 13, 2026 at 10:52 AM Robert Stupp <[email protected]> wrote: > 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 > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > >
