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