Yicong-Huang commented on code in PR #4809:
URL: https://github.com/apache/texera/pull/4809#discussion_r3177582216
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/ImageViz/ImageVisualizerOpDescSpec.scala:
##########
@@ -19,18 +19,61 @@
package org.apache.texera.amber.operator.visualization.ImageViz
+import org.apache.texera.amber.core.tuple.AttributeType
+import org.apache.texera.amber.operator.metadata.OperatorGroupConstants
import org.scalatest.BeforeAndAfter
import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
-class ImageVisualizerOpDescSpec extends AnyFlatSpec with BeforeAndAfter {
+class ImageVisualizerOpDescSpec extends AnyFlatSpec with BeforeAndAfter with
Matchers {
var opDesc: ImageVisualizerOpDesc = _
before {
opDesc = new ImageVisualizerOpDesc()
}
it should "throw assertion error if BinaryContent is empty" in {
+ // Pin: `binaryContent` is declared `var binaryContent: EncodableString =
_`
+ // — an uninitialized reference field defaults to null, so the
+ // `assert(binaryContent.nonEmpty)` inside `createBinaryData` reaches
+ // `null.nonEmpty` and throws NullPointerException before the assert
+ // message can fire. Other OpDescs that initialize their fields to `""`
+ // hit the assert path and raise AssertionError instead.
assertThrows[NullPointerException] {
Review Comment:
Restructured in 9b9f7a4863: split into two tests. The first now reads
`should currently throw NullPointerException when binaryContent is
uninitialized` so the title no longer claims this is the contract. The second
is a `pendingUntilFixed` that asserts the intended controlled-error contract
(`AssertionError`) — currently Pending, will deliberately fail once a
validation fix lands and force removal of the marker.
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/volcanoPlot/VolcanoPlotOpDescSpec.scala:
##########
@@ -0,0 +1,81 @@
+/*
+ * 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.volcanoPlot
+
+import org.apache.texera.amber.core.tuple.AttributeType
+import org.apache.texera.amber.operator.metadata.OperatorGroupConstants
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+class VolcanoPlotOpDescSpec extends AnyFlatSpec with Matchers {
+
+ private def configured: VolcanoPlotOpDesc = {
+ val op = new VolcanoPlotOpDesc
+ op.effectColumn = "log2fc"
+ op.pvalueColumn = "pvalue"
+ op
+ }
+
+ "VolcanoPlotOpDesc.operatorInfo" should "advertise the user-friendly name
and Scientific group" in {
+ val info = (new VolcanoPlotOpDesc).operatorInfo
+ info.userFriendlyName shouldBe "Volcano Plot"
+ info.operatorGroupName shouldBe
OperatorGroupConstants.VISUALIZATION_SCIENTIFIC_GROUP
+ info.operatorDescription should include("statistical")
+ }
+
+ it should "expose exactly one output port wired through forVisualization" in
{
+ (new VolcanoPlotOpDesc).operatorInfo.outputPorts should have length 1
+ }
+
+ "VolcanoPlotOpDesc.getOutputSchemas" should "return a single-port schema
with an html-content STRING column" in {
+ val op = configured
+ val schemas = op.getOutputSchemas(Map.empty)
+ schemas should have size 1
+ val (portId, schema) = schemas.head
+ portId shouldBe op.operatorInfo.outputPorts.head.id
+ schema.getAttributes should have length 1
+ schema.getAttributes.head.getName shouldBe "html-content"
+ schema.getAttributes.head.getType shouldBe AttributeType.STRING
+ }
+
+ "VolcanoPlotOpDesc.generatePythonCode" should "render a UDFTableOperator
source that decodes both column references" in {
+ // EncodableString fields are NOT emitted as literal column names — the
+ // pyb macro wraps them in `self.decode_python_template.decode("<base64>")`
+ // calls so the column name is resolved at runtime. Verify the structure
+ // (class + import + decode site count) instead of substring matches.
+ val code = configured.generatePythonCode()
+ code should include("class ProcessTableOperator(UDFTableOperator)")
+ code should include("plotly.express")
+ code should include("-log10(pvalue)")
+ val decodeOccurrences = "decode_python_template".r.findAllIn(code).length
+ decodeOccurrences should be >= 2
+ }
+
+ it should "still render code when required fields are empty (no assert
guard, current behavior)" in {
+ // Pin: unlike FunnelPlot / ImageVisualizer, VolcanoPlotOpDesc does not
+ // assert on its required fields inside generatePythonCode. An empty
+ // configuration therefore renders weird (but syntactically valid) Python
+ // that references an empty string. Documenting the divergence so a
+ // future tightening that adds the assert breaks this spec deliberately.
+ val op = new VolcanoPlotOpDesc
+ val code = op.generatePythonCode()
+ code should include("class ProcessTableOperator(UDFTableOperator)")
+ }
Review Comment:
Same split applied in 9b9f7a4863: the no-validation case is renamed to
`should currently render code even when required fields are empty (no assert
guard)` to document the present behavior, and a sibling `pendingUntilFixed`
test asserts that an empty `effectColumn`/`pvalueColumn` should raise. Once
VolcanoPlot adopts FunnelPlot/ImageVisualizer-style validation the pending test
fails and forces marker removal.
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartOpDescSpec.scala:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.bulletChart
+
+import org.apache.texera.amber.core.tuple.AttributeType
+import org.apache.texera.amber.operator.metadata.OperatorGroupConstants
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+import java.util
+import java.util.{List => JList}
+
+class BulletChartOpDescSpec extends AnyFlatSpec with Matchers {
+
+ private def configured: BulletChartOpDesc = {
+ val op = new BulletChartOpDesc
+ op.value = "actualValue"
+ op.deltaReference = "100"
+ op
+ }
+
+ "BulletChartOpDesc.operatorInfo" should "advertise the user-friendly name
and Financial group" in {
+ val info = (new BulletChartOpDesc).operatorInfo
+ info.userFriendlyName shouldBe "Bullet Chart"
+ info.operatorGroupName shouldBe
OperatorGroupConstants.VISUALIZATION_FINANCIAL_GROUP
+ info.operatorDescription should include("Bullet Chart")
+ }
+
+ it should "expose exactly one output port wired through forVisualization" in
{
+ (new BulletChartOpDesc).operatorInfo.outputPorts should have length 1
+ }
+
+ "BulletChartOpDesc.getOutputSchemas" should "return a single-port schema
with an html-content STRING column" in {
+ val op = configured
+ val schemas = op.getOutputSchemas(Map.empty)
+ schemas should have size 1
+ val (portId, schema) = schemas.head
+ portId shouldBe op.operatorInfo.outputPorts.head.id
+ schema.getAttributes should have length 1
+ schema.getAttributes.head.getName shouldBe "html-content"
+ schema.getAttributes.head.getType shouldBe AttributeType.STRING
+ }
+
+ "BulletChartOpDesc.generatePythonCode" should "render Python source with a
runtime decode site for the value column" in {
+ // EncodableString fields are NOT emitted as literal strings — the pyb
+ // macro wraps them in `self.decode_python_template.decode("<base64>")`
+ // calls. The rendered source must reference the decoder symbol at least
+ // for `value` and `deltaReference`.
+ val code = configured.generatePythonCode()
+ code should include("plotly.graph_objects")
+ val decodeOccurrences = "decode_python_template".r.findAllIn(code).length
+ decodeOccurrences should be >= 2
+ }
+
+ it should "default to an empty steps list when none are configured" in {
+ val code = configured.generatePythonCode()
+ code should include("[]")
Review Comment:
Tightened in 9b9f7a4863: replaced `code should include("[]")` with `code
should include regex """steps_data\\s*=\\s*\\[\\]"""` so the assertion anchors
on the generated `steps_data = ...` literal directly. Incidental `[]` from
`colors`, `valid_steps`, `step_errors`, `steps_list`, `html_chunks` no longer
satisfy the test.
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/bulletChart/BulletChartOpDescSpec.scala:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.bulletChart
+
+import org.apache.texera.amber.core.tuple.AttributeType
+import org.apache.texera.amber.operator.metadata.OperatorGroupConstants
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+import java.util
+import java.util.{List => JList}
+
+class BulletChartOpDescSpec extends AnyFlatSpec with Matchers {
+
+ private def configured: BulletChartOpDesc = {
+ val op = new BulletChartOpDesc
+ op.value = "actualValue"
+ op.deltaReference = "100"
+ op
+ }
+
+ "BulletChartOpDesc.operatorInfo" should "advertise the user-friendly name
and Financial group" in {
+ val info = (new BulletChartOpDesc).operatorInfo
+ info.userFriendlyName shouldBe "Bullet Chart"
+ info.operatorGroupName shouldBe
OperatorGroupConstants.VISUALIZATION_FINANCIAL_GROUP
+ info.operatorDescription should include("Bullet Chart")
+ }
+
+ it should "expose exactly one output port wired through forVisualization" in
{
+ (new BulletChartOpDesc).operatorInfo.outputPorts should have length 1
+ }
+
+ "BulletChartOpDesc.getOutputSchemas" should "return a single-port schema
with an html-content STRING column" in {
+ val op = configured
+ val schemas = op.getOutputSchemas(Map.empty)
+ schemas should have size 1
+ val (portId, schema) = schemas.head
+ portId shouldBe op.operatorInfo.outputPorts.head.id
+ schema.getAttributes should have length 1
+ schema.getAttributes.head.getName shouldBe "html-content"
+ schema.getAttributes.head.getType shouldBe AttributeType.STRING
+ }
+
+ "BulletChartOpDesc.generatePythonCode" should "render Python source with a
runtime decode site for the value column" in {
+ // EncodableString fields are NOT emitted as literal strings — the pyb
+ // macro wraps them in `self.decode_python_template.decode("<base64>")`
+ // calls. The rendered source must reference the decoder symbol at least
+ // for `value` and `deltaReference`.
+ val code = configured.generatePythonCode()
+ code should include("plotly.graph_objects")
+ val decodeOccurrences = "decode_python_template".r.findAllIn(code).length
+ decodeOccurrences should be >= 2
+ }
+
+ it should "default to an empty steps list when none are configured" in {
+ val code = configured.generatePythonCode()
+ code should include("[]")
+ }
+
+ it should "include each configured step's start/end JSON keys with extra
decode sites" in {
+ val op = configured
+ val steps: JList[BulletChartStepDefinition] = new
util.ArrayList[BulletChartStepDefinition]()
+ steps.add(new BulletChartStepDefinition("0", "50"))
+ steps.add(new BulletChartStepDefinition("50", "100"))
+ op.steps = steps
+ val code = op.generatePythonCode()
+ code should include("\"start\":")
+ code should include("\"end\":")
+ // Two steps × 2 EncodableString fields each = 4 extra decode sites on
+ // top of the value/deltaReference decodes from the base configuration.
+ val baseDecodes =
"decode_python_template".r.findAllIn(configured.generatePythonCode()).length
+ val withSteps = "decode_python_template".r.findAllIn(code).length
+ withSteps shouldBe baseDecodes + 4
+ }
+
+ it should "render a code block even with the default empty configuration (no
assert guard)" in {
+ // Pin: BulletChartOpDesc, unlike FunnelPlot / ImageVisualizer, has no
+ // assert guards inside generatePythonCode. Empty defaults still produce
+ // valid Python source. Pinning so a future tightening that adds asserts
+ // breaks this spec deliberately.
+ val op = new BulletChartOpDesc
Review Comment:
Same split as the VolcanoPlot case, applied in 9b9f7a4863: the no-validation
test is renamed to `should currently render a code block even with the default
empty configuration (no assert guard)` to document present behavior, and a
sibling `pendingUntilFixed` test asserts that an empty `value`/`deltaReference`
should raise. The pending test flips to a deliberate failure once BulletChart
starts validating its required fields.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
