Skip to content

Commit 480fcc8

Browse files
mohamedawnallahmohamedawnallah
authored
sdks/python/scripts: fix posargs for run_pytest.sh (#35733)
* sdks/python/scripts: fix posargs for `run_pytest.sh` * sdks/python/scripts: isolate test paths from pyargs * sdks/python/scripts: address some gemini feedback * .github: trigger postcommit python * .github: trigger postcommit python * .github+scripts: handle quotes properly for `run_pytest.sh` * .github: trigger postcommit python
1 parent 1e1c62a commit 480fcc8

File tree

4 files changed

+84
-21
lines changed

4 files changed

+84
-21
lines changed

‎.github/trigger_files/beam_PostCommit_Python.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
22
"comment": "Modify this file in a trivial way to cause this test suite to run.",
3-
"modification": 4
3+
"modification": 29
44
}
55

‎.github/workflows/beam_PostCommit_Python.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ jobs:
108108
-PuseWheelDistribution \
109109
-Pposargs="${{
110110
contains(matrix.os, 'self-hosted') &&
111-
'-m ''not require_docker_in_docker''' ||
112-
'-m ''require_docker_in_docker'''
111+
'-m (not require_docker_in_docker)' ||
112+
'-m require_docker_in_docker'
113113
}}" \
114114
-PpythonVersion=${{ matrix.python_version }} \
115115
env:

‎.github/workflows/beam_PreCommit_Python_ML.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@ jobs:
105105
arguments: |
106106
-Pposargs="${{
107107
contains(matrix.os, 'self-hosted') &&
108-
'apache_beam/ml/ -m ''not require_docker_in_docker''' ||
109-
'apache_beam/ml/ -m ''require_docker_in_docker'''
108+
'apache_beam/ml/ -m (not require_docker_in_docker)' ||
109+
'apache_beam/ml/ -m require_docker_in_docker'
110110
}}" \
111111
-PpythonVersion=${{ matrix.python_version }}
112112
- name: Archive Python Test Results

‎sdks/python/scripts/run_pytest.sh‎

Lines changed: 79 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,15 @@ envname=${1?First argument required: suite base name}
3030
posargs=$2
3131
pytest_args=$3
3232

33-
# strip leading/trailing quotes from posargs because it can get double quoted as its passed through.
34-
posargs=$(sed -e 's/^"//' -e 's/"$//' -e "s/'$//" -e "s/^'//" <<<$posargs)
33+
# strip leading/trailing quotes from posargs because it can get double quoted as
34+
# its passed through.
35+
if [[ $posargs == '"'*'"' ]]; then
36+
# If wrapped in double quotes, remove them
37+
posargs="${posargs:1:${#posargs}-2}"
38+
elif [[ $posargs == "'"*"'" ]]; then
39+
# If wrapped in single quotes, remove them.
40+
posargs="${posargs:1:${#posargs}-2}"
41+
fi
3542
echo "pytest_args: $pytest_args"
3643
echo "posargs: $posargs"
3744

@@ -41,15 +48,26 @@ marker_regex="-m\s+('[^']+'|\"[^\"]+\"|[^ ]+)"
4148
# Initialize the user_marker variable.
4249
user_marker=""
4350

44-
# Isolate the user-provided -m argument (matching only).
45-
if [[ $pytest_args =~ "-m" ]]; then
46-
# Extract the marker value using the defined regex.
47-
user_marker=$(echo "$pytest_args" | sed -nE "s/.*$marker_regex.*/\1/p")
48-
fi
51+
# Define regex pattern for quoted strings
52+
quotes_regex="^[\"\'](.*)[\"\']$"
4953

50-
# Remove the -m argument from pytest_args (substitution only).
51-
if [[ -n $user_marker ]]; then
52-
pytest_args=$(echo "$pytest_args" | sed -E "s/$marker_regex//")
54+
# Extract the user markers.
55+
if [[ $posargs =~ $marker_regex ]]; then
56+
# Get the full match including -m and the marker.
57+
full_match="${BASH_REMATCH[0]}"
58+
59+
# Get the marker with quotes (this is the first capture group).
60+
quoted_marker="${BASH_REMATCH[1]}"
61+
62+
# Remove any quotes around the marker.
63+
if [[ $quoted_marker =~ $quotes_regex ]]; then
64+
user_marker="${BASH_REMATCH[1]}"
65+
else
66+
user_marker="$quoted_marker"
67+
fi
68+
69+
# Remove the entire -m marker portion from posargs.
70+
posargs="${posargs/$full_match/}"
5371
fi
5472

5573
# Combine user-provided marker with script's internal logic.
@@ -58,20 +76,65 @@ marker_for_sequential_tests="no_xdist"
5876

5977
if [[ -n $user_marker ]]; then
6078
# Combine user marker with internal markers.
61-
marker_for_parallel_tests="($user_marker) and ($marker_for_parallel_tests)"
62-
marker_for_sequential_tests="($user_marker) and ($marker_for_sequential_tests)"
79+
marker_for_parallel_tests="$user_marker and ($marker_for_parallel_tests)"
80+
marker_for_sequential_tests="$user_marker and ($marker_for_sequential_tests)"
81+
fi
82+
83+
# Parse posargs to separate pytest options from test paths.
84+
options=""
85+
test_paths=""
86+
87+
# Safely split the posargs string into individual arguments.
88+
eval "set -- $posargs"
89+
90+
# Iterate through arguments.
91+
while [[ $# -gt 0 ]]; do
92+
arg="$1"
93+
shift
94+
95+
# If argument starts with dash, it's an option.
96+
if [[ "$arg" == -* ]]; then
97+
options+=" $arg"
98+
99+
# Check if there's a next argument and it doesn't start with a dash.
100+
# This assumes it's a value for the current option.
101+
if [[ $# -gt 0 && "$1" != -* ]]; then
102+
# Get the next argument.
103+
next_arg="$1"
104+
105+
# Check if it's quoted and remove quotes if needed.
106+
if [[ $next_arg =~ $quotes_regex ]]; then
107+
# Extract the content inside quotes.
108+
next_arg="${BASH_REMATCH[1]}"
109+
fi
110+
111+
# Add the unquoted value to options.
112+
options+=" $next_arg"
113+
shift
114+
fi
115+
else
116+
# Otherwise it's a test path.
117+
test_paths+=" $arg"
118+
fi
119+
done
120+
121+
# Construct the final pytest command arguments.
122+
pyargs_section=""
123+
if [[ -n "$test_paths" ]]; then
124+
pyargs_section="--pyargs $test_paths"
63125
fi
126+
pytest_command_args="$options $pyargs_section"
64127

65128
# Run tests in parallel.
66-
echo "Running parallel tests with: pytest -m \"$marker_for_parallel_tests\" $pytest_args"
129+
echo "Running parallel tests with: pytest -m \"$marker_for_parallel_tests\" $pytest_command_args"
67130
pytest -v -rs -o junit_suite_name=${envname} \
68-
--junitxml=pytest_${envname}.xml -m "$marker_for_parallel_tests" -n 6 --import-mode=importlib ${pytest_args} --pyargs ${posargs}
131+
--junitxml=pytest_${envname}.xml -m "$marker_for_parallel_tests" -n 6 --import-mode=importlib ${pytest_args} ${pytest_command_args}
69132
status1=$?
70133

71134
# Run tests sequentially.
72-
echo "Running sequential tests with: pytest -m \"$marker_for_sequential_tests\" $pytest_args"
135+
echo "Running sequential tests with: pytest -m \"$marker_for_sequential_tests\" $pytest_command_args"
73136
pytest -v -rs -o junit_suite_name=${envname}_no_xdist \
74-
--junitxml=pytest_${envname}_no_xdist.xml -m "$marker_for_sequential_tests" --import-mode=importlib ${pytest_args} --pyargs ${posargs}
137+
--junitxml=pytest_${envname}_no_xdist.xml -m "$marker_for_sequential_tests" --import-mode=importlib ${pytest_args} ${pytest_command_args}
75138
status2=$?
76139

77140
# Exit with error if no tests were run in either suite (status code 5).

0 commit comments

Comments
 (0)