Copilot commented on code in PR #7699:
URL: https://github.com/apache/texera/pull/7699#discussion_r3790543758
##########
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:
These tests rely on `File#setExecutable` toggling `Files.isExecutable`, but
`setExecutable(...)` can silently fail (it returns a boolean) or behave
differently across filesystems/OSes (e.g., Windows / `noexec` mounts). Adding
explicit precondition checks (and skipping when the platform can’t represent
“non-executable”) will make the test deterministic.
This issue also appears on line 604 of the same file.
--
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]
