nchammas commented on code in PR #59250:
URL: https://github.com/apache/spark/pull/59250#discussion_r4437504155


##########
dev/sparktestsupport/modules.py:
##########
@@ -102,6 +105,70 @@ def is_ignored_file(filename: str) -> bool:
     return False
 
 
+class TestStage(Enum):
+    """
+    When a shell script test goal runs in dev/run-tests.py.
+    """
+
+    # Before the Spark build.
+    PRE_BUILD = "pre_build"
+
+
+@dataclass(frozen=True)
+class ShellScriptTestGoal:
+    """
+    Describe a shell script test goal.
+
+    >>> goal = ShellScriptTestGoal("dev/example")
+    >>> goal.stage.value
+    'pre_build'
+
+    A plain-string stage would silently never match, so it is rejected:
+
+    >>> ShellScriptTestGoal("dev/example", "post_build")
+    Traceback (most recent call last):
+        ...
+    TypeError: Unsupported test stage: 'post_build'
+
+    The path may be supplied as a `str` or `Path`, and is stored as a 
normalized `Path`:
+
+    >>> goal.path == Path("dev/example")
+    True
+    >>> ShellScriptTestGoal(Path("dev/example")) == goal
+    True
+    >>> ShellScriptTestGoal("./dev//example") == goal
+    True
+    >>> len({goal, ShellScriptTestGoal("./dev//example"),
+    ...      ShellScriptTestGoal(Path("dev/example"))})
+    1
+
+    The path is a non-blank, repository-relative script:
+
+    >>> ShellScriptTestGoal("/tmp/example")
+    Traceback (most recent call last):
+        ...
+    ValueError: Shell script path must be relative to repo root: '/tmp/example'
+    >>> ShellScriptTestGoal("  ")
+    Traceback (most recent call last):
+        ...
+    ValueError: Shell script path must be relative to repo root: '  '
+    """
+
+    path: Path
+    stage: TestStage = TestStage.PRE_BUILD
+
+    def __init__(self, path: str | Path, stage: TestStage = 
TestStage.PRE_BUILD) -> None:
+        if not isinstance(stage, TestStage):
+            raise TypeError(f"Unsupported test stage: {stage!r}")
+        # `Path` rejects non-path types and normalizes spellings like 
"./dev//x".
+        path = Path(path)
+        if not str(path).strip() or path == Path() or path.is_absolute() or 
".." in path.parts:

Review Comment:
   Paths can contain whitespace, even though we don't have any such paths in 
the repo today. I would leave out this extra check.



##########
dev/sparktestsupport/modules.py:
##########
@@ -102,6 +105,70 @@ def is_ignored_file(filename: str) -> bool:
     return False
 
 
+class TestStage(Enum):
+    """
+    When a shell script test goal runs in dev/run-tests.py.
+    """
+
+    # Before the Spark build.
+    PRE_BUILD = "pre_build"
+
+
+@dataclass(frozen=True)
+class ShellScriptTestGoal:
+    """
+    Describe a shell script test goal.
+
+    >>> goal = ShellScriptTestGoal("dev/example")
+    >>> goal.stage.value
+    'pre_build'
+
+    A plain-string stage would silently never match, so it is rejected:
+
+    >>> ShellScriptTestGoal("dev/example", "post_build")
+    Traceback (most recent call last):
+        ...
+    TypeError: Unsupported test stage: 'post_build'
+
+    The path may be supplied as a `str` or `Path`, and is stored as a 
normalized `Path`:
+
+    >>> goal.path == Path("dev/example")
+    True
+    >>> ShellScriptTestGoal(Path("dev/example")) == goal
+    True
+    >>> ShellScriptTestGoal("./dev//example") == goal
+    True
+    >>> len({goal, ShellScriptTestGoal("./dev//example"),
+    ...      ShellScriptTestGoal(Path("dev/example"))})
+    1
+
+    The path is a non-blank, repository-relative script:
+
+    >>> ShellScriptTestGoal("/tmp/example")
+    Traceback (most recent call last):
+        ...
+    ValueError: Shell script path must be relative to repo root: '/tmp/example'
+    >>> ShellScriptTestGoal("  ")
+    Traceback (most recent call last):
+        ...
+    ValueError: Shell script path must be relative to repo root: '  '
+    """
+
+    path: Path
+    stage: TestStage = TestStage.PRE_BUILD

Review Comment:
   Good call. Fixed.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to