mengw15 opened a new pull request, #7699:
URL: https://github.com/apache/texera/pull/7699
### What changes were proposed in this PR?
Takes the untaken side of the conditionals in the two files #7698 lists. 12
new tests.
**PveManager** — the two pure guards. The create/install flows above reach
them
incidentally; these decide each conjunct directly.
- `isValidPveName`: null, a name over 128 characters (with 128 itself pinned
as
allowed), characters outside the safe set, the empty string, and a valid
name — so
each of the three `&&` operands decides the result at least once.
- `getPythonBin`: a name outside the safe set (rejected before any disk
access), an
interpreter that has not been created, one that exists but is not
executable, and one
that exists and is executable.
**SQLSourceOpExec** — the result iterator and the keyword binding, driven
against a
mocked JDBC chain.
- The iterator over a multi-row result set: one tuple per row, a second
`hasNext` that
does not consume the cached tuple, and exhaustion once the rows run out
and no further
query is available; plus a query that returns no rows at all.
- The keyword guard `keywordSearch && keywordSearchByColumn != null &&
keywords != null`
in all four shapes, asserting the bind happens only when all three hold.
The spec's existing `TestSQLSourceOpExec` already overrides
`establishConn()`, so the
mocked `Connection`/`PreparedStatement`/`ResultSet` go in through that seam
rather than by
registering a stub `java.sql.Driver` with the global `DriverManager` as the
issue
originally suggested — same hermetic result, no global state.
No production code was changed.
### One guard that cannot be reached
`getPythonBin`'s `if (!resolved.startsWith(root)) return None` is
unreachable as written:
the name must match `^[A-Za-z0-9._-]+$`, so it is a single path segment with
no separator,
and `..` climbs at most to the root itself before `pve/bin/python` is
appended — the result
always starts with the root. The test named for the traversal attempt
therefore covers the
*pattern* guard on the line above it, which is what actually rejects such a
name.
### Any related issues, documentation, discussions?
Closes #7698.
The issue originally also listed `DPThread`; it has been removed from the
issue, since
every arm it named lives inside `runDPThreadMainLogic()` — `private[this]`,
so unreachable
from a test — and `start()` constructs `Executors.newSingleThreadExecutor`
inline with no
seam to inject a same-thread executor. The one synchronously reachable item
there,
`handleActorCommand`'s two arms, is already covered by `DPThreadSpec`.
Covering the rest
needs a production seam and belongs in its own refactor issue.
### How was this PR tested?
`sbt "WorkflowOperator/testOnly *SQLSourceOpExecSpec"` — 64 pass;
`sbt "WorkflowExecutionService/testOnly *PveResourceSpec"` — 44 pass. The
failure path was
verified by breaking one assertion in each spec (red, non-zero exit) and
restoring them.
`Test/scalafmtCheck` and `Test/scalafix --check` are clean on both modules.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])
--
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]
