Actionable comments posted: 4

In @scripts/update_tests.sh:
- Around line 197-200: The if-condition uses the literal word "attempts" instead
of the variable value; change the test from if [[ attempts -gt 10 ]]; then to if
[[ $attempts -gt 10 ]]; then (or [[ $attempts -ge 10 ]] if you prefer inclusive)
so the numeric comparison evaluates the attempts variable; ensure the variable
attempts is initialized and incremented elsewhere so this condition can break
the loop when it exceeds the threshold.
- Line 159: Replace the combined declaration+assignment "local
output=$(rustpython $lib 2>&1)" with a separate declaration and capture so the
command's exit status isn't masked: first declare "local output" (and optionally
"local rc"), then run "output=$(rustpython $lib 2>&1)" and immediately save the
exit code with "rc=$?" (or check $?) before any other commands; use the saved rc
when deciding failure handling for the rustpython invocation.
- Around line 286-297: The readarray lines use unquoted command substitution
which allows shell globbing/word-splitting to corrupt the file list; update the
two occurrences of readarray -t libraries <<< $(find ...) so the command
substitution is quoted (e.g., readarray -t libraries <<< "$(find ...)" ) and
also ensure path vars used inside find (cpython_path and rpython_path) and the
grep pattern expansion (ignored_libraries) are properly quoted/escaped; this
preserves filenames with glob chars or spaces and keeps update_tests and
check_skips receiving the exact file list.
- Around line 96-98: The script sets cpython_path and rpython_path but doesn't
validate them; add a check after those assignments that when running in update
mode (i.e., not using the --update-skipped flag) you verify cpython_path is
non-empty and points to an existing directory (use test -d) and exit with a
clear error if it fails; do the same for rpython_path if it is required by
update operations; reference the cpython_path and rpython_path variables and
ensure any subsequent use assumes the validated paths.

scripts/update_tests.sh (9)

69-72: Redundant exit 1 after usage call.

The usage function already calls exit 1 at line 37, so the explicit exit 1 here is unreachable.

Suggested fix

         -h|--help)
             usage
-            exit 1
             ;;

---

132-134: Unquoted variables and fragile boolean check.

1. $annotate is being used as a command (relying on true/false being executables). This works but is fragile - prefer explicit comparison.
2. Variables passed to annotate_lib should be quoted to handle paths with spaces.

Suggested fix

-    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

---

147-154: Unquoted variables in function calls.

Variables should be quoted to handle paths with spaces correctly.

Suggested fix

 check_skip() {
     local lib=$1
     local rlib_path="$rpython_path/$lib"

-    remove_skips $rlib_path
+    remove_skips "$rlib_path"

-    annotate_lib $lib $rlib_path
+    annotate_lib "$lib" "$rlib_path"
 }

---

175-182: Unquoted variables in loop.

Variables $rlib_path and $test should be quoted to handle paths/names with special characters.

Suggested fix

         for test in "${failed_tests[@]}"
         do
-            if already_failed $rlib_path $test; then
-                replace_expected_with_skip $rlib_path $test
+            if already_failed "$rlib_path" "$test"; then
+                replace_expected_with_skip "$rlib_path" "$test"
             else
-                add_above_test $rlib_path $test "@unittest.expectedFailure # TODO: RUSTPYTHON" 
+                add_above_test "$rlib_path" "$test" "@unittest.expectedFailure # TODO: RUSTPYTHON" 
             fi
         done

---

207-211: Unquoted variables in sed command.

Variables $file and $test_name should be quoted to prevent word splitting and glob expansion issues.

Suggested fix

 replace_expected_with_skip() {
-    file=$1
-    test_name=$2
-    sed -E "/^\s*@unittest\.expectedFailure\s+# TODO: RUSTPYTHON/ { N; /\n\s*def $test_name/ { s/^(\s*)@unittest\.expectedFailure\s+# TODO: RUSTPYTHON/\1@unittest.skip\('TODO: RUSTPYTHON'\)/ } }" -i $file
+    local file=$1
+    local test_name=$2
+    sed -E "/^\s*@unittest\.expectedFailure\s+# TODO: RUSTPYTHON/ { N; /\n\s*def $test_name/ { s/^(\s*)@unittest\.expectedFailure\s+# TODO: RUSTPYTHON/\1@unittest.skip\('TODO: RUSTPYTHON'\)/ } }" -i "$file"
 }

