From 344a9cef8db02dac79b3f41552ef88bcba6c4954 Mon Sep 17 00:00:00 2001 From: Mohamed Awnallah Date: Wed, 30 Jul 2025 01:44:11 +0000 Subject: [PATCH 1/7] sdks/python/scripts: fix posargs for `run_pytest.sh` --- sdks/python/scripts/run_pytest.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sdks/python/scripts/run_pytest.sh b/sdks/python/scripts/run_pytest.sh index 11bdd4f8577f..392b37f5fa00 100755 --- a/sdks/python/scripts/run_pytest.sh +++ b/sdks/python/scripts/run_pytest.sh @@ -42,14 +42,14 @@ marker_regex="-m\s+('[^']+'|\"[^\"]+\"|[^ ]+)" user_marker="" # Isolate the user-provided -m argument (matching only). -if [[ $pytest_args =~ "-m" ]]; then +if [[ $posargs =~ "-m" ]]; then # Extract the marker value using the defined regex. - user_marker=$(echo "$pytest_args" | sed -nE "s/.*$marker_regex.*/\1/p") + user_marker=$(echo "$posargs" | sed -nE "s/.*$marker_regex.*/\1/p") fi -# Remove the -m argument from pytest_args (substitution only). +# Remove the -m argument from posargs (substitution only). if [[ -n $user_marker ]]; then - pytest_args=$(echo "$pytest_args" | sed -E "s/$marker_regex//") + posargs=$(echo "$posargs" | sed -E "s/$marker_regex//") fi # Combine user-provided marker with script's internal logic. @@ -63,7 +63,7 @@ if [[ -n $user_marker ]]; then fi # Run tests in parallel. -echo "Running parallel tests with: pytest -m \"$marker_for_parallel_tests\" $pytest_args" +echo "Running parallel tests with: pytest -m \"$marker_for_parallel_tests\" $posargs" pytest -v -rs -o junit_suite_name=${envname} \ --junitxml=pytest_${envname}.xml -m "$marker_for_parallel_tests" -n 6 --import-mode=importlib ${pytest_args} --pyargs ${posargs} status1=$? From 3a94a87a529f861dd66b9bb02a926a6a11667238 Mon Sep 17 00:00:00 2001 From: Mohamed Awnallah Date: Wed, 30 Jul 2025 03:11:55 +0000 Subject: [PATCH 2/7] sdks/python/scripts: isolate test paths from pyargs --- sdks/python/scripts/run_pytest.sh | 87 ++++++++++++++++++++++++++----- 1 file changed, 75 insertions(+), 12 deletions(-) diff --git a/sdks/python/scripts/run_pytest.sh b/sdks/python/scripts/run_pytest.sh index 392b37f5fa00..d30a37126ba2 100755 --- a/sdks/python/scripts/run_pytest.sh +++ b/sdks/python/scripts/run_pytest.sh @@ -30,8 +30,15 @@ envname=${1?First argument required: suite base name} posargs=$2 pytest_args=$3 -# strip leading/trailing quotes from posargs because it can get double quoted as its passed through. -posargs=$(sed -e 's/^"//' -e 's/"$//' -e "s/'$//" -e "s/^'//" <<<$posargs) +# strip leading/trailing quotes from posargs because it can get double quoted as +# its passed through. +if [[ $posargs == \"*\" ]]; then + # If wrapped in double quotes, remove them + posargs="${posargs:1:${#posargs}-2}" +elif [[ $posargs == \'*\' ]]; then + # If wrapped in single quotes, remove them. + posargs="${posargs:1:${#posargs}-2}" +fi echo "pytest_args: $pytest_args" echo "posargs: $posargs" @@ -41,17 +48,29 @@ marker_regex="-m\s+('[^']+'|\"[^\"]+\"|[^ ]+)" # Initialize the user_marker variable. user_marker="" -# Isolate the user-provided -m argument (matching only). -if [[ $posargs =~ "-m" ]]; then - # Extract the marker value using the defined regex. - user_marker=$(echo "$posargs" | sed -nE "s/.*$marker_regex.*/\1/p") -fi +# Define regex pattern for quoted strings +quotes_regex="^[\"\'](.*)[\"\']$" -# Remove the -m argument from posargs (substitution only). -if [[ -n $user_marker ]]; then - posargs=$(echo "$posargs" | sed -E "s/$marker_regex//") +# Extract the user markers. +if [[ $posargs =~ $marker_regex ]]; then + # Get the full match including -m and the marker. + full_match="${BASH_REMATCH[0]}" + + # Get the marker with quotes (this is the first capture group). + quoted_marker="${BASH_REMATCH[1]}" + + # Remove any quotes around the marker. + if [[ $quoted_marker =~ $quotes_regex ]]; then + user_marker="${BASH_REMATCH[1]}" + else + user_marker="$quoted_marker" + fi + + # Remove the entire -m marker portion from posargs. + posargs="${posargs/$full_match/}" fi + # Combine user-provided marker with script's internal logic. marker_for_parallel_tests="not no_xdist" marker_for_sequential_tests="no_xdist" @@ -62,16 +81,60 @@ if [[ -n $user_marker ]]; then marker_for_sequential_tests="($user_marker) and ($marker_for_sequential_tests)" fi +# Parse posargs to separate pytest options from test paths. +options="" +test_paths="" + +# Safely split the posargs string into individual arguments. +eval "set -- $posargs" + +# Iterate through arguments. +while [[ $# -gt 0 ]]; do + arg="$1" + shift + + # If argument starts with dash, it's an option. + if [[ "$arg" == -* ]]; then + options+=" $arg" + + # Handle options that take a value (like -k). + if [[ "$arg" == "-k" && $# -gt 0 ]]; then + # Get the next argument. + next_arg="$1" + + # Check if it's quoted and remove quotes if needed. + if [[ $next_arg =~ $quotes_regex ]]; then + # Extract the content inside quotes. + next_arg="${BASH_REMATCH[1]}" + fi + + # Add the unquoted value to options. + options+=" $next_arg" + shift + fi + else + # Otherwise it's a test path. + test_paths+=" $arg" + fi +done + +# Construct the final pytest command arguments. +pyargs_section="" +if [[ -n "$test_paths" ]]; then + pyargs_section="--pyargs $test_paths" +fi +pytest_command_args="$options $pyargs_section" + # Run tests in parallel. echo "Running parallel tests with: pytest -m \"$marker_for_parallel_tests\" $posargs" pytest -v -rs -o junit_suite_name=${envname} \ - --junitxml=pytest_${envname}.xml -m "$marker_for_parallel_tests" -n 6 --import-mode=importlib ${pytest_args} --pyargs ${posargs} + --junitxml=pytest_${envname}.xml -m "$marker_for_parallel_tests" -n 6 --import-mode=importlib ${pytest_args} ${pytest_command_args} status1=$? # Run tests sequentially. echo "Running sequential tests with: pytest -m \"$marker_for_sequential_tests\" $pytest_args" pytest -v -rs -o junit_suite_name=${envname}_no_xdist \ - --junitxml=pytest_${envname}_no_xdist.xml -m "$marker_for_sequential_tests" --import-mode=importlib ${pytest_args} --pyargs ${posargs} + --junitxml=pytest_${envname}_no_xdist.xml -m "$marker_for_sequential_tests" --import-mode=importlib ${pytest_args} ${pytest_command_args} status2=$? # Exit with error if no tests were run in either suite (status code 5). From 4d5f7020aad5de2f7be96dee9f4785fb85b6ad79 Mon Sep 17 00:00:00 2001 From: Mohamed Awnallah Date: Wed, 30 Jul 2025 03:47:15 +0000 Subject: [PATCH 3/7] sdks/python/scripts: address some gemini feedback --- sdks/python/scripts/run_pytest.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sdks/python/scripts/run_pytest.sh b/sdks/python/scripts/run_pytest.sh index d30a37126ba2..7157a30a03df 100755 --- a/sdks/python/scripts/run_pytest.sh +++ b/sdks/python/scripts/run_pytest.sh @@ -32,10 +32,10 @@ pytest_args=$3 # strip leading/trailing quotes from posargs because it can get double quoted as # its passed through. -if [[ $posargs == \"*\" ]]; then +if [[ $posargs == '"'*'"' ]]; then # If wrapped in double quotes, remove them posargs="${posargs:1:${#posargs}-2}" -elif [[ $posargs == \'*\' ]]; then +elif [[ $posargs == "'"*"'" ]]; then # If wrapped in single quotes, remove them. posargs="${posargs:1:${#posargs}-2}" fi @@ -70,7 +70,6 @@ if [[ $posargs =~ $marker_regex ]]; then posargs="${posargs/$full_match/}" fi - # Combine user-provided marker with script's internal logic. marker_for_parallel_tests="not no_xdist" marker_for_sequential_tests="no_xdist" @@ -97,8 +96,9 @@ while [[ $# -gt 0 ]]; do if [[ "$arg" == -* ]]; then options+=" $arg" - # Handle options that take a value (like -k). - if [[ "$arg" == "-k" && $# -gt 0 ]]; then + # Check if there's a next argument and it doesn't start with a dash. + # This assumes it's a value for the current option. + if [[ $# -gt 0 && "$1" != -* ]]; then # Get the next argument. next_arg="$1" @@ -126,13 +126,13 @@ fi pytest_command_args="$options $pyargs_section" # Run tests in parallel. -echo "Running parallel tests with: pytest -m \"$marker_for_parallel_tests\" $posargs" +echo "Running parallel tests with: pytest -m \"$marker_for_parallel_tests\" $pytest_command_args" pytest -v -rs -o junit_suite_name=${envname} \ --junitxml=pytest_${envname}.xml -m "$marker_for_parallel_tests" -n 6 --import-mode=importlib ${pytest_args} ${pytest_command_args} status1=$? # Run tests sequentially. -echo "Running sequential tests with: pytest -m \"$marker_for_sequential_tests\" $pytest_args" +echo "Running sequential tests with: pytest -m \"$marker_for_sequential_tests\" $pytest_command_args" pytest -v -rs -o junit_suite_name=${envname}_no_xdist \ --junitxml=pytest_${envname}_no_xdist.xml -m "$marker_for_sequential_tests" --import-mode=importlib ${pytest_args} ${pytest_command_args} status2=$? From 864683cc5f55db0f3d3a0a6239e0a8b1cdbc8517 Mon Sep 17 00:00:00 2001 From: Mohamed Awnallah Date: Wed, 30 Jul 2025 03:50:12 +0000 Subject: [PATCH 4/7] .github: trigger postcommit python --- .github/trigger_files/beam_PostCommit_Python.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/trigger_files/beam_PostCommit_Python.json b/.github/trigger_files/beam_PostCommit_Python.json index 9e1d1e1b80dd..ca3300f988db 100644 --- a/.github/trigger_files/beam_PostCommit_Python.json +++ b/.github/trigger_files/beam_PostCommit_Python.json @@ -1,5 +1,5 @@ { "comment": "Modify this file in a trivial way to cause this test suite to run.", - "modification": 4 + "modification": 512 } From 0f6d8c0f8650aad59c16486f0f1eea82207e07f5 Mon Sep 17 00:00:00 2001 From: Mohamed Awnallah Date: Wed, 30 Jul 2025 09:46:55 +0000 Subject: [PATCH 5/7] .github: trigger postcommit python --- .github/trigger_files/beam_PostCommit_Python.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/trigger_files/beam_PostCommit_Python.json b/.github/trigger_files/beam_PostCommit_Python.json index ca3300f988db..ed56f65ef50f 100644 --- a/.github/trigger_files/beam_PostCommit_Python.json +++ b/.github/trigger_files/beam_PostCommit_Python.json @@ -1,5 +1,5 @@ { "comment": "Modify this file in a trivial way to cause this test suite to run.", - "modification": 512 + "modification": 32 } From 151e2787ed48251b503b8006061f76fe368eb143 Mon Sep 17 00:00:00 2001 From: Mohamed Awnallah Date: Wed, 30 Jul 2025 10:31:00 +0000 Subject: [PATCH 6/7] .github+scripts: handle quotes properly for `run_pytest.sh` --- .github/workflows/beam_PostCommit_Python.yml | 4 ++-- .github/workflows/beam_PreCommit_Python_ML.yml | 4 ++-- sdks/python/scripts/run_pytest.sh | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/beam_PostCommit_Python.yml b/.github/workflows/beam_PostCommit_Python.yml index 4fc76c50b00e..fef02dc8f92f 100644 --- a/.github/workflows/beam_PostCommit_Python.yml +++ b/.github/workflows/beam_PostCommit_Python.yml @@ -108,8 +108,8 @@ jobs: -PuseWheelDistribution \ -Pposargs="${{ contains(matrix.os, 'self-hosted') && - '-m ''not require_docker_in_docker''' || - '-m ''require_docker_in_docker''' + '-m (not require_docker_in_docker)' || + '-m require_docker_in_docker' }}" \ -PpythonVersion=${{ matrix.python_version }} \ env: diff --git a/.github/workflows/beam_PreCommit_Python_ML.yml b/.github/workflows/beam_PreCommit_Python_ML.yml index 691c21af3ecb..de920428a24b 100644 --- a/.github/workflows/beam_PreCommit_Python_ML.yml +++ b/.github/workflows/beam_PreCommit_Python_ML.yml @@ -105,8 +105,8 @@ jobs: arguments: | -Pposargs="${{ contains(matrix.os, 'self-hosted') && - 'apache_beam/ml/ -m ''not require_docker_in_docker''' || - 'apache_beam/ml/ -m ''require_docker_in_docker''' + 'apache_beam/ml/ -m (not require_docker_in_docker)' || + 'apache_beam/ml/ -m require_docker_in_docker' }}" \ -PpythonVersion=${{ matrix.python_version }} - name: Archive Python Test Results diff --git a/sdks/python/scripts/run_pytest.sh b/sdks/python/scripts/run_pytest.sh index 7157a30a03df..e4acad4840ff 100755 --- a/sdks/python/scripts/run_pytest.sh +++ b/sdks/python/scripts/run_pytest.sh @@ -76,8 +76,8 @@ marker_for_sequential_tests="no_xdist" if [[ -n $user_marker ]]; then # Combine user marker with internal markers. - marker_for_parallel_tests="($user_marker) and ($marker_for_parallel_tests)" - marker_for_sequential_tests="($user_marker) and ($marker_for_sequential_tests)" + marker_for_parallel_tests="$user_marker and ($marker_for_parallel_tests)" + marker_for_sequential_tests="$user_marker and ($marker_for_sequential_tests)" fi # Parse posargs to separate pytest options from test paths. From f53d37b5e19f94a3e494e53595377e4538018fa8 Mon Sep 17 00:00:00 2001 From: Mohamed Awnallah Date: Wed, 30 Jul 2025 10:31:56 +0000 Subject: [PATCH 7/7] .github: trigger postcommit python --- .github/trigger_files/beam_PostCommit_Python.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/trigger_files/beam_PostCommit_Python.json b/.github/trigger_files/beam_PostCommit_Python.json index ed56f65ef50f..1fa29a890c2f 100644 --- a/.github/trigger_files/beam_PostCommit_Python.json +++ b/.github/trigger_files/beam_PostCommit_Python.json @@ -1,5 +1,5 @@ { "comment": "Modify this file in a trivial way to cause this test suite to run.", - "modification": 32 + "modification": 29 }