From 299cf77a7ee9e80457300592bf83d53d1d34bad5 Mon Sep 17 00:00:00 2001 From: kary zheng Date: Thu, 10 Sep 2026 21:00:33 -0700 Subject: [PATCH 1/8] test(verify): compare the two charts the paths drew A chart's meaning is in the numbers behind it, so the comparison reads those rather than the rendered picture, which would fail over a layout detail that carries none. Plotly writes both a JSON figure and an HTML page, and each needs its own reading: the figure is compared trace by trace, and the page has a Styler id regenerated per run and a line ending chosen by whichever platform wrote the file, neither of which is markup the operator chose. Split out of #8359 on review. That change compares two tables, which is a different question and shares no code with this one. Co-Authored-By: Claude Opus 5 (1M context) --- .../verify/VisualizationHtmlComparator.scala | 93 +++++++++++++ .../VisualizationHtmlComparatorSpec.scala | 72 ++++++++++ .../verify/VisualizationJsonComparator.scala | 123 ++++++++++++++++++ 3 files changed, 288 insertions(+) create mode 100644 workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala create mode 100644 workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala create mode 100644 workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparator.scala diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala new file mode 100644 index 00000000000..2455833577b --- /dev/null +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala @@ -0,0 +1,93 @@ +/* + * 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.translator.verify + +import org.apache.texera.amber.util.JSONUtils.objectMapper + +import java.nio.charset.StandardCharsets +import java.nio.file.{Files, Path} + +object VisualizationHtmlComparator { + + /** A pandas Styler namespaces its CSS with a uuid drawn per Styler instance, so + * the same table rendered twice differs in every `id=` and every selector even + * though the markup is identical. The uuid carries no information about the + * table — it only keeps two tables on one page from colliding — so it is + * normalized away before comparing. Only the random prefix is replaced: the + * `_row0_col0` suffix that identifies the cell stays, so a genuine structural + * difference still fails. + */ + private val StylerUuid = "T_[0-9a-f]+".r + + /** The standalone script writes its page with Python's text mode, which on Windows + * turns every newline into CRLF, while the runtime path carries the same markup + * through JSONL untouched and so keeps LF. The line ending is the platform writing + * the file rather than anything the operator chose, so it is normalized away too. + */ + private val LineEnding = "\r\n|\r".r + + private def normalize(html: String): String = + StylerUuid.replaceAllIn(LineEnding.replaceAllIn(html, "\n"), "T_uuid") + + def assertEqual(actualVisualizationJsonl: Path, expectedHtmlFile: Path): Unit = { + val actual = readActualHtml(actualVisualizationJsonl) + val expected = new String(Files.readAllBytes(expectedHtmlFile), StandardCharsets.UTF_8) + + if (normalize(actual) != normalize(expected)) { + throw new VisualizationHtmlMismatchException( + actual = actualVisualizationJsonl, + expected = expectedHtmlFile, + actualHtml = actual, + expectedHtml = expected + ) + } + } + + private def readActualHtml(path: Path): String = { + val line = Files + .readAllLines(path, StandardCharsets.UTF_8) + .stream() + .filter(_.trim.nonEmpty) + .findFirst() + .orElseThrow(() => new AssertionError(s"$path is empty")) + + val node = objectMapper.readTree(line) + val htmlNode = node.get("html-content") + if (htmlNode == null || htmlNode.isNull) { + throw new AssertionError(s"$path has no html-content field") + } + htmlNode.asText() + } +} + +final class VisualizationHtmlMismatchException( + val actual: Path, + val expected: Path, + val actualHtml: String, + val expectedHtml: String +) extends RuntimeException( + s"""Visualization HTML mismatch: + | actual: $actual + | expected: $expected + |--- actual html --- + |$actualHtml + |--- expected html --- + |$expectedHtml""".stripMargin + ) diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala new file mode 100644 index 00000000000..32ccbee7ac6 --- /dev/null +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala @@ -0,0 +1,72 @@ +/* + * 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.translator.verify + +import org.apache.texera.amber.util.JSONUtils.objectMapper +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.nio.charset.StandardCharsets +import java.nio.file.{Files, Path} + +// Untagged: the comparator reads two files and compares strings, with no Python +// to shell out to, so this belongs in the plain unit job. +class VisualizationHtmlComparatorSpec extends AnyFlatSpec with Matchers { + + private val dir: Path = Files.createTempDirectory("visualization-html-comparator-spec-") + + /** The runtime path's side: the markup as one `html-content` field of a JSONL row. */ + private def writeActual(name: String, html: String): Path = { + val node = objectMapper.createObjectNode() + node.put("html-content", html) + val p = dir.resolve(name) + Files.write(p, objectMapper.writeValueAsBytes(node)) + p + } + + /** The standalone path's side: the markup as the script wrote it to a file. */ + private def writeExpected(name: String, html: String): Path = { + val p = dir.resolve(name) + Files.write(p, html.getBytes(StandardCharsets.UTF_8)) + p + } + + private val page = "\n\n