---

213-217: Grep output pollutes stdout; add local declarations.

The already_failed function's grep output goes to stdout, cluttering the script's output. Also, variables should be declared local.

Suggested fix

 already_failed() {
-    file=$1
-    test_name=$2
-    grep -Pz "\s*@unittest\.expectedFailure # TODO: RUSTPYTHON\n\s*def\s+${test_name}\(" $file
+    local file=$1
+    local test_name=$2
+    grep -Pz "\s*@unittest\.expectedFailure # TODO: RUSTPYTHON\n\s*def\s+${test_name}\(" "$file" > /dev/null
 }

---

233-238: Potential sed injection if $line contains special characters.

The $line variable is used directly in the sed replacement without escaping. Characters like /, &, or \ in the line content could break the sed command or cause unexpected substitutions. Currently safe given controlled inputs from this script, but fragile.

Note

Consider escaping special characters in $line if this function might receive arbitrary input in the future:

escaped_line=$(printf '%s\n' "$line" | sed 's/[&/\]/\\&/g')

---

240-248: Unquoted variable at end of sed command.

$rlib_path should be quoted to handle paths with spaces.

Suggested fix

-    sed -i -E '/^[[:space:]]*@unittest\.skip.*\(["'\'']TODO\s?:\s?RUSTPYTHON.*["'\'']\)/Id' $rlib_path
+    sed -i -E '/^[[:space:]]*@unittest\.skip.*\(["'\'']TODO\s?:\s?RUSTPYTHON.*["'\'']\)/Id' "$rlib_path"
 }

---

250-264: Missing local declaration for message variable.

The message variable should be declared local to avoid polluting the global namespace.

