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