Copilot commented on code in PR #4812: URL: https://github.com/apache/texera/pull/4812#discussion_r3177583084
########## common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/lineChart/LineChartOpDescSpec.scala: ########## @@ -0,0 +1,95 @@ +/* + * 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.lineChart + +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 + +class LineChartOpDescSpec extends AnyFlatSpec with Matchers { + + private def lineConfig(x: String, y: String): LineConfig = { + val c = new LineConfig + c.xValue = x + c.yValue = y + c + } + + private def configured: LineChartOpDesc = { + val op = new LineChartOpDesc + op.xLabel = "x_col" + op.yLabel = "y_col" + val ls = new util.ArrayList[LineConfig]() + ls.add(lineConfig("x_col", "y_col")) + op.lines = ls + op + } + + "LineChartOpDesc.operatorInfo" should "advertise the user-friendly name and Basic group" in { + val info = (new LineChartOpDesc).operatorInfo + info.userFriendlyName shouldBe "Line Chart" + info.operatorGroupName shouldBe OperatorGroupConstants.VISUALIZATION_BASIC_GROUP + info.operatorDescription should include("line chart") + } + + it should "expose exactly one output port wired through forVisualization" in { + (new LineChartOpDesc).operatorInfo.outputPorts should have length 1 + } + + "LineChartOpDesc.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 + } + + "LineChartOpDesc.generatePythonCode" should "render Python source with runtime decode sites for both labels" in { + val code = configured.generatePythonCode() + code should include("plotly") + val decodeOccurrences = "decode_python_template".r.findAllIn(code).length + decodeOccurrences should be >= 2 Review Comment: This decode-site count doesn’t specifically validate that `xLabel` and `yLabel` are encoded, because `LineConfig.xValue`/`yValue` also contribute decode sites. As written, `decodeOccurrences >= 2` could still pass if label encoding regresses. Consider using distinct sentinel strings for labels vs. line-config values and asserting the encoded output contains decode expressions for the label sentinels (or that the raw label sentinels are absent). ########## common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/pieChart/PieChartOpDescSpec.scala: ########## @@ -33,4 +36,56 @@ class PieChartOpDescSpec extends AnyFlatSpec with BeforeAndAfter { opDesc.manipulateTable() } } + + "PieChartOpDesc.operatorInfo" should "advertise the user-friendly name and Basic group" in { + val info = opDesc.operatorInfo + info.userFriendlyName shouldBe "Pie Chart" + info.operatorGroupName shouldBe OperatorGroupConstants.VISUALIZATION_BASIC_GROUP + info.operatorDescription should include("Pie Chart") + } + + it should "expose exactly one output port wired through forVisualization" in { + opDesc.operatorInfo.outputPorts should have length 1 + } + + "PieChartOpDesc.getOutputSchemas" should "return a single-port schema with an html-content STRING column" in { + opDesc.value = "amount" + opDesc.name = "label" + val schemas = opDesc.getOutputSchemas(Map.empty) + schemas should have size 1 + val (portId, schema) = schemas.head + portId shouldBe opDesc.operatorInfo.outputPorts.head.id + schema.getAttributes should have length 1 + schema.getAttributes.head.getName shouldBe "html-content" + schema.getAttributes.head.getType shouldBe AttributeType.STRING + } + + "PieChartOpDesc.generatePythonCode" should "render Python source with runtime decode sites for value and name" in { + opDesc.value = "amount" + opDesc.name = "label" + val code = opDesc.generatePythonCode() + code should include("class ProcessTableOperator(UDFTableOperator)") + code should include("plotly.express") + val decodeOccurrences = "decode_python_template".r.findAllIn(code).length + decodeOccurrences should be >= 2 Review Comment: The decode-site assertion here is too weak to prove that *both* EncodableString inputs are being encoded. Since `value` appears multiple times in `PieChartOpDesc.generatePythonCode`, a regression where `name` is rendered as a raw literal could still produce `decodeOccurrences >= 2` and this test would pass. Consider asserting encoding per-field (e.g., compute the expected Base64 for each sentinel string and assert the generated code includes `self.decode_python_template('<b64>')` for both, or use distinct sentinel values and assert the raw sentinel strings do not appear in the encoded output). ########## common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/barChart/BarChartOpDescSpec.scala: ########## @@ -50,4 +56,46 @@ class BarChartOpDescSpec extends AnyFlatSpec with BeforeAndAfter { } } + "BarChartOpDesc.operatorInfo" should "advertise the user-friendly name and Basic group" in { + val info = opDesc.operatorInfo + info.userFriendlyName shouldBe "Bar Chart" + info.operatorGroupName shouldBe OperatorGroupConstants.VISUALIZATION_BASIC_GROUP + info.operatorDescription should include("Bar Chart") + } + + it should "expose exactly one output port wired through forVisualization" in { + opDesc.operatorInfo.outputPorts should have length 1 + } + + "BarChartOpDesc.getOutputSchemas" should "return a single-port schema with an html-content STRING column" in { + opDesc.value = "v" + opDesc.fields = "f" + val schemas = opDesc.getOutputSchemas(Map.empty) + schemas should have size 1 + val (portId, schema) = schemas.head + portId shouldBe opDesc.operatorInfo.outputPorts.head.id + schema.getAttributes should have length 1 + schema.getAttributes.head.getName shouldBe "html-content" + schema.getAttributes.head.getType shouldBe AttributeType.STRING + } + + "BarChartOpDesc.generatePythonCode" should "render a UDFTableOperator source with at least two runtime decode sites for value/fields" in { + // EncodableString fields are wrapped in `self.decode_python_template(...)` + // calls by the pyb macro; pin a structural count instead of literal names. + opDesc.value = "v" + opDesc.fields = "f" + val code = opDesc.generatePythonCode() + code should include("class ProcessTableOperator(UDFTableOperator)") + code should include("plotly.express") + val decodeOccurrences = "decode_python_template".r.findAllIn(code).length + decodeOccurrences should be >= 2 Review Comment: The `decodeOccurrences >= 2` check can pass even if only one of the two EncodableString fields (`value` or `fields`) is properly encoded, because each is referenced multiple times in the generated template. To ensure this test actually covers both fields, assert encoding per-field (e.g., verify the code contains decode expressions for the Base64 of two distinct sentinel strings, or assert the raw sentinel column names don’t appear in the encoded output). -- 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]
