mengw15 commented on code in PR #7699:
URL: https://github.com/apache/texera/pull/7699#discussion_r3790580803
##########
amber/src/test/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResourceSpec.scala:
##########
@@ -549,4 +557,55 @@ class PveResourceSpec
new PveResource().listPves(sessionUser).asScala shouldBe empty
}
+ /*
+ * PveManager's two pure guards. Everything above reaches them incidentally
through the
+ * create/install flows; these take each conjunct's untaken side directly,
which is what the
+ * partially-covered branch arms on this file are.
+ */
+ "PveManager.isValidPveName" should "reject a null name" in {
+ PveManager.isValidPveName(null) shouldBe false
+ }
+
+ it should "reject a name longer than 128 characters" in {
+ PveManager.isValidPveName("a" * 129) shouldBe false
+ // The boundary itself is allowed.
+ PveManager.isValidPveName("a" * 128) shouldBe true
+ }
+
+ it should "reject a name with characters outside the safe set" in {
+ PveManager.isValidPveName("has space") shouldBe false
+ PveManager.isValidPveName("has/slash") shouldBe false
+ PveManager.isValidPveName("") shouldBe false
+ }
+
+ it should "accept a name of safe characters" in {
+ PveManager.isValidPveName("env-1.2_3") shouldBe true
+ }
+
+ "PveManager.getPythonBin" should "refuse a name outside the safe set without
touching the disk" in {
+ PveManager.getPythonBin(testCuid, "../escape") shouldBe None
+ }
+
+ it should "return nothing when the interpreter has not been created" in {
+ PveManager.getPythonBin(testCuid, testPveName) shouldBe None
+ }
+
+ it should "return nothing when the interpreter exists but is not executable"
in {
+ val python = pythonBinFor(testPveName)
+ Files.createDirectories(python.getParent)
+ Files.write(python, Array.emptyByteArray)
+ python.toFile.setExecutable(false)
+
+ PveManager.getPythonBin(testCuid, testPveName) shouldBe None
Review Comment:
Valid — applied. Both tests now assert the state they need with
`assume(...)` and cancel, rather than fail, where the platform cannot produce
it (Windows ACLs, a root user, and a `noexec` mount each break one direction or
the other).
Worth noting they are not silently skipped where it matters: the run reports
`canceled 0`, so both branches are still genuinely taken here, and CI runs
amber on ubuntu only. The failure path still works — breaking the `None`
assertion fails the spec.
--
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]
