This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7656-e4500eb0395e431b9f81279afd7b8cc1c423bf95 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 09fa90337a8fb7e109adebd24ccb0d4b81f46ad9 Author: Meng Wang <[email protected]> AuthorDate: Fri Aug 14 06:03:52 2026 +0000 test(amber): cover PveResource's duplicate-name conflict and empty-result paths (#7656) ### What changes were proposed in this PR? Extends `PveResourceSpec` with the conflict and empty-result paths that were still unhit. No production code was changed. Everything runs on the spec's existing embedded-Postgres setup — the duplicate cases drive the real `(uid, name)` unique index rather than mocking the DAO. 5 tests: - `savePve` — 409 with `An environment named "<name>" already exists.` when the user already owns that name, and the negative case: the same name under a *different* user still returns 201. - `listPves` — the empty-result arm for a user that owns nothing. - `fetchPVEs` — the empty-result arm for a computing unit with no environments. - `deleteEnvironments` — removes every environment of a computing unit, and is a no-op for one that has none. Measured with `sbt WorkflowExecutionService/jacoco` scoped to this spec: `PveResource.scala` goes from **49/59 to 50/59 lines**. That is smaller than the issue's estimate because most of what it lists is either already covered or not unit-reachable — details below, in case it is worth adjusting the issue: - `updatePve`'s 409-rename, 404 and 400 arms, and `getSystemPackages`' success path, are **already covered** by tests added in #7179. - `getSystemPackages`' `catch` is **unreachable**: `PveManager.getSystemPackages` just returns the cached `systemPackages` value and never shells out, so nothing in the call can throw. - The bad-JSON fallback in `listPves` is **unreachable**: `packages` is a `jsonb` column, so Postgres rejects a malformed document at insert time. - Of the 9 lines still missed, none is untested logic: two are single-instruction remnants on lines that do execute (`mi=1, ci=22` / `ci=4` — Scala bridge bytecode), four are the `catch` arms above, and the two conflict handlers are hit (`ci=19, cb=3`) with only the `sqlState != "23505"` guard path unhit, which needs a different database error to construct. ### Any related issues, documentation, discussions? Closes #7653 ### How was this PR tested? Unit tests, run locally against embedded Postgres. All pass, and the failure path was verified by breaking an assertion to confirm the suite goes red: ``` sbt "WorkflowExecutionService/testOnly *PveResourceSpec" # Tests: succeeded 36, failed 0 sbt "WorkflowExecutionService/Test/scalafmtCheck" # clean sbt "WorkflowExecutionService/Test/scalafix --check" # clean ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --- .../pythonvirtualenvironment/PveResourceSpec.scala | 80 ++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResourceSpec.scala index 73f9899c83..1934ea0d5f 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResourceSpec.scala @@ -139,6 +139,19 @@ class PveResourceSpec queue.iterator().asScala.toList.mkString("\n") } + /** + * A computing-unit id whose venv directory does not exist on this machine. + * PveManager.getEnvironments lists /tmp/texera-pve/venvs/<cuid> directly, so a fixed id + * could pick up environments left behind by an earlier local run. + */ + private def unusedCuid(): Int = { + val venvRoot = Paths.get("/tmp/texera-pve/venvs") + Iterator + .continually(900000 + scala.util.Random.nextInt(90000)) + .find(cuid => !Files.exists(venvRoot.resolve(cuid.toString))) + .get + } + "PveManager" should "create a new PVE and list it" in { expectProcessCalls() PveManager.createNewPve(testCuid, queue, testPveName) @@ -453,6 +466,29 @@ class PveResourceSpec pves.map(_.get("pveName")) should contain(testPveName) } + it should "return an empty list for a computing unit with no environments" in { + // getEnvironments reads /tmp/texera-pve/venvs/<cuid> straight off disk, so use a cuid + // that cannot collide with leftovers from an earlier local run. + val resp = new PveResource().fetchPVEs(Int.box(unusedCuid())) + + resp.getStatus shouldBe Response.Status.OK.getStatusCode + resp.getEntity.asInstanceOf[java.util.List[_]].asScala shouldBe empty + } + + "PveResource.deleteEnvironments" should "remove every environment of the computing unit" in { + expectProcessCalls() + PveManager.createNewPve(testCuid, queue, testPveName) + PveManager.getEnvironments(testCuid).map(_.pveName) should contain(testPveName) + + new PveResource().deleteEnvironments(testCuid) + + PveManager.getEnvironments(testCuid) shouldBe empty + } + + it should "be a no-op for a computing unit that has none" in { + noException should be thrownBy new PveResource().deleteEnvironments(unusedCuid()) + } + "PveResource.deletePackage" should "return 200 when the uninstall succeeds" in { expectProcessCalls() PveManager.createNewPve(testCuid, queue, testPveName) @@ -469,4 +505,48 @@ class PveResourceSpec val resp = new PveResource().deletePackage(testCuid, testPveName, "pyarrow") resp.getStatus shouldBe Response.Status.BAD_REQUEST.getStatusCode } + + // ─── duplicate-name conflicts ────────────────────────────────────────────── + // The unique index on (uid, name) is what surfaces a duplicate as SQLSTATE 23505, + // so these drive real constraint violations rather than mocking the DAO. + // The resources' 500 handlers are not covered here: PveManager.getSystemPackages + // returns a cached value and never throws, and the generic `case e: Exception` arms + // would need the DAO mocked out to reach. + + "PveResource.savePve" should "return 409 when the user already has an environment with that name" in { + PveManager.savePve(testUid, "env-dup", "{}") + + val resp = new PveResource().savePve(SavePvePayload("env-dup", Map.empty), sessionUser) + + resp.getStatus shouldBe Response.Status.CONFLICT.getStatusCode + resp.getEntity shouldBe """An environment named "env-dup" already exists.""" + } + + it should "still accept the same name for a different user" in { + val otherUid = testUid + 1 + val otherUser = new User + otherUser.setUid(otherUid) + otherUser.setName(s"pve_other_$otherUid") + otherUser.setEmail(s"other_${UUID.randomUUID()}@example.com") + val userDao = new UserDao(getDSLContext.configuration()) + userDao.insert(otherUser) + try { + PveManager.savePve(otherUid, "env-shared", "{}") + + val resp = new PveResource().savePve(SavePvePayload("env-shared", Map.empty), sessionUser) + + resp.getStatus shouldBe Response.Status.CREATED.getStatusCode + } finally { + getDSLContext + .deleteFrom(VIRTUAL_ENVIRONMENTS) + .where(VIRTUAL_ENVIRONMENTS.UID.eq(otherUid)) + .execute() + userDao.deleteById(otherUid) + } + } + + "PveResource.listPves" should "return an empty list when the user owns nothing" in { + new PveResource().listPves(sessionUser).asScala shouldBe empty + } + }
