Copilot commented on code in PR #7899:
URL: https://github.com/apache/texera/pull/7899#discussion_r3840947947
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/ecdfPlot/ECDFPlotOpDescSpec.scala:
##########
@@ -252,6 +273,15 @@ class ECDFPlotOpDescSpec extends AnyFlatSpec with
BeforeAndAfter with Matchers {
code should include("no valid rows left after removing missing or
non-numeric values.")
code should include("plotly.io.to_html(fig, include_plotlyjs='cdn',
auto_play=False)")
code should include("yield {'html-content': html}")
+
+ // Order matters, and a bag of unordered `include`s cannot see it. The
cleaning
+ // step has to be spliced BEFORE the figure so the plot is built on
cleaned rows,
+ // with the "no valid rows left" guard sitting between the two; swapping
the two
+ // splices would plot uncleaned data and leave that second guard dead.
+ code.indexOf("pd.to_numeric(") should be < code.indexOf("px.ecdf(")
+ code.indexOf("px.ecdf(") should be < code.indexOf("plotly.io.to_html(")
Review Comment:
The ordering assertions using `indexOf` can still pass if a substring is
missing (because `indexOf` returns `-1`, and `-1 < someIndex` is true). In this
test you don't assert that `pd.to_numeric(` and `px.ecdf(` are present in
`code` before comparing their indices, so a regression that removes one of
these steps could slip through while the ordering checks still succeed. Add
explicit `code should include("pd.to_numeric(")` and `code should
include("px.ecdf(")` (and optionally assert the indices are not -1) before the
`<` comparisons.
--
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]