chart

\n\n\n" + + "VisualizationHtmlComparator" should "accept two sides that differ only in line endings" in { + val actual = writeActual("crlf-actual.jsonl", page) + val expected = writeExpected("crlf-expected.html", page.replace("\n", "\r\n")) + noException should be thrownBy VisualizationHtmlComparator.assertEqual(actual, expected) + } + + it should "accept two sides that differ only in a Styler uuid" in { + val actual = writeActual("styler-actual.jsonl", "1") + val expected = writeExpected("styler-expected.html", "1") + noException should be thrownBy VisualizationHtmlComparator.assertEqual(actual, expected) + } + + it should "still reject markup that differs in more than its line endings" in { + val actual = writeActual("differ-actual.jsonl", page) + val expected = + writeExpected("differ-expected.html", page.replace("chart", "table").replace("\n", "\r\n")) + a[VisualizationHtmlMismatchException] should be thrownBy VisualizationHtmlComparator + .assertEqual(actual, expected) + } +} diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparator.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparator.scala new file mode 100644 index 00000000000..f559b2c39e6 --- /dev/null +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparator.scala @@ -0,0 +1,123 @@ +/* + * 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.translator.verify + +import com.typesafe.scalalogging.LazyLogging +import org.apache.texera.amber.util.JSONUtils.objectMapper +import org.apache.texera.amber.util.python.PythonWorkerPool + +import java.nio.file.{Files, Path, StandardCopyOption} +import scala.collection.mutable.ArrayBuffer +import scala.sys.process._ + +/** + * Compares the Plotly figure the two paths render, via `compare.py`'s + * `--plotly` mode. + * + * Shares that script — and therefore [[Comparator]]'s pool — rather than + * carrying one of its own: a worker is bound to the script it was launched + * with, so a separate script would mean a separate pool of interpreters for + * what is the same job, comparing one operator's two outputs. One comparison + * pool serves both output shapes. + */ +object VisualizationJsonComparator extends LazyLogging { + + private val ScriptResourcePath = "/python/compare.py" + + def assertEqual( + actualVisualizationJsonl: Path, + expectedPlotlyJson: Path, + pythonExe: String = resolvePython() + ): Unit = { + val (exit, stdout, stderr) = + compare(actualVisualizationJsonl, expectedPlotlyJson, pythonExe) + if (exit != 0) { + throw new VisualizationJsonMismatchException( + actual = actualVisualizationJsonl, + expected = expectedPlotlyJson, + exitCode = exit, + stdout = stdout, + stderr = stderr + ) + } + } + + // Pooled worker first, one-shot CLI as the fallback and as the behavior + // selected by TEXERA_TEST_PYTHON_WORKER=0. Both run the same + // `_run_plotly_comparison`, so results are identical. + private def compare(actual: Path, expected: Path, pythonExe: String): (Int, String, String) = { + if (PythonWorkerPool.enabled) { + try { + val req = objectMapper.createObjectNode() + req.put("kind", "plotly") + req.put("actual", actual.toString) + req.put("expected", expected.toString) + val o = PythonWorkerPool.run(ScriptResourcePath, Seq("--serve"), pythonExe, req) + return (o.exit, o.stdout, o.stderr) + } catch { + case e: PythonWorkerPool.WorkerDiedException => + logger.warn( + s"Comparator worker unavailable; falling back to one-shot CLI: ${e.getMessage}" + ) + } + } + runCli(actual, expected, pythonExe) + } + + private def runCli(actual: Path, expected: Path, pythonExe: String): (Int, String, String) = { + val scriptPath = extractScript() + val outBuf = ArrayBuffer.empty[String] + val errBuf = ArrayBuffer.empty[String] + val processLogger = ProcessLogger(line => outBuf += line, line => errBuf += line) + + val exit = Process( + Seq(pythonExe, scriptPath.toString, "--plotly", actual.toString, expected.toString) + ).!(processLogger) + (exit, outBuf.mkString("\n"), errBuf.mkString("\n")) + } + + private def extractScript(): Path = { + val stream = getClass.getResourceAsStream(ScriptResourcePath) + require(stream != null, s"compare.py not found at $ScriptResourcePath") + try { + val tmp = Files.createTempFile("compare-", ".py") + Files.copy(stream, tmp, StandardCopyOption.REPLACE_EXISTING) + tmp.toFile.deleteOnExit() + tmp + } finally stream.close() + } + + private def resolvePython(): String = + sys.env.get("UDF_PYTHON_PATH").filter(_.nonEmpty).getOrElse("python3.12") +} + +final class VisualizationJsonMismatchException( + val actual: Path, + val expected: Path, + val exitCode: Int, + val stdout: String, + val stderr: String +) extends RuntimeException( + s"""Visualization JSON mismatch (compare.py --plotly exit $exitCode): + | actual: $actual + | expected: $expected + |--- stderr --- + |$stderr""".stripMargin + ) From 715058ad99bf5f3b276700eb1bf1d19eb1063d5e Mon Sep 17 00:00:00 2001 From: kary zheng Date: Sat, 12 Sep 2026 00:51:59 -0700 Subject: [PATCH 2/8] test(verify): normalize a Styler uuid only where it is one The uuid pattern matched anywhere in the page, so two tables whose cells read T_dead and T_beef compared equal and a real difference in exported data went unreported. Replace the uuid only where a Styler writes it, in the id attribute and the selector that targets it. Co-Authored-By: Claude Opus 5 (1M context) --- .../verify/VisualizationHtmlComparator.scala | 10 +++++----- .../VisualizationHtmlComparatorSpec.scala | 17 +++++++++++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala index 2455833577b..e738e28171f 100644 --- a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala @@ -29,12 +29,12 @@ object VisualizationHtmlComparator { /** A pandas Styler namespaces its CSS with a uuid drawn per Styler instance, so * the same table rendered twice differs in every `id=` and every selector even * though the markup is identical. The uuid carries no information about the - * table — it only keeps two tables on one page from colliding — so it is - * normalized away before comparing. Only the random prefix is replaced: the - * `_row0_col0` suffix that identifies the cell stays, so a genuine structural - * difference still fails. + * table, it only keeps two tables on one page from colliding, so it is + * normalized away before comparing. The rewrite reaches only the `id` attribute + * and the `#` selector, and keeps the `_row0_col0` suffix, so a real difference + * in a cell still fails. */ - private val StylerUuid = "T_[0-9a-f]+".r + private val StylerUuid = """(?<=#|id=")T_[0-9a-f]+""".r /** The standalone script writes its page with Python's text mode, which on Windows * turns every newline into CRLF, while the runtime path carries the same markup diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala index 32ccbee7ac6..d2110fa0f33 100644 --- a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala @@ -56,12 +56,25 @@ class VisualizationHtmlComparatorSpec extends AnyFlatSpec with Matchers { noException should be thrownBy VisualizationHtmlComparator.assertEqual(actual, expected) } + /** The two places a Styler writes its uuid: the CSS selector and the `id` attribute. */ + private def styledCell(uuid: String, value: String) = + s"""""" + + s"""$value""" + it should "accept two sides that differ only in a Styler uuid" in { - val actual = writeActual("styler-actual.jsonl", "1") - val expected = writeExpected("styler-expected.html", "1") + val actual = writeActual("styler-actual.jsonl", styledCell("T_a1b2c3", "1")) + val expected = writeExpected("styler-expected.html", styledCell("T_9f8e7d", "1")) noException should be thrownBy VisualizationHtmlComparator.assertEqual(actual, expected) } + // A Styler does not escape a cell, so a value can read exactly like a uuid. + it should "still reject two sides whose cell text only looks like a Styler uuid" in { + val actual = writeActual("cell-uuid-actual.jsonl", styledCell("T_a1b2c3", "T_dead")) + val expected = writeExpected("cell-uuid-expected.html", styledCell("T_a1b2c3", "T_beef")) + a[VisualizationHtmlMismatchException] should be thrownBy VisualizationHtmlComparator + .assertEqual(actual, expected) + } + it should "still reject markup that differs in more than its line endings" in { val actual = writeActual("differ-actual.jsonl", page) val expected = From 357b74c4d3dcb097876060a8e0e7789827ae8df5 Mon Sep 17 00:00:00 2001 From: kary zheng Date: Sun, 13 Sep 2026 21:08:14 -0700 Subject: [PATCH 3/8] test(verify): read the Styler uuid in its HTML context A cell is not escaped, so a value reading `#T_dead` looked like a CSS selector and two tables disagreeing about it still compared equal. The uuid is now replaced inside the `".r + private val Tag = "<[^>]*>".r + private val SelectorUuid = "(?<=#)T_[0-9a-f]+".r + private val IdUuid = """(?<=id=")T_[0-9a-f]+""".r /** The standalone script writes its page with Python's text mode, which on Windows * turns every newline into CRLF, while the runtime path carries the same markup @@ -43,8 +52,15 @@ object VisualizationHtmlComparator { */ private val LineEnding = "\r\n|\r".r - private def normalize(html: String): String = - StylerUuid.replaceAllIn(LineEnding.replaceAllIn(html, "\n"), "T_uuid") + private def normalize(html: String): String = { + val page = LineEnding.replaceAllIn(html, "\n") + val styled = StyleElement.replaceAllIn(page, within(SelectorUuid)) + Tag.replaceAllIn(styled, within(IdUuid)) + } + + /** The same page with `uuid` normalized inside the region that matched. */ + private def within(uuid: Regex): Regex.Match => String = + region => Regex.quoteReplacement(uuid.replaceAllIn(region.matched, "T_uuid")) def assertEqual(actualVisualizationJsonl: Path, expectedHtmlFile: Path): Unit = { val actual = readActualHtml(actualVisualizationJsonl) diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala index d2110fa0f33..ce4559cf946 100644 --- a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala @@ -67,12 +67,18 @@ class VisualizationHtmlComparatorSpec extends AnyFlatSpec with Matchers { noException should be thrownBy VisualizationHtmlComparator.assertEqual(actual, expected) } - // A Styler does not escape a cell, so a value can read exactly like a uuid. - it should "still reject two sides whose cell text only looks like a Styler uuid" in { - val actual = writeActual("cell-uuid-actual.jsonl", styledCell("T_a1b2c3", "T_dead")) - val expected = writeExpected("cell-uuid-expected.html", styledCell("T_a1b2c3", "T_beef")) - a[VisualizationHtmlMismatchException] should be thrownBy VisualizationHtmlComparator - .assertEqual(actual, expected) + // A Styler does not escape a cell, so a value can read exactly like a uuid, + // `#` and all. What the table says is compared wherever it lands. + Seq("T_dead" -> "T_beef", "#T_dead" -> "#T_beef").foreach { + case (oneValue, otherValue) => + it should s"still reject two sides whose cell reads $oneValue against $otherValue" in { + val name = oneValue.filter(_.isLetterOrDigit) + val actual = writeActual(s"cell-$name-actual.jsonl", styledCell("T_a1b2c3", oneValue)) + val expected = + writeExpected(s"cell-$name-expected.html", styledCell("T_a1b2c3", otherValue)) + a[VisualizationHtmlMismatchException] should be thrownBy VisualizationHtmlComparator + .assertEqual(actual, expected) + } } it should "still reject markup that differs in more than its line endings" in { From dfa14dc66077ecaaf03c60c192f1dd811b29237e Mon Sep 17 00:00:00 2001 From: kary zheng Date: Mon, 14 Sep 2026 00:35:55 -0700 Subject: [PATCH 4/8] build: let the compiling service read the worker pool VisualizationJsonComparator reuses PythonWorkerPool, which lives in workflow-operator's test sources, so the module needs it in test scope. Co-Authored-By: Claude Opus 5 (1M context) --- build.sbt | 1 + 1 file changed, 1 insertion(+) diff --git a/build.sbt b/build.sbt index 035edbb68cc..94a5af5606c 100644 --- a/build.sbt +++ b/build.sbt @@ -239,6 +239,7 @@ lazy val WorkflowCompiler = (project in file("common/workflow-compiler")) .dependsOn(WorkflowOperator) lazy val WorkflowCompilingService = (project in file("workflow-compiling-service")) .dependsOn(WorkflowCompiler, Auth, Config, Resource) + .dependsOn(WorkflowOperator % "test->test") // reuse PythonWorkerPool in verify tests .settings(commonModuleSettings) .settings( dependencyOverrides ++= Seq( From fe7ac26a265244151bcfd6047c1e72748b3872b3 Mon Sep 17 00:00:00 2001 From: kary zheng Date: Fri, 18 Sep 2026 13:23:40 -0700 Subject: [PATCH 5/8] test(verify): compare every chart a run drew, not the first The comparison read the first row of the runtime path's output and stopped, so a run that drew a chart the exported script never drew still passed. The two sides are now compared as sequences, and the failure says how many each path drew and what the unmatched one held. Co-Authored-By: Claude Opus 5 (1M context) --- .../verify/VisualizationHtmlComparator.scala | 49 +++++++++++-------- .../VisualizationHtmlComparatorSpec.scala | 29 +++++++++-- 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala index 32f1df20662..4a8c5e7d2b7 100644 --- a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparator.scala @@ -23,6 +23,7 @@ import org.apache.texera.amber.util.JSONUtils.objectMapper import java.nio.charset.StandardCharsets import java.nio.file.{Files, Path} +import scala.jdk.CollectionConverters._ import scala.util.matching.Regex object VisualizationHtmlComparator { @@ -62,11 +63,20 @@ object VisualizationHtmlComparator { private def within(uuid: Regex): Regex.Match => String = region => Regex.quoteReplacement(uuid.replaceAllIn(region.matched, "T_uuid")) + /** Every chart the runtime path drew, against every chart the standalone one + * wrote. + * + * A visualization operator emits a row per chart, and the exported script + * writes one page, so the sides are compared as sequences rather than by + * reading a row off each: a run whose second chart the other path never drew + * is a disagreement, and looking only at the first row would call it a match. + */ def assertEqual(actualVisualizationJsonl: Path, expectedHtmlFile: Path): Unit = { val actual = readActualHtml(actualVisualizationJsonl) - val expected = new String(Files.readAllBytes(expectedHtmlFile), StandardCharsets.UTF_8) + val expected = + Seq(new String(Files.readAllBytes(expectedHtmlFile), StandardCharsets.UTF_8)) - if (normalize(actual) != normalize(expected)) { + if (actual.map(normalize) != expected.map(normalize)) { throw new VisualizationHtmlMismatchException( actual = actualVisualizationJsonl, expected = expectedHtmlFile, @@ -76,34 +86,33 @@ object VisualizationHtmlComparator { } } - private def readActualHtml(path: Path): String = { - val line = Files - .readAllLines(path, StandardCharsets.UTF_8) - .stream() - .filter(_.trim.nonEmpty) - .findFirst() - .orElseThrow(() => new AssertionError(s"$path is empty")) + private def readActualHtml(path: Path): Seq[String] = { + val lines = + Files.readAllLines(path, StandardCharsets.UTF_8).asScala.filter(_.trim.nonEmpty).toSeq + if (lines.isEmpty) throw new AssertionError(s"$path is empty") - val node = objectMapper.readTree(line) - val htmlNode = node.get("html-content") - if (htmlNode == null || htmlNode.isNull) { - throw new AssertionError(s"$path has no html-content field") + lines.zipWithIndex.map { + case (line, row) => + val htmlNode = objectMapper.readTree(line).get("html-content") + if (htmlNode == null || htmlNode.isNull) { + throw new AssertionError(s"$path row ${row + 1} has no html-content field") + } + htmlNode.asText() } - htmlNode.asText() } } final class VisualizationHtmlMismatchException( val actual: Path, val expected: Path, - val actualHtml: String, - val expectedHtml: String + val actualHtml: Seq[String], + val expectedHtml: Seq[String] ) extends RuntimeException( s"""Visualization HTML mismatch: - | actual: $actual - | expected: $expected + | actual: $actual (${actualHtml.size} drawn) + | expected: $expected (${expectedHtml.size} drawn) |--- actual html --- - |$actualHtml + |${actualHtml.mkString("\n--- and ---\n")} |--- expected html --- - |$expectedHtml""".stripMargin + |${expectedHtml.mkString("\n--- and ---\n")}""".stripMargin ) diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala index ce4559cf946..cfbb28f5e95 100644 --- a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationHtmlComparatorSpec.scala @@ -32,12 +32,15 @@ class VisualizationHtmlComparatorSpec extends AnyFlatSpec with Matchers { private val dir: Path = Files.createTempDirectory("visualization-html-comparator-spec-") - /** The runtime path's side: the markup as one `html-content` field of a JSONL row. */ - private def writeActual(name: String, html: String): Path = { - val node = objectMapper.createObjectNode() - node.put("html-content", html) + /** The runtime path's side: a JSONL row per chart, each in `html-content`. */ + private def writeActual(name: String, html: String*): Path = { val p = dir.resolve(name) - Files.write(p, objectMapper.writeValueAsBytes(node)) + val rows = html.map { page => + val node = objectMapper.createObjectNode() + node.put("html-content", page) + objectMapper.writeValueAsString(node) + } + Files.write(p, rows.mkString("\n").getBytes(StandardCharsets.UTF_8)) p } @@ -81,6 +84,22 @@ class VisualizationHtmlComparatorSpec extends AnyFlatSpec with Matchers { } } + // The runtime path draws a chart per row it emits, and the exported script + // writes one page. A run that drew a second chart the other path never drew is + // a disagreement, and it is the one a comparison that reads the first row and + // stops would call a match. + it should "reject a run that drew a chart the other path did not" in { + val actual = writeActual("extra-actual.jsonl", page, page.replace("chart", "second chart")) + val expected = writeExpected("extra-expected.html", page) + val thrown = the[VisualizationHtmlMismatchException] thrownBy VisualizationHtmlComparator + .assertEqual(actual, expected) + thrown.actualHtml should have length 2 + thrown.expectedHtml should have length 1 + // The rows are named in the message, so the one nobody compared is readable + // from the failure rather than only from the file. + thrown.getMessage should include("second chart") + } + it should "still reject markup that differs in more than its line endings" in { val actual = writeActual("differ-actual.jsonl", page) val expected = From 5ce2dd1f27fd6b648120ea1028c4f0da647e3879 Mon Sep 17 00:00:00 2001 From: kary zheng Date: Fri, 18 Sep 2026 15:52:25 -0700 Subject: [PATCH 6/8] test(verify): pin that every figure a run drew is compared The comparison had no spec of its own, and the case worth holding is the multi-figure one: an operator that draws a figure per input row agrees on the first long after it has stopped agreeing, so a comparison that read one figure and stopped called that a match. A figure the other path drew differently, and one it never drew at all, are each their own test. Co-Authored-By: Claude Opus 5 (1M context) --- .../VisualizationJsonComparatorSpec.scala | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala new file mode 100644 index 00000000000..d283dc0375f --- /dev/null +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.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.translator.verify + +import org.apache.texera.amber.translator.verify.tags.IntegrationTest +import org.apache.texera.amber.util.JSONUtils.objectMapper +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.nio.charset.StandardCharsets +import java.nio.file.{Files, Path} + +/** Tagged @IntegrationTest with the rest of the specs that fork Python: the + * comparison itself is `compare.py --plotly`. + */ +@IntegrationTest +class VisualizationJsonComparatorSpec extends AnyFlatSpec with Matchers { + + private val dir: Path = Files.createTempDirectory("visualization-json-comparator-spec-") + + /** One figure, as plotly writes it into a page. */ + private def figure(value: Int): String = + s"""
" + + /** The runtime path's side: the whole page as one `html-content` JSONL row, + * which is how an operator that draws a figure per row hands them over. + */ + private def writeActual(name: String, figures: Int*): Path = { + val node = objectMapper.createObjectNode() + node.put("html-content", figures.map(figure).mkString("
", "", "
")) + val p = dir.resolve(name) + Files.write(p, objectMapper.writeValueAsBytes(node)) + p + } + + /** The standalone path's side: its page, holding every figure it drew. */ + private def writePage(name: String, figures: Int*): Path = { + val p = dir.resolve(name) + Files.write(p, figures.map(figure).mkString.getBytes(StandardCharsets.UTF_8)) + p + } + + /** The standalone path's other side: the single figure `write_json` leaves. */ + private def writeFigureJson(name: String, value: Int): Path = { + val p = dir.resolve(name) + val json = s"""{"data": [{"type": "indicator", "value": $value}], """ + + """"layout": {"height": 150}}""" + Files.write(p, json.getBytes(StandardCharsets.UTF_8)) + p + } + + "VisualizationJsonComparator" should "accept the one figure the script wrote as JSON" in { + val actual = writeActual("single-actual.jsonl", 1) + val expected = writeFigureJson("single-expected.json", 1) + noException should be thrownBy VisualizationJsonComparator.assertEqual(actual, expected) + } + + it should "accept a run whose every figure the other path drew the same" in { + val actual = writeActual("multi-actual.jsonl", 1, 2, 3) + val expected = writePage("multi-expected.html", 1, 2, 3) + noException should be thrownBy VisualizationJsonComparator.assertEqual(actual, expected) + } + + // The one a comparison that reads the first figure and stops would call a + // match, and the reason it reads them all: an operator drawing a figure per + // input row agrees on the first long after it has stopped agreeing. + it should "reject a second figure the other path drew differently" in { + val actual = writeActual("second-actual.jsonl", 1, 2) + val expected = writePage("second-expected.html", 1, 7) + val thrown = the[VisualizationJsonMismatchException] thrownBy VisualizationJsonComparator + .assertEqual(actual, expected) + thrown.stderr should include("\"value\": 7") + } + + it should "reject a figure the other path did not draw at all" in { + val actual = writeActual("count-actual.jsonl", 1, 2) + val expected = writePage("count-expected.html", 1) + val thrown = the[VisualizationJsonMismatchException] thrownBy VisualizationJsonComparator + .assertEqual(actual, expected) + // The counts, so the failure reads as a missing figure rather than as two + // long dumps to count out by hand. + thrown.stderr should include("(2 drawn, 1 expected)") + } +} From e47995c44a45f3432f19905d9c9e45d133bab3bc Mon Sep 17 00:00:00 2001 From: kary zheng Date: Fri, 18 Sep 2026 16:34:50 -0700 Subject: [PATCH 7/8] test(verify): read the count out of the message compare.py now writes The comparison says "the run drew 2 and the exported script drew 1" where the assertion was still reading the wording this spec was written against. Co-Authored-By: Claude Opus 5 (1M context) --- .../translator/verify/VisualizationJsonComparatorSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala index d283dc0375f..6d3573eb420 100644 --- a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala @@ -98,6 +98,6 @@ class VisualizationJsonComparatorSpec extends AnyFlatSpec with Matchers { .assertEqual(actual, expected) // The counts, so the failure reads as a missing figure rather than as two // long dumps to count out by hand. - thrown.stderr should include("(2 drawn, 1 expected)") + thrown.stderr should include("the run drew 2 and the exported script drew 1") } } From 739b31602d8eb6cea0226d6214596056716c57ac Mon Sep 17 00:00:00 2001 From: kary zheng Date: Sat, 26 Sep 2026 15:31:11 -0700 Subject: [PATCH 8/8] test(verify): leave the page-to-page chart cases to the comparator's own spec Co-Authored-By: Claude Opus 5.5 (1M context) --- .../VisualizationJsonComparatorSpec.scala | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala index 6d3573eb420..b314e473565 100644 --- a/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/amber/translator/verify/VisualizationJsonComparatorSpec.scala @@ -74,23 +74,6 @@ class VisualizationJsonComparatorSpec extends AnyFlatSpec with Matchers { noException should be thrownBy VisualizationJsonComparator.assertEqual(actual, expected) } - it should "accept a run whose every figure the other path drew the same" in { - val actual = writeActual("multi-actual.jsonl", 1, 2, 3) - val expected = writePage("multi-expected.html", 1, 2, 3) - noException should be thrownBy VisualizationJsonComparator.assertEqual(actual, expected) - } - - // The one a comparison that reads the first figure and stops would call a - // match, and the reason it reads them all: an operator drawing a figure per - // input row agrees on the first long after it has stopped agreeing. - it should "reject a second figure the other path drew differently" in { - val actual = writeActual("second-actual.jsonl", 1, 2) - val expected = writePage("second-expected.html", 1, 7) - val thrown = the[VisualizationJsonMismatchException] thrownBy VisualizationJsonComparator - .assertEqual(actual, expected) - thrown.stderr should include("\"value\": 7") - } - it should "reject a figure the other path did not draw at all" in { val actual = writeActual("count-actual.jsonl", 1, 2) val expected = writePage("count-expected.html", 1)