From e82a3469f5dd7b5459009cd0ccf087c6e2707e45 Mon Sep 17 00:00:00 2001 From: kary zheng Date: Fri, 31 Jul 2026 19:40:09 -0700 Subject: [PATCH 1/2] build: let WorkflowOperator route Python-executing tests to amber-integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `amber` job runs this module's test suite and installs no Python packages. The runner's interpreter is there — PythonCodeRawInvalidTextSpec uses it for a stdlib-only py_compile check — but pandas and plotly are not importable, so a test that executes an operator's generated template rather than only compiling it fails its dependency probe and cancels. A cancellation is neither a pass nor a failure, so the suite still reports "All tests passed" and the missing coverage leaves no trace in the build. This gives WorkflowOperator the split amber already has, reading the same AMBER_TEST_FILTER the amber job sets on the step that invokes WorkflowOperator/jacoco, so that job needs no change for the exclusion to take effect. amber's tag cannot be reused: it lives in amber/src/test/integration, and amber depends on WorkflowOperator, not the other way around. With the selection logic now wanted in two modules it moves to project/TestFilters.scala, beside the build's other shared sbt helpers, and each module passes its own env var and tag. PythonCodeRawInvalidTextSpec gains a tagged case asserting that pandas and plotly import in the interpreter it already resolves: py_compile only parses the emitted code, while running it needs the packages it imports. Being the first user of the tag it also exercises the routing, and it turns a missing install in amber-integration into a failure rather than the silent cancellation above. Elsewhere a bare interpreter is a local-setup fact, so it cancels instead. Verified in all four states: with the packages present the case passes under integration-only; under skip-integration it is excluded while the spec's two existing tests still run; pointed at a bare interpreter it fails under integration-only and cancels with no filter set. Both modules' testOptions were compared under all three env values before and after the extraction. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/build.yml | 3 ++ amber/build.sbt | 20 +++----- common/workflow-operator/build.sbt | 8 ++++ .../amber/operator/tags/IntegrationTest.java | 46 +++++++++++++++++++ .../util/PythonCodeRawInvalidTextSpec.scala | 27 +++++++++++ project/TestFilters.scala | 46 +++++++++++++++++++ 6 files changed, 137 insertions(+), 13 deletions(-) create mode 100644 common/workflow-operator/src/test/java/org/apache/texera/amber/operator/tags/IntegrationTest.java create mode 100644 project/TestFilters.scala diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a82cbfde95b..ccb6cbc003f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -620,6 +620,8 @@ jobs: # specs. The Java @TagAnnotation makes the marker visible to # ScalaTest's reflection, so `-n TAG` correctly narrows the # run. + # WorkflowOperator/test is here for the same reason, with its own tag: + # its Python-dependent tests need packages the `amber` job lacks. # # scalafmtCheckAll + scalafixAll --check are run here as well # because an integration-only PR fires only the @@ -646,6 +648,7 @@ jobs: run: | sbt scalafmtCheckAll \ "scalafixAll --check" \ + "WorkflowOperator/test" \ "WorkflowExecutionService/test" - name: Build amber dist for the texera-web boot test # Boot smoke test for texera-web (mirrors platform-integration for the diff --git a/amber/build.sbt b/amber/build.sbt index 769bcb15bd1..2e4d180460e 100644 --- a/amber/build.sbt +++ b/amber/build.sbt @@ -69,19 +69,13 @@ Test / unmanagedSourceDirectories += baseDirectory.value / "src" / "test" / "int // scalafix still cover it and `sbt Test/runMain` can invoke benches. Test / unmanagedSourceDirectories += baseDirectory.value / "src" / "bench" / "scala" -// Test-filter switch driven by the AMBER_TEST_FILTER env var so the -// amber and amber-integration CI jobs select disjoint subsets without -// each invocation having to embed a `set Tests.Argument(...)` prefix. -// skip-integration : exclude @IntegrationTest-tagged specs (amber job) -// integration-only : include only @IntegrationTest-tagged specs (amber-integration job) -// (unset) : run everything (default for local sbt) -Test / testOptions ++= (sys.env.get("AMBER_TEST_FILTER") match { - case Some("skip-integration") => - Seq(Tests.Argument(TestFrameworks.ScalaTest, "-l", "org.apache.texera.amber.tags.IntegrationTest")) - case Some("integration-only") => - Seq(Tests.Argument(TestFrameworks.ScalaTest, "-n", "org.apache.texera.amber.tags.IntegrationTest")) - case _ => Nil -}) +// Lets the amber and amber-integration CI jobs select disjoint subsets without +// each invocation having to embed a `set Tests.Argument(...)` prefix. See +// project/TestFilters.scala. +Test / testOptions ++= TestFilters.integrationSplit( + envVar = "AMBER_TEST_FILTER", + tag = "org.apache.texera.amber.tags.IntegrationTest" +) // Excluding some proto files: PB.generate / excludeFilter := "scalapb.proto" diff --git a/common/workflow-operator/build.sbt b/common/workflow-operator/build.sbt index d1cd1d4cc35..b2eb9565d44 100644 --- a/common/workflow-operator/build.sbt +++ b/common/workflow-operator/build.sbt @@ -35,6 +35,14 @@ ThisBuild / conflictManager := ConflictManager.latestRevision // Restrict parallel execution of tests to avoid conflicts Global / concurrentRestrictions += Tags.limit(Tags.Test, 1) +// A test needing more than a bare Python interpreter is tagged, so the amber job +// excludes it and amber-integration, which installs operator-requirements.txt, +// runs it. The amber job already sets this env var on the step that invokes +// WorkflowOperator/jacoco, so no workflow change is needed for the exclusion. +Test / testOptions ++= TestFilters.integrationSplit( + envVar = "AMBER_TEST_FILTER", + tag = "org.apache.texera.amber.operator.tags.IntegrationTest" +) ///////////////////////////////////////////////////////////////////////////// // Compiler Options diff --git a/common/workflow-operator/src/test/java/org/apache/texera/amber/operator/tags/IntegrationTest.java b/common/workflow-operator/src/test/java/org/apache/texera/amber/operator/tags/IntegrationTest.java new file mode 100644 index 00000000000..ac0cd2346d3 --- /dev/null +++ b/common/workflow-operator/src/test/java/org/apache/texera/amber/operator/tags/IntegrationTest.java @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.amber.operator.tags; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.scalatest.TagAnnotation; + +/** + * Marks a test in this module as needing more than a bare Python interpreter — + * pandas or plotly, which the {@code amber} job does not install. See the + * AMBER_TEST_FILTER block in {@code common/workflow-operator/build.sbt} for how + * it routes to {@code amber-integration}. + * + *

amber's own tag lives in {@code amber/src/test/integration} and is not + * reachable here, since amber depends on this module rather than the reverse. + * + *

Java, not Scala: ScalaTest finds tag annotations by + * {@code java.lang.annotation} reflection, and a Scala {@code StaticAnnotation} + * produces no JVM annotation interface for {@code @TagAnnotation} to mark. + */ +@TagAnnotation +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.METHOD, ElementType.TYPE}) +public @interface IntegrationTest { +} diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/util/PythonCodeRawInvalidTextSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/util/PythonCodeRawInvalidTextSpec.scala index 122b1dbae8b..ab4ca5048ff 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/util/PythonCodeRawInvalidTextSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/util/PythonCodeRawInvalidTextSpec.scala @@ -21,8 +21,10 @@ package org.apache.texera.amber.util import com.typesafe.config.ConfigFactory import org.apache.texera.amber.operator.PythonOperatorDescriptor +import org.apache.texera.amber.operator.tags.IntegrationTest import org.apache.texera.amber.pybuilder.PythonReflectionTextUtils.truncateBlock import org.apache.texera.amber.pybuilder.PythonReflectionUtils +import org.scalatest.Tag import org.scalatest.funsuite.AnyFunSuite import java.nio.charset.StandardCharsets @@ -263,4 +265,29 @@ final class PythonCodeRawInvalidTextSpec extends AnyFunSuite { } } + /** py_compile above only parses the emitted code; running it needs the packages + * it imports. Tagged, so only amber-integration — the job that installs them — + * runs this. There a missing package is a defect; elsewhere it is a local-setup + * fact, so cancel rather than fail. + */ + test( + "the Python interpreter operator templates run in should import pandas and plotly", + Tag(classOf[IntegrationTest].getName) + ) { + val provisioned = sys.env.get("AMBER_TEST_FILTER").contains("integration-only") + def unavailable(message: String): Nothing = + if (provisioned) fail(message) else cancel(message) + + val python = loadPythonExeFromUdfConf().getOrElse(unavailable("no runnable python")) + val imported = Try { + val process = new ProcessBuilder(python, "-c", "import pandas, plotly") + .redirectErrorStream(true) + .start() + process.waitFor(60, TimeUnit.SECONDS) && process.exitValue() == 0 + } + if (!imported.getOrElse(false)) { + unavailable(s"'$python' cannot import pandas and plotly") + } + } + } diff --git a/project/TestFilters.scala b/project/TestFilters.scala new file mode 100644 index 00000000000..ed73d272200 --- /dev/null +++ b/project/TestFilters.scala @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import sbt._ + +/** + * Selects a module's tagged tests for the fast-unit job or the integration job: + * skip-integration excludes them, integration-only runs only them, unset runs + * everything. Shared because the mapping is identical in every module, while the + * env var and the tag are not — the tag annotation has to live somewhere the + * module's own Test config can see. + */ +object TestFilters { + + /** @param integrationOnlyExtra further ScalaTest args for the integration side, + * e.g. "-P4" to bound its pool when a suite forks + * a process per test. + */ + def integrationSplit( + envVar: String, + tag: String, + integrationOnlyExtra: Seq[String] = Seq.empty + ): Seq[TestOption] = + sys.env.get(envVar) match { + case Some("skip-integration") => + Seq(Tests.Argument(TestFrameworks.ScalaTest, "-l", tag)) + case Some("integration-only") => + Seq(Tests.Argument(TestFrameworks.ScalaTest, Seq("-n", tag) ++ integrationOnlyExtra: _*)) + case _ => Nil + } +} From 5fa413e808faff46039d52939133384716e47741 Mon Sep 17 00:00:00 2001 From: kary zheng Date: Fri, 31 Jul 2026 20:02:37 -0700 Subject: [PATCH 2/2] docs(workflow-operator): say how to apply the IntegrationTest tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The javadoc explained what the tag means and how the build routes it, but not how a test opts in — and the two ScalaTest styles in this module take the tag in different positions. Naming both, and noting that per-case tagging keeps a spec's cheaper assertions in the unit job and its coverage report, saves the next reader a trip through build.sbt and ScalaTest's docs. Co-Authored-By: Claude Opus 5 (1M context) --- .../apache/texera/amber/operator/tags/IntegrationTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common/workflow-operator/src/test/java/org/apache/texera/amber/operator/tags/IntegrationTest.java b/common/workflow-operator/src/test/java/org/apache/texera/amber/operator/tags/IntegrationTest.java index ac0cd2346d3..448a051d834 100644 --- a/common/workflow-operator/src/test/java/org/apache/texera/amber/operator/tags/IntegrationTest.java +++ b/common/workflow-operator/src/test/java/org/apache/texera/amber/operator/tags/IntegrationTest.java @@ -32,6 +32,11 @@ * AMBER_TEST_FILTER block in {@code common/workflow-operator/build.sbt} for how * it routes to {@code amber-integration}. * + *

Apply it to a whole spec as an annotation, or to a single case — {@code + * test(name, Tag(classOf[IntegrationTest].getName))} in a FunSuite, + * {@code taggedAs} in a FlatSpec — so that a spec's cheaper assertions stay in + * the unit job and its coverage report. + * *

amber's own tag lives in {@code amber/src/test/integration} and is not * reachable here, since amber depends on this module rather than the reverse. *