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 > > > >> > > > > >> > > > > > > > > > >
