This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-5770-97480f16c1eacd95985f9339de3dc74637cf8c18 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 5e23f22add77a700049aa53526544fe69ea43ca7 Author: Xinyuan Lin <[email protected]> AuthorDate: Fri Jun 19 15:58:56 2026 -0700 test(workflow-operator): add unit test coverage for source-descriptor abstractions and small Jackson config bags (#5770) ### What changes were proposed in this PR? Pin behavior of four small contract / config classes in `common/workflow-operator/`. Each is too thin to justify its own PR but the bundle stays cohesive (everything sits in the descriptor / metadata surface). No production-code changes. | Spec | Source class | Tests | | --- | --- | --- | | `SourceOperatorDescriptorSpec` | `SourceOperatorDescriptor` (abstract) | 4 | | `PythonSourceOperatorDescriptorSpec` | `PythonSourceOperatorDescriptor` (abstract) | 5 | | `GaugeChartStepsSpec` | `GaugeChartSteps` (Jackson bag) | 7 | | `DummyPropertiesSpec` | `DummyProperties` (Jackson bag) | 7 | All four spec files follow the `<srcClassName>Spec.scala` one-to-one convention. **Behavior pinned — `SourceOperatorDescriptor`** | Surface | Contract | | --- | --- | | `sourceSchema()` | declared as an abstract member; pinned via a minimal test-only concrete subclass | | `LogicalOp` inheritance | upcast compiles; `case _: LogicalOp` and `case _: SourceOperatorDescriptor` both match a concrete instance | **Behavior pinned — `PythonSourceOperatorDescriptor`** | Surface | Contract | | --- | --- | | Composition | a concrete subclass IS a `SourceOperatorDescriptor` AND a `PythonOperatorDescriptor` (compile-time enforced + four-way `isInstanceOf` check) | | Inherited defaults | `parallelizable() == false` and `asSource() == false` (the documented base defaults) when the subclass does not override | **Behavior pinned — `GaugeChartSteps`** | Surface | Contract | | --- | --- | | Defaults | `start == ""` and `end == ""` on a fresh instance | | Mutability | both fields are `var`-assignable post-construction | | JSON wire-keys | serialize under `start` / `end` (Jackson tree-API verified) | | JSON round-trip | preserves both fields | | Annotations | `@JsonProperty("start")` and `@JsonProperty("end")` on the corresponding fields (verified via reflection) | | Instance independence | no static state shared across `new` | **Behavior pinned — `DummyProperties`** | Surface | Contract | | --- | --- | | Defaults | `dummyProperty == ""` and `dummyValue == ""` | | Mutability | both fields are `var`-assignable | | JSON round-trip | preserves both fields (including the default-empty round-trip) | | Annotations | `@JsonProperty` present on both fields | | Instance independence | no static state shared across `new` | ### Any related issues, documentation, discussions? Closes #5767. ### How was this PR tested? Pure unit-test additions; verified locally with: - `sbt "WorkflowOperator/testOnly org.apache.texera.amber.operator.source.SourceOperatorDescriptorSpec org.apache.texera.amber.operator.source.PythonSourceOperatorDescriptorSpec org.apache.texera.amber.operator.visualization.gaugeChart.GaugeChartStepsSpec org.apache.texera.amber.operator.DummyPropertiesSpec"` — 23 tests, all green - `sbt scalafmtCheckAll` — clean - CI to confirm ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.7 [1M context]) --- .../amber/operator/DummyPropertiesSpec.scala | 103 ++++++++++++++++++++ .../PythonSourceOperatorDescriptorSpec.scala | 75 ++++++++++++++ .../source/SourceOperatorDescriptorSpec.scala | 64 ++++++++++++ .../gaugeChart/GaugeChartStepsSpec.scala | 108 +++++++++++++++++++++ .../amber/testsupport/source/SourceStubs.scala | 77 +++++++++++++++ 5 files changed, 427 insertions(+) diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/DummyPropertiesSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/DummyPropertiesSpec.scala new file mode 100644 index 0000000000..d96f73db87 --- /dev/null +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/DummyPropertiesSpec.scala @@ -0,0 +1,103 @@ +/* + * 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 + +import com.fasterxml.jackson.annotation.JsonProperty +import org.apache.texera.amber.util.JSONUtils.objectMapper +import org.scalatest.flatspec.AnyFlatSpec + +class DummyPropertiesSpec extends AnyFlatSpec { + + // --------------------------------------------------------------------------- + // Defaults + // --------------------------------------------------------------------------- + + "DummyProperties" should "default dummyProperty and dummyValue to the empty string" in { + val d = new DummyProperties + assert(d.dummyProperty == "") + assert(d.dummyValue == "") + } + + // --------------------------------------------------------------------------- + // Mutability + // --------------------------------------------------------------------------- + + it should "allow both fields to be assigned post-construction" in { + val d = new DummyProperties + d.dummyProperty = "p" + d.dummyValue = "v" + assert(d.dummyProperty == "p") + assert(d.dummyValue == "v") + } + + // --------------------------------------------------------------------------- + // JSON round-trip + // --------------------------------------------------------------------------- + + "DummyProperties JSON round-trip" should "preserve both fields" in { + val d = new DummyProperties + d.dummyProperty = "hello" + d.dummyValue = "world" + val restored = objectMapper.readValue( + objectMapper.writeValueAsString(d), + classOf[DummyProperties] + ) + assert(restored.dummyProperty == "hello") + assert(restored.dummyValue == "world") + } + + it should "round-trip default (empty) values cleanly" in { + val restored = objectMapper.readValue( + objectMapper.writeValueAsString(new DummyProperties), + classOf[DummyProperties] + ) + assert(restored.dummyProperty == "") + assert(restored.dummyValue == "") + } + + // --------------------------------------------------------------------------- + // Annotations + // --------------------------------------------------------------------------- + + "DummyProperties#dummyProperty" should "carry @JsonProperty" in { + val jp = classOf[DummyProperties] + .getDeclaredField("dummyProperty") + .getAnnotation(classOf[JsonProperty]) + assert(jp != null) + } + + "DummyProperties#dummyValue" should "carry @JsonProperty" in { + val jp = classOf[DummyProperties] + .getDeclaredField("dummyValue") + .getAnnotation(classOf[JsonProperty]) + assert(jp != null) + } + + // --------------------------------------------------------------------------- + // Instance independence + // --------------------------------------------------------------------------- + + it should "construct two independent instances (no static state shared)" in { + val a = new DummyProperties + val b = new DummyProperties + a.dummyProperty = "first" + assert(b.dummyProperty == "") + } +} diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/PythonSourceOperatorDescriptorSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/PythonSourceOperatorDescriptorSpec.scala new file mode 100644 index 0000000000..797ee8bb94 --- /dev/null +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/PythonSourceOperatorDescriptorSpec.scala @@ -0,0 +1,75 @@ +/* + * 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.source + +import org.apache.texera.amber.operator.{LogicalOp, PythonOperatorDescriptor} +import org.apache.texera.amber.testsupport.source.{SourceStubs, StubPythonSource} +import org.scalatest.flatspec.AnyFlatSpec + +class PythonSourceOperatorDescriptorSpec extends AnyFlatSpec { + + // --------------------------------------------------------------------------- + // Composition — extends BOTH SourceOperatorDescriptor and + // PythonOperatorDescriptor + // --------------------------------------------------------------------------- + + "PythonSourceOperatorDescriptor (concrete subclass)" should + "be a SourceOperatorDescriptor (compile-time enforced)" in { + val s: SourceOperatorDescriptor = new StubPythonSource + assert(s.sourceSchema() == SourceStubs.testSchema) + } + + it should "be a PythonOperatorDescriptor (compile-time enforced)" in { + val s: PythonOperatorDescriptor = new StubPythonSource + assert(s.generatePythonCode() == "yield {'col': 'value'}") + } + + it should "be a LogicalOp (transitively, via SourceOperatorDescriptor)" in { + val s: LogicalOp = new StubPythonSource + assert(s != null) + } + + // --------------------------------------------------------------------------- + // Type-pattern matching — every layer of the composition is reachable + // --------------------------------------------------------------------------- + + it should "match every type in the composition via pattern-matching" in { + val any: AnyRef = new StubPythonSource + assert(any.isInstanceOf[PythonSourceOperatorDescriptor]) + assert(any.isInstanceOf[SourceOperatorDescriptor]) + assert(any.isInstanceOf[PythonOperatorDescriptor]) + assert(any.isInstanceOf[LogicalOp]) + } + + // --------------------------------------------------------------------------- + // Defaults inherited from PythonOperatorDescriptor (no override) + // --------------------------------------------------------------------------- + + "PythonSourceOperatorDescriptor inherited defaults" should + "default `parallelizable()` to false and `asSource()` to false unless overridden" in { + val s = new StubPythonSource + // Both are open methods on PythonOperatorDescriptor with the + // documented `false` default. A concrete Python source typically + // overrides `asSource()` to true; the stub does not, so the + // default surfaces here. + assert(!s.parallelizable()) + assert(!s.asSource()) + } +} diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/SourceOperatorDescriptorSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/SourceOperatorDescriptorSpec.scala new file mode 100644 index 0000000000..23228caaaf --- /dev/null +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/SourceOperatorDescriptorSpec.scala @@ -0,0 +1,64 @@ +/* + * 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.source + +import org.apache.texera.amber.operator.LogicalOp +import org.apache.texera.amber.testsupport.source.{SourceStubs, StubSource} +import org.scalatest.flatspec.AnyFlatSpec + +class SourceOperatorDescriptorSpec extends AnyFlatSpec { + + // --------------------------------------------------------------------------- + // sourceSchema — abstract member is observable + // --------------------------------------------------------------------------- + + "SourceOperatorDescriptor (concrete subclass)" should + "expose the `sourceSchema()` value supplied by the impl" in { + val s = new StubSource + assert(s.sourceSchema() == SourceStubs.testSchema) + } + + // --------------------------------------------------------------------------- + // Inheritance — SourceOperatorDescriptor is a LogicalOp + // --------------------------------------------------------------------------- + + it should "be a LogicalOp (compile-time enforced)" in { + val s: LogicalOp = new StubSource + assert(s != null) + } + + it should "match the LogicalOp type-pattern" in { + val any: AnyRef = new StubSource + val matched = any match { + case _: LogicalOp => true + case _ => false + } + assert(matched) + } + + it should "match the SourceOperatorDescriptor type-pattern" in { + val any: AnyRef = new StubSource + val matched = any match { + case _: SourceOperatorDescriptor => true + case _ => false + } + assert(matched) + } +} diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartStepsSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartStepsSpec.scala new file mode 100644 index 0000000000..840bd425c2 --- /dev/null +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/gaugeChart/GaugeChartStepsSpec.scala @@ -0,0 +1,108 @@ +/* + * 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.visualization.gaugeChart + +import com.fasterxml.jackson.annotation.JsonProperty +import org.apache.texera.amber.util.JSONUtils.objectMapper +import org.scalatest.flatspec.AnyFlatSpec + +class GaugeChartStepsSpec extends AnyFlatSpec { + + // --------------------------------------------------------------------------- + // Defaults + // --------------------------------------------------------------------------- + + "GaugeChartSteps" should "default start and end to the empty string" in { + val s = new GaugeChartSteps + assert(s.start == "") + assert(s.end == "") + } + + // --------------------------------------------------------------------------- + // Mutability + // --------------------------------------------------------------------------- + + it should "allow start and end to be assigned post-construction" in { + val s = new GaugeChartSteps + s.start = "10" + s.end = "90" + assert(s.start == "10") + assert(s.end == "90") + } + + // --------------------------------------------------------------------------- + // JSON round-trip — wire keys are `start` / `end` + // --------------------------------------------------------------------------- + + "GaugeChartSteps JSON round-trip" should + "serialize start and end under the canonical wire keys" in { + val s = new GaugeChartSteps + s.start = "low" + s.end = "high" + val tree = objectMapper.readTree(objectMapper.writeValueAsString(s)) + assert(tree.has("start")) + assert(tree.get("start").asText() == "low") + assert(tree.has("end")) + assert(tree.get("end").asText() == "high") + } + + it should "round-trip both fields cleanly" in { + val s = new GaugeChartSteps + s.start = "0" + s.end = "100" + val restored = objectMapper.readValue( + objectMapper.writeValueAsString(s), + classOf[GaugeChartSteps] + ) + assert(restored.start == "0") + assert(restored.end == "100") + } + + // --------------------------------------------------------------------------- + // Annotations + // --------------------------------------------------------------------------- + + "GaugeChartSteps#start" should "carry @JsonProperty(\"start\")" in { + val jp = classOf[GaugeChartSteps] + .getDeclaredField("start") + .getAnnotation(classOf[JsonProperty]) + assert(jp != null) + assert(jp.value == "start") + } + + "GaugeChartSteps#end" should "carry @JsonProperty(\"end\")" in { + val jp = classOf[GaugeChartSteps] + .getDeclaredField("end") + .getAnnotation(classOf[JsonProperty]) + assert(jp != null) + assert(jp.value == "end") + } + + // --------------------------------------------------------------------------- + // Instance independence + // --------------------------------------------------------------------------- + + it should "construct two independent instances (no static state shared)" in { + val a = new GaugeChartSteps + val b = new GaugeChartSteps + a.start = "1" + assert(b.start == "") + } +} diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/testsupport/source/SourceStubs.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/testsupport/source/SourceStubs.scala new file mode 100644 index 0000000000..80c4d73da3 --- /dev/null +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/testsupport/source/SourceStubs.scala @@ -0,0 +1,77 @@ +/* + * 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.testsupport.source + +import org.apache.texera.amber.core.tuple.{Attribute, AttributeType, Schema} +import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, WorkflowIdentity} +import org.apache.texera.amber.core.workflow.{PhysicalOp, PortIdentity} +import org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, OperatorInfo} +import org.apache.texera.amber.operator.source.{ + PythonSourceOperatorDescriptor, + SourceOperatorDescriptor +} + +// Both stubs deliberately live outside `org.apache.texera.amber.operator`: +// PythonReflectionUtils.scanCandidates scopes itself to that package prefix +// (see PythonCodeRawInvalidTextSpec / SklearnOpDescRegistrySpec) and would +// otherwise try to instantiate and codegen these test-only fixtures. + +object SourceStubs { + val testSchema: Schema = + Schema().add(new Attribute("col", AttributeType.STRING)) +} + +/** Minimal concrete `SourceOperatorDescriptor` for contract tests. */ +class StubSource extends SourceOperatorDescriptor { + override def sourceSchema(): Schema = SourceStubs.testSchema + override def operatorInfo: OperatorInfo = + OperatorInfo( + "Stub", + "stub source", + OperatorGroupConstants.INPUT_GROUP, + inputPorts = List.empty, + outputPorts = List.empty + ) + override def getPhysicalOp( + workflowId: WorkflowIdentity, + executionId: ExecutionIdentity + ): PhysicalOp = + throw new NotImplementedError( + "getPhysicalOp is not needed for the SourceOperatorDescriptor contract test" + ) +} + +/** Minimal concrete `PythonSourceOperatorDescriptor` for composition tests. */ +class StubPythonSource extends PythonSourceOperatorDescriptor { + override def sourceSchema(): Schema = SourceStubs.testSchema + override def generatePythonCode(): String = "yield {'col': 'value'}" + override def getOutputSchemas( + inputSchemas: Map[PortIdentity, Schema] + ): Map[PortIdentity, Schema] = + Map(PortIdentity() -> SourceStubs.testSchema) + override def operatorInfo: OperatorInfo = + OperatorInfo( + "StubPySrc", + "stub python source", + OperatorGroupConstants.INPUT_GROUP, + inputPorts = List.empty, + outputPorts = List.empty + ) +}