Suggested fix

 apply_skip() {
     local rlib_path=$1
     local test_name=$2
     local hanging=$3
-    message="unknown"
+    local message="unknown"

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 00bfea8 and fdb476d.

• scripts/update_tests.sh

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to Lib/** : In the `Lib/` directory, when modifications are necessary, add a `# TODO: RUSTPYTHON` comment or use `unittest.skip('TODO: RustPython <reason>')` or `unittest.expectedFailure` with `# TODO: RUSTPYTHON <reason>` comment

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 6089
File: scripts/lib_updater.py:292-297
Timestamp: 2025-08-30T14:40:05.858Z
Learning: In scripts/lib_updater.py, the --inplace flag intentionally writes to orig_file (not remote_file) even though patches are applied to remote_file content. This workflow allows updating the original RustPython test file with patches applied to new upstream CPython content.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: When testing Python code, always use RustPython instead of the standard `python` command; use `cargo run -- script.py` or `cargo run` for interactive REPL

Learnt from: reactive-firewall
Repo: RustPython/RustPython PR: 6176
File: .github/workflows/Check_Tests.yml:133-141
Timestamp: 2025-09-28T22:22:55.921Z
Learning: In the RustPython project's CI-5974-Test-RustPython-Integration action, the override-rustpython-path input is marked as required but has runtime fallback logic that defaults to RUSTPYTHONPATH environment variable or "Lib" if neither is provided, making explicit specification unnecessary in most cases.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to Lib/** : In the `Lib/` directory, when modifications are necessary, add a `# TODO: RUSTPYTHON` comment or use `unittest.skip('TODO: RustPython <reason>')` or `unittest.expectedFailure` with `# TODO: RUSTPYTHON <reason>` comment

Applied to files:

• scripts/update_tests.sh

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 6089
File: scripts/lib_updater.py:292-297
Timestamp: 2025-08-30T14:40:05.858Z
Learning: In scripts/lib_updater.py, the --inplace flag intentionally writes to orig_file (not remote_file) even though patches are applied to remote_file content. This workflow allows updating the original RustPython test file with patches applied to new upstream CPython content.

Applied to files:

• scripts/update_tests.sh

[warning] 159-159: Declare and assign separately to avoid masking return values.

(SC2155)

[warning] 287-287: Quote this to prevent word splitting.

(SC2046)

[warning] 294-294: Quote this to prevent word splitting.

(SC2046)

• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (windows-2025)
• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Run rust tests (windows-2025)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Check the WASM package and demo
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Run tests under miri
• GitHub Check: Check Rust code with clippy

scripts/update_tests.sh (3)

3-38: Well-documented usage function.

The usage function provides comprehensive documentation including options, examples, notes, and known limitations. This is helpful for users of the script.

---

227-231: Simple semaphore implementation is adequate for the use case.

The busy-wait approach with sleep 0.1 is reasonable for a developer tool. The known limitation about orphaned threads causing deadlock is properly documented in usage.

---

266-281: Backup mechanism for skip reasons is well-designed.

The function properly extracts skip messages and associates them with test names for later restoration. Using declare -gA for the global associative array is appropriate here since it needs to be accessed by apply_skip. Based on learnings, this aligns with the RustPython convention of using TODO: RUSTPYTHON in skip annotations.

Currently fix_test.py have similar categorized feature that appending expectedFailure for failing tests.
I am not sure what you meant by A yet, but B sounds good.
I wish it can be is integrated as fix_test.py --try-unskip.

Could you share examples runs of this script what it solves?
The command and git diff after the command run.

Actionable comments posted: 5

In @scripts/update_tests.sh:
- Around line 202-205: The if condition is comparing the literal string
"attempts" instead of the variable; change the test to reference the variable
(e.g., replace [[ attempts -gt 10 ]] with [[ $attempts -gt 10 ]] or use
arithmetic form (( attempts > 10 ))), so the loop can exit correctly when the
numeric attempts counter exceeds 10; keep the existing echo and return behavior.
- Line 22: In the scripts/update_tests.sh example usage the command contains an
extraneous dash and space ("$0 -c ~/cpython -r - . -t 300"); update that example
to remove the spurious "-" so it reads "$0 -c ~/cpython -r . -t 300" (i.e.,
replace "-r - ." with "-r .") to correct the typo.
- Around line 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.
- Around line 291-293: The unquoted command substitution can cause
word-splitting and the use of find -printf is GNU-specific; fix by replacing the
find -printf pipeline with a portable command and quoting the substitution: run
a subshell that cd's into "${cpython_path}" and uses portable find (e.g. find .
-type f -print) piped to sed 's|^\./||' to strip the leading ./, then grep -vE
"$(IFS=\|; echo "${ignored_libraries[*]}")"; finally assign with readarray -t
libraries <<< "$(cd "${cpython_path}" && find . -type f -print | sed 's|^\./||'
| grep -vE "$(IFS=\|; echo "${ignored_libraries[*]}")")" so variables libraries,
cpython_path and ignored_libraries are quoted and portable tools are used.

scripts/update_tests.sh (8)

70-72: Redundant exit 1 after usage.

The usage function already calls exit 1 on line 38, making this exit statement unreachable.

         -h|--help)
             usage
-            exit 1
             ;;

---

129-129: Quote variables to prevent word splitting.

Arguments should be quoted to handle paths containing spaces.

-        ./scripts/lib_updater.py --from $clib_path --to $rlib_path -o $rlib_path
+        ./scripts/lib_updater.py --from "$clib_path" --to "$rlib_path" -o "$rlib_path"

---

133-135: Quote variables in function call and use explicit boolean comparison.

The boolean check works but is implicit (relies on empty string being falsy). Using explicit comparison is clearer, and arguments should be quoted.

-    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

---

164-164: Declare and assign separately to avoid masking return values.

Per static analysis (SC2155), if the command substitution fails, the exit status is masked by the assignment.

-    local output=$(rustpython $lib 2>&1)
+    local output
+    output=$(rustpython "$lib" 2>&1)

---

180-187: Quote variables in function calls.

Variables passed to functions should be quoted to prevent word splitting issues.

         for test in "${failed_tests[@]}"
         do
-            if already_failed $rlib_path $test; then
-                replace_expected_with_skip $rlib_path $test
+            if already_failed "$rlib_path" "$test"; then
+                replace_expected_with_skip "$rlib_path" "$test"
             else
-                add_above_test $rlib_path $test "@unittest.expectedFailure # TODO: RUSTPYTHON" 
+                add_above_test "$rlib_path" "$test" "@unittest.expectedFailure # TODO: RUSTPYTHON" 
             fi
         done

---

212-216: Quote variables in sed command.

Unquoted $file could break with paths containing spaces.

 replace_expected_with_skip() {
-    file=$1
-    test_name=$2
-    sed -E "/^\s*@unittest\.expectedFailure\s+# TODO: RUSTPYTHON/ { N; /\n\s*def $test_name/ { s/^(\s*)@unittest\.expectedFailure\s+# TODO: RUSTPYTHON/\1@unittest.skip\('TODO: RUSTPYTHON'\)/ } }" -i $file
+    local file=$1
+    local test_name=$2
+    sed -E "/^\s*@unittest\.expectedFailure\s+# TODO: RUSTPYTHON/ { N; /\n\s*def $test_name/ { s/^(\s*)@unittest\.expectedFailure\s+# TODO: RUSTPYTHON/\1@unittest.skip\('TODO: RUSTPYTHON'\)/ } }" -i "$file"
 }

---

238-243: Sed injection risk if $line contains metacharacters.

If the $line argument contains sed metacharacters (e.g., /, &, \), the sed command could break or behave unexpectedly. Since $line is controlled internally (decorator strings), this is low risk but worth noting.

Consider escaping or using a different delimiter if this will handle varied input in the future.

---

252-252: Quote variable in sed command.

-    sed -i -E '/^[[:space:]]*@unittest\.skip.*\(["'\'']TODO\s?:\s?RUSTPYTHON.*["'\'']\)/Id' $rlib_path
+    sed -i -E '/^[[:space:]]*@unittest\.skip.*\(["'\'']TODO\s?:\s?RUSTPYTHON.*["'\'']\)/Id' "$rlib_path"

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between fdb476d and 5b9ae5c.

• scripts/update_tests.sh

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to Lib/** : In the `Lib/` directory, when modifications are necessary, add a `# TODO: RUSTPYTHON` comment or use `unittest.skip('TODO: RustPython <reason>')` or `unittest.expectedFailure` with `# TODO: RUSTPYTHON <reason>` comment

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 6089
File: scripts/lib_updater.py:292-297
Timestamp: 2025-08-30T14:40:05.858Z
Learning: In scripts/lib_updater.py, the --inplace flag intentionally writes to orig_file (not remote_file) even though patches are applied to remote_file content. This workflow allows updating the original RustPython test file with patches applied to new upstream CPython content.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to Lib/** : In the `Lib/` directory, when modifications are necessary, add a `# TODO: RUSTPYTHON` comment or use `unittest.skip('TODO: RustPython <reason>')` or `unittest.expectedFailure` with `# TODO: RUSTPYTHON <reason>` comment

Applied to files:

• scripts/update_tests.sh

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 6089
File: scripts/lib_updater.py:292-297
Timestamp: 2025-08-30T14:40:05.858Z
Learning: In scripts/lib_updater.py, the --inplace flag intentionally writes to orig_file (not remote_file) even though patches are applied to remote_file content. This workflow allows updating the original RustPython test file with patches applied to new upstream CPython content.

Applied to files:

• scripts/update_tests.sh

Learnt from: reactive-firewall
Repo: RustPython/RustPython PR: 6176
File: .github/workflows/Check_Tests.yml:133-141
Timestamp: 2025-09-28T22:22:55.921Z
Learning: In the RustPython project's CI-5974-Test-RustPython-Integration action, the override-rustpython-path input is marked as required but has runtime fallback logic that defaults to RUSTPYTHONPATH environment variable or "Lib" if neither is provided, making explicit specification unnecessary in most cases.

Applied to files:

• scripts/update_tests.sh

crates/vm/src/function/argument.rs (1)

• shift (147-149)

crates/vm/src/exceptions.rs (1)

• basename (2148-2155)

crates/vm/src/builtins/list.rs (1)

• sort (314-328)

crates/vm/src/stdlib/builtins.rs (1)

• all (46-53)

[warning] 164-164: Declare and assign separately to avoid masking return values.

(SC2155)

[warning] 292-292: Quote this to prevent word splitting.

(SC2046)

[warning] 299-299: Quote this to prevent word splitting.

(SC2046)

• GitHub Check: Check Rust code with clippy

scripts/update_tests.sh (1)

228-236: LGTM!

The rustpython wrapper and semaphore implementation are reasonable for this scripting context. The busy-wait in sem is acceptable given the 0.1s sleep.

Actionable comments posted: 5

In @scripts/update_tests.sh:
- Around line 271-275: The helper function replace_expected_with_skip should
quote the $file argument and preserve any existing skip reason instead of
injecting a generic one; update replace_expected_with_skip to call sed on
"$file" (quoted) and change the substitution to capture the original inline
comment/reason (e.g., the text after the # marker or after "TODO: RUSTPYTHON")
and reuse that captured reason inside the @unittest.skip(...) replacement so the
original reason is preserved while converting @unittest.expectedFailure to
@unittest.skip.
- Around line 201-250: In annotate_lib, avoid combining declaration and
assignment for output (currently local output=$(rustpython ...)) so the
rustpython exit code isn’t masked; instead declare local output first, run
output=$(rustpython $lib 2>&1) and capture its exit status ($?) immediately,
logging and returning on a non-zero catastrophic failure; also harden the retry
loop around attempts by detecting unchanged or oscillating output (e.g., keep
previous_output and if output == previous_output or it alternates between two
states for multiple iterations then abort with an error) so the while loop can’t
hang indefinitely even within the 15-attempt cap.
- Around line 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.
- Around line 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.

scripts/update_tests.sh (5)

70-73: Exit code inconsistency for --help.

When --help is explicitly requested, it's conventional to exit with code 0 (success) rather than 1. Currently, both invalid usage and --help exit with code 1.

Suggested fix

         -h|--help)
             usage
-            exit 1
+            exit 0
             ;;

Also update the usage() function to accept an optional exit code:

usage() {
    cat >&2 <<EOF
...
EOF
    exit "${1:-1}"
}

---

98-98: Unused variable RUSTPYTHON_CANONICAL_SKIP_RE.

This constant is defined but never referenced in the script. Either remove it or add a comment explaining its intended future use.

 RUSTPYTHON_POSSIBLE_SKIP_RE="@unittest.skip.*\([\"']TODO:\s*RUSTPYTHON.*[\"']\)"
-RUSTPYTHON_CANONICAL_SKIP_RE="@unittest.skip\('TODO: RUSTPYTHON; .*'\)"
 RUSTPYTHON_CANONICAL_EX_FAILURE="@unittest.expectedFailure # TODO: RUSTPYTHON"

---

258-263: Potential sed pattern injection with special characters.

The $line variable is interpolated directly into the sed replacement string. If $line contains & or \, sed will interpret them specially (& = matched text, \n = backreference). While your current decorators don't contain these, this could cause issues if the pattern evolves.

Additionally, if $test contains regex metacharacters (unlikely for Python function names but possible), the pattern could break.

Consider escaping special characters or using a different approach:

add_above_test() {
    local file=$1
    local test=$2
    local line=$3
    # Escape sed special chars in replacement
    local escaped_line="${line//\\/\\\\}"
    escaped_line="${escaped_line//&/\\&}"
    sed -i "s/^\([[:space:]]*\)def $test(/\1$escaped_line\n\1def $test(/" "$file"
}

---

277-281: Portability concern: grep -P is not universally available.

The -P (Perl regex) flag is a GNU grep extension and is not available on macOS/BSD systems by default. This limits the script's portability.

Consider using grep -E with a different pattern or pcregrep if Perl regex is required. Also quote variables:

 already_failed() {
-    file=$1
-    test_name=$2
-    grep -qPz "$RUSTPYTHON_CANONICAL_EX_FAILURE_RE\n\s*def\s+${test_name}\(" $file
+    local file=$1
+    local test_name=$2
+    grep -qPz "$RUSTPYTHON_CANONICAL_EX_FAILURE_RE\n\s*def\s+${test_name}\(" "$file"
 }

For better portability, consider using awk or sed for multiline matching.

---

104-112: Parallel execution lacks error propagation.

The semaphore pattern works but has limitations:

1. Race condition between checking job count and spawning new job
2. Background job failures are not captured - wait doesn't propagate errors
3. If a job crashes, the script continues without notice

For a tool that's explicitly "not meant to be used as a standalone updater" requiring manual review, this is acceptable but worth noting.

For more robust error handling in Bash 4.3+:

update_tests() {
    local libraries=("$@")
    local pids=()
    for lib in "${libraries[@]}"; do
        sem
        update_test "$lib" &
        pids+=($!)
    done
    local failed=0
    for pid in "${pids[@]}"; do
        wait "$pid" || ((failed++))
    done
    if ((failed > 0)); then
        echo "Warning: $failed update job(s) failed" >&2
    fi
}

Also applies to: 252-256

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 5b9ae5c and db005d4.

• scripts/update_tests.sh

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to Lib/** : In the `Lib/` directory, when modifications are necessary, add a `# TODO: RUSTPYTHON` comment or use `unittest.skip('TODO: RustPython <reason>')` or `unittest.expectedFailure` with `# TODO: RUSTPYTHON <reason>` comment

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 6089
File: scripts/lib_updater.py:292-297
Timestamp: 2025-08-30T14:40:05.858Z
Learning: In scripts/lib_updater.py, the --inplace flag intentionally writes to orig_file (not remote_file) even though patches are applied to remote_file content. This workflow allows updating the original RustPython test file with patches applied to new upstream CPython content.

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 6089
File: scripts/lib_updater.py:292-297
Timestamp: 2025-08-30T14:40:05.858Z
Learning: In scripts/lib_updater.py, the --inplace flag intentionally writes to orig_file (not remote_file) even though patches are applied to remote_file content. This workflow allows updating the original RustPython test file with patches applied to new upstream CPython content.

Applied to files:

• scripts/update_tests.sh

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to Lib/** : In the `Lib/` directory, when modifications are necessary, add a `# TODO: RUSTPYTHON` comment or use `unittest.skip('TODO: RustPython <reason>')` or `unittest.expectedFailure` with `# TODO: RUSTPYTHON <reason>` comment

Applied to files:

• scripts/update_tests.sh

Learnt from: reactive-firewall
Repo: RustPython/RustPython PR: 6176
File: .github/workflows/Check_Tests.yml:133-141
Timestamp: 2025-09-28T22:22:55.921Z
Learning: In the RustPython project's CI-5974-Test-RustPython-Integration action, the override-rustpython-path input is marked as required but has runtime fallback logic that defaults to RUSTPYTHONPATH environment variable or "Lib" if neither is provided, making explicit specification unnecessary in most cases.

Applied to files:

• scripts/update_tests.sh

[warning] 98-98: RUSTPYTHON_CANONICAL_SKIP_RE appears unused. Verify use (or export if used externally).

(SC2034)

[warning] 204-204: Declare and assign separately to avoid masking return values.

(SC2155)

[warning] 304-304: Quote this to prevent word splitting.

(SC2046)

[warning] 312-312: Quote this to prevent word splitting.

(SC2046)

• GitHub Check: Check Rust code with clippy

scripts/update_tests.sh (1)

1-39: Good documentation of usage, examples, and known limitations.

The comprehensive help message with examples and clearly documented limitations helps users understand the tool's scope and rough edges. This aligns well with the author's stated intent that "Care needs to be taken to review everything done."

Based on learnings, the use of @unittest.skip('TODO: RUSTPYTHON...') and @unittest.expectedFailure # TODO: RUSTPYTHON patterns matches the project conventions for marking RustPython-specific test modifications.