mengw15 opened a new issue, #8103:
URL: https://github.com/apache/texera/issues/8103
### Task Summary
`PythonWorkflowWorker` is at **21 % — the lowest-covered file of its size in
the engine, with 86 uncovered lines**. It looks untestable at a glance because
it is a pekko actor that spawns a Python process, but most of the gap is not
the process: it is a pure binary-selection helper, a path builder, the teardown
sequence, and the actor's own message handlers. `pekko.testkit` is already used
by **46 specs** in this repository, so the actor half needs no new
infrastructure.
There is no `PythonWorkflowWorkerSpec` today (the package has specs for the
proxy client/server, the startup config and the batch queue) — this is an ADD.
### Behavior to add
**`PythonWorkflowWorker`**
(`amber/src/main/scala/org/apache/texera/amber/engine/architecture/pythonworker/PythonWorkflowWorker.scala`,
codecov 21 % — ADD `PythonWorkflowWorkerSpec`)
**Pure helpers — no actor needed**
- `choosePythonBin()` (239–245) — resolves the interpreter as
`workerConfig.cuid.filter(_ => pveName.nonEmpty).flatMap(cuid =>
PveManager.getPythonBin(cuid, pveName)).map(_.toString).getOrElse(fallback)`.
Four distinct outcomes, each one input value apart: no `cuid` → the
`PythonUtils.getPythonExecutable` fallback; a `cuid` but a blank/whitespace
`pveName` → fallback (note it is `.trim`-ed, so cover a whitespace-only name);
`cuid` and `pveName` present but `PveManager.getPythonBin` returning `None` →
fallback; everything present → the virtual environment's interpreter. Assert
the chosen path in each case.
- `pythonSrcDirectory` (132–135) — the `Utils.amberHomePath / src / main /
python` path. Assert the tail of the resolved path rather than an absolute
string, so the test does not depend on where the repo is checked out.
- `RENVPath` — `UdfConfig.rPath.trim`; cover a configured value and a blank
one.
**Actor lifecycle — with `TestKit`**
Construct the actor with `Props` inside a `TestKit`-provided `ActorSystem`,
as the existing engine specs do:
- `postStop()` (190–203) — the teardown sequence:
`pythonProxyClient.close()`, both executors shut down, then
`pythonServerProcess.destroy()`. Stop the actor and assert the sequence ran.
Then cover the `catch` arm (203): make one of those calls throw (for example a
stubbed proxy client whose `close()` raises) and assert the failure is logged
and does **not** escape `postStop`.
- The output-message handler passed to `NetworkOutputGateway` (151–168) and
`receiveCreditMessages` (172–177) — send the corresponding messages to the
actor and assert what is enqueued on the proxy client (`Backpressure`,
`CreditUpdate`).
- `loadFromCheckpoint` is `???` — assert it throws `NotImplementedError` if
you want the line, or leave it out; do not build anything around it.
Determinism notes:
- **Do not let the real Python process start.** Inject or stub the proxy
client and the process handle; the `Process(...).run(BasicIO.standard(false))`
call in `startPythonProcess` (258–272) is **out of scope** — the interpreter is
not guaranteed present on a CI runner and the process is not deterministic.
- Use `TestKit`'s `expectMsg` / probes rather than sleeping; if you must
wait for an actor side effect, poll a condition to a generous deadline instead
of asserting after a fixed delay.
- Shut the `ActorSystem` down in `afterAll` (`TestKit.shutdownActorSystem`)
so the suite does not leak threads between specs.
- Do not assert on log output as a test's only assertion.
### Task Type
- [ ] Refactor / Cleanup
- [ ] DevOps / Deployment / CI
- [x] Testing / QA
- [ ] Documentation
- [ ] Performance
- [ ] Other
--
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]
