X Tutup
Skip to content

Added a test updater tool #6709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
terryluan12 wants to merge 12 commits into from
Closed

Added a test updater tool #6709

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0a6ec87
Added initial update_tests.sh script
terryluan12 Jan 11, 2026
0fe8ac2
Updated update_tests.sh
terryluan12 Jan 11, 2026
0a2ea1b
Added conditioning for erroring tests + moved some logic into separat…
terryluan12 Jan 11, 2026
f8ff03e
Added logic for hanging tests
terryluan12 Jan 11, 2026
965c27a
Added checking existing skipped tests + setting custom timeout + chec…
terryluan12 Jan 11, 2026
f6b69b7
Some quality of life changes + bug fixes
terryluan12 Jan 11, 2026
e9265bd
Added local annotations + added initial concurrency + added a limit o…
terryluan12 Jan 11, 2026
28ef3b4
Added a semaphore + limited number of jobs at the same time
terryluan12 Jan 11, 2026
61e485b
Added backing up and reapplying skips
terryluan12 Jan 11, 2026
fdb476d
Added ignored_libraries for multiprocessing and concurrent libraries
terryluan12 Jan 12, 2026
5b9ae5c
When checking skips skipping tests with no skip in them + updated doc…
terryluan12 Jan 12, 2026
db005d4
Rearranged the update_tests.sh to be easier to read + fixed minor bug…
terryluan12 Jan 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Added initial update_tests.sh script
  • Loading branch information
@terryluan12
terryluan12 committed Jan 11, 2026
commit 0a6ec877b989ce78198e81750c444c4197a00374
73 changes: 73 additions & 0 deletions scripts/update_tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
#!/bin/bash

usage() { echo "Usage: $0 [-c <cpython path>] [-r <rustpython path>] [-u <copy untracked test>]" 1>&2; exit 1; }


cpython_path=""
rpython_path=""
copy_untracked=false
libraries=()

while [[ $# -gt 0 ]]; do
case "$1" in
-c|--cpython-path)
cpython_path="$2"
shift 2
;;
-r|--rpython-path)
rpython_path="$2"
shift 2
;;
-u|--copy-untracked)
copy_untracked=true
shift
;;
*)
libraries+=("$1")
shift
;;
esac
done
Comment on lines +56 to +95
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing validation for required arguments and option values.

Several issues with argument parsing:

  1. Options like -t, -j, -c, -r don't validate that $2 exists before shift 2, which can silently consume the next flag.
  2. No validation that cpython_path is provided when not using --update-skipped.
  3. No validation that paths exist or that timeout/num_jobs are valid integers.
Suggested validation pattern 
         -t|--timeout)
+            if [[ -z "$2" || "$2" == -* ]]; then
+                echo "Error: -t requires a numeric value" >&2
+                exit 1
+            fi
             timeout="$2"
             shift 2
             ;;

Add post-parsing validation:

# After the while loop
if ! $check_skip_flag && [[ -z "$cpython_path" ]]; then
    echo "Error: --cpython-path is required for update mode" >&2
    exit 1
fi

if [[ -z "$rpython_path" ]]; then
    echo "Error: --rpython-path is required" >&2
    exit 1
fi

if [[ ! -d "$cpython_path" ]] && ! $check_skip_flag; then
    echo "Error: CPython path does not exist: $cpython_path" >&2
    exit 1
fi
🤖 Prompt for AI Agents 
In @scripts/update_tests.sh around lines 56 - 95, Argument parsing lacks
validation: ensure each option that does "shift 2" (flags -c/--cpython-path,
-r/--rpython-path, -t/--timeout, -j/--jobs) first checks that a following value
($2) exists and is not another flag before shifting; after the while loop
validate required variables (if check_skip_flag is false require cpython_path,
always require rpython_path), verify that cpython_path and rpython_path point to
existing directories, and validate timeout and num_jobs are positive integers;
update the case branches for cpython_path/rpython_path/timeout/num_jobs to error
and exit when $2 missing, and add post-parsing checks for existence and integer
format for timeout/num_jobs before proceeding.


cpython_path="$cpython_path/Lib/test"
rpython_path="$rpython_path/Lib/test"

Copy link
Contributor

@coderabbitai coderabbitai bot Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing validation for required paths.

If -c or -r are not provided, cpython_path or rpython_path remain empty, resulting in paths like /Lib/test which will silently cause incorrect behavior. Add validation before constructing these paths.

Proposed fix 
+if [[ -z "$rpython_path" ]]; then
+    echo "Error: -r/--rpython-path is required" >&2
+    usage
+fi
+
+if [[ "$check_skip_flag" == false && -z "$cpython_path" ]]; then
+    echo "Error: -c/--cpython-path is required for updating tests" >&2
+    usage
+fi
+
 cpython_path="$cpython_path/Lib/test"
 rpython_path="$rpython_path/Lib/test"
🤖 Prompt for AI Agents 
In @scripts/update_tests.sh around lines 97 - 99, The script constructs
cpython_path and rpython_path without validating that the -c and -r options were
provided, causing values like "/Lib/test"; add a guard after argument parsing to
check that the variables cpython_path and rpython_path are non-empty (and
optionally that the resulting directories exist) and if either is missing print
a clear error/usage message and exit non-zero before the lines that set
cpython_path="$cpython_path/Lib/test" and rpython_path="$rpython_path/Lib/test".
Ensure you reference and validate the same variable names (cpython_path,
rpython_path) and fail-fast so the paths are never built from empty values.


if [[ ${#libraries[@]} -eq 0 ]]; then
libraries=$(find ${cpython_path} -type f -printf "%P\n")
fi
echo "libraries is ${libraries}"

for lib in "${arr[@]}"
do
cpython_file="$cpython_path/$lib"
rpython_file="$rpython_path/$lib"


if [[ $files_equal $cpython_file $rpython_file -eq 0 ]]; then
continue
fi
Comment on lines +132 to +138
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing error handling and variable quoting.

  1. No error handling if lib_updater.py fails - the script continues silently.
  2. Variables passed to annotate_lib should be quoted to handle paths with spaces.
  3. The boolean check $annotate works but [[ $annotate == true ]] is clearer.
Suggested improvements 
     else
         echo "Using lib_updater to update $lib"
-        ./scripts/lib_updater.py --from $clib_path --to $rlib_path -o $rlib_path
+        if ! ./scripts/lib_updater.py --from "$clib_path" --to "$rlib_path" -o "$rlib_path"; then
+            echo "Warning: lib_updater.py failed for $lib" >&2
+        fi
     fi


-    if [[ $annotate && -f "$rlib_path" && $(basename -- "$rlib_path") == test_*.py ]]; then
-        annotate_lib $lib $rlib_path
+    if [[ $annotate == true && -f "$rlib_path" && $(basename -- "$rlib_path") == test_*.py ]]; then
+        annotate_lib "$lib" "$rlib_path"
     fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
./scripts/lib_updater.py --from $clib_path --to $rlib_path -o $rlib_path
fi
if [[ $annotate && -f "$rlib_path" && $(basename -- "$rlib_path") == test_*.py ]]; then
annotate_lib $lib $rlib_path
fi
if ! ./scripts/lib_updater.py --from "$clib_path" --to "$rlib_path" -o "$rlib_path"; then
echo "Warning: lib_updater.py failed for $lib" >&2
fi
fi
if [[ $annotate == true && -f "$rlib_path" && $(basename -- "$rlib_path") == test_*.py ]]; then
annotate_lib "$lib" "$rlib_path"
fi
🤖 Prompt for AI Agents 
In @scripts/update_tests.sh around lines 132 - 138, The block calling
lib_updater.py should check its exit status and fail fast: after running
./scripts/lib_updater.py --from $clib_path --to $rlib_path -o $rlib_path, test
the command's return code and log/exit (e.g., non-zero -> echo error and exit 1)
instead of continuing silently; also change the annotate conditional to
explicitly test [[ $annotate == true ]] and quote variables when calling
annotate_lib (use annotate_lib "$lib" "$rlib_path") so paths with spaces are
handled safely.


if [[ ! -f $cpython_file ]]; then
if $copy_untracked then
echo "Test file $lib missing. Copying..."
cp "$cpython_file" "$rpython_file"
fi
else
echo "Updating $lib..."
./scripts/lib_updater.py --from ${rpython_path}/Lib/test/$lib --to ${rpython_path}/Lib/test/$lib
fi


cargo run
done



files_equal() {
file1=$1
file2=$2
cmp --silent $file1 $file2 && files_equal=0 || files_equal=1
return $files_equal
}

X Tutup