Copilot commented on code in PR #7883:
URL: https://github.com/apache/texera/pull/7883#discussion_r3837966814
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/dictionary/DictionaryMatcherOpExecSpec.scala:
##########
@@ -200,4 +200,142 @@ class DictionaryMatcherOpExecSpec extends AnyFlatSpec
with BeforeAndAfter {
assert(opExec.tokenizedDictionaryEntries == null)
assert(opExec.luceneAnalyzer == null)
}
+
+ // NOTE: every test below must stay BELOW "close properly". That test asserts
+ // `tokenizedDictionaryEntries == null` on whatever executor the preceding
test
+ // left in the shared `opExec` var, and close() only *clears* that buffer --
it
+ // never nulls it. Inserting a CONJUNCTION_INDEXBASED test above it would
make
+ // it fail.
Review Comment:
The "close properly" test is not self-contained and currently relies on the
previous test leaving `opExec` in a state where `tokenizedDictionaryEntries`
was never initialized (so it stays `null`). This makes the suite
order-dependent and also asserts the wrong close() behavior for
`CONJUNCTION_INDEXBASED` (where close() clears the buffer but does not null
it). Consider rewriting the test to create its own executor and assert
`tokenizedDictionaryEntries` is either null or empty; this removes the need for
the ordering NOTE entirely.
--
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]
