FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Run CI tests with max processes by ShaharNaveh · Pull Request #7254 · RustPython/RustPython · GitHub

/ RustPython Public

Run CI tests with max processes #7254

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

Merged
youknowone merged 7 commits into from
Mar 20, 2026
Conversation 13 Commits 7 Checks 15 Files changed
Merged

Run CI tests with max processes #7254

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7f1ca40
Run CI witb max cores
ShaharNaveh Mar 17, 2026
ea0d06e
Step for flaky MP tests
ShaharNaveh Mar 17, 2026
ccb66b1
Merge remote-tracking branch 'upstream/main' into parallel-ci
ShaharNaveh Mar 18, 2026
b641625
Merge remote-tracking branch 'upstream/main' into parallel-ci
ShaharNaveh Mar 19, 2026
4cf7973
Merge remote-tracking branch 'upstream/main' into parallel-ci
ShaharNaveh Mar 19, 2026
04274b4
Add `test_class` to flkay mp tests
ShaharNaveh Mar 19, 2026
954b0be
Merge remote-tracking branch 'upstream/main' into parallel-ci
ShaharNaveh Mar 20, 2026
File filter

Filter by extension

Filter by extension .yaml  (1) All 1 file type selected
Only manifest files
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
95 changes: 19 additions & 76 deletions .github/workflows/ci.yaml
Show comments Show annotations
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@

- name: Install dependencies
uses: ./.github/actions/install-linux-deps
with: ${{ matrix.dependencies || fromJSON('{}') }}

Check failure on line 144 in .github/workflows/ci.yaml

View workflow job for this annotation

GitHub Actions / format_check 

[actionlint] reported by reviewdog 🐶
"with" section is scalar node but mapping node is expected [syntax-check]

Raw Output:
e:.github/workflows/ci.yaml:144:15: "with" section is scalar node but mapping node is expected [syntax-check]

- uses: dtolnay/rust-toolchain@stable
with:
Expand Down Expand Up @@ -180,76 +180,13 @@
if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip:ci') }}
env:
RUST_BACKTRACE: full
# PLATFORM_INDEPENDENT_TESTS are tests that do not depend on the underlying OS.
# They are currently only run on Linux to speed up the CI.
PLATFORM_INDEPENDENT_TESTS: >-
test__colorize
test_array
test_asyncgen
test_binop
test_bisect
test_bool
test_bytes
test_call
test_cmath
test_collections
test_complex
test_contains
test_copy
test_dataclasses
test_decimal
test_decorators
test_defaultdict
test_deque
test_dict
test_dictcomps
test_dictviews
test_dis
test_enumerate
test_exception_variations
test_float
test_fractions
test_genericalias
test_genericclass
test_grammar
test_range
test_index
test_int
test_int_literal
test_isinstance
test_iter
test_iterlen
test_itertools
test_json
test_keyword
test_keywordonlyarg
test_list
test_long
test_longexp
test_operator
test_ordered_dict
test_pep646_syntax
test_pow
test_raise
test_richcmp
test_scope
test_set
test_slice
test_sort
test_string
test_string_literals
test_strtod
test_structseq
test_subclassinit
test_super
test_syntax
test_tstring
test_tuple
test_unary
test_unpack
test_unpack_ex
test_weakref
test_yield_from
# Tests that can be flaky when running with multiple processes `-j 2`. We will use `-j 1` for these.
FLAKY_MP_TESTS: >-
test_class
test_eintr
test_multiprocessing_fork
test_multiprocessing_forkserver
test_multiprocessing_spawn
name: Run snippets and cpython tests
runs-on: ${{ matrix.os }}
strategy:
Expand Down Expand Up @@ -304,24 +241,30 @@
run: python -m pip install -r requirements.txt && pytest -v
working-directory: ./extra_tests

- name: run cpython platform-independent tests
if: runner.os == 'Linux'
- name: Detect available cores
id: cores
shell: bash
run: |

Check warning on line 247 in .github/workflows/ci.yaml

View workflow job for this annotation

GitHub Actions / format_check 

[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:2:26: Double quote to prevent globbing and word splitting [shellcheck]

Raw Output:
i:.github/workflows/ci.yaml:247:9: shellcheck reported issue in this script: SC2086:info:2:26: Double quote to prevent globbing and word splitting [shellcheck]
target/release/rustpython -m test -j 1 -u all --slowest --fail-env-changed --timeout 600 -v ${{ env.PLATFORM_INDEPENDENT_TESTS }}
timeout-minutes: 45
cores=$(python -c 'print(__import__("os").process_cpu_count())')
echo "cores=${cores}" >> $GITHUB_OUTPUT

Comment on lines +244 to +250

coderabbitai Bot Mar 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

In Python 3.14, can os.process_cpu_count() return None, and is a fallback required before using it as an integer worker count?

💡 Result:

No, os.process_cpu_count does not exist in Python 3.14, and thus cannot return None. The standard function is os.cpu_count, which returns the number of logical CPUs in the system or None if indeterminable. A fallback (e.g., defaulting to 1 or 4) is required when using it as an integer worker count, such as in multiprocessing.Pool(processes=os.cpu_count).

Citations:

🏁 Script executed:

# Check the actual content of lines 243-249 in .github/workflows/ci.yaml
head -n 249 .github/workflows/ci.yaml | tail -n 7

Repository: RustPython/RustPython

Length of output: 280

Fix non-existent function call in core detection.

Line 247 calls os.process_cpu_count(), which does not exist in Python—the correct function is os.cpu_count(), which can also return None. This will cause runtime failure. Additionally, Python for CI test orchestration introduces unnecessary complexity; use bash directly with fallbacks.

🛠️ Suggested fix 
       - name: Detect available cores
         id: cores
         shell: bash
         run: |
-          cores=$(python -c 'print(__import__("os").process_cpu_count())')
-          echo "cores=${cores}" >> $GITHUB_OUTPUT
+          cores="${NUMBER_OF_PROCESSORS:-}"
+          if [ -z "${cores}" ]; then
+            cores="$(getconf _NPROCESSORS_ONLN 2>/dev/null || nproc 2>/dev/null || echo 1)"
+          fi
+          echo "cores=${cores}" >> "$GITHUB_OUTPUT"
📝 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
- name: Detect available cores
id: cores
shell: bash
run: |
target/release/rustpython -m test -j 1 -u all --slowest --fail-env-changed --timeout 600 -v ${{ env.PLATFORM_INDEPENDENT_TESTS }}
timeout-minutes: 45
cores=$(python -c 'print(__import__("os").process_cpu_count())')
echo "cores=${cores}" >> $GITHUB_OUTPUT
- name: Detect available cores
id: cores
shell: bash
run: |
cores="${NUMBER_OF_PROCESSORS:-}"
if [ -z "${cores}" ]; then
cores="$(getconf _NPROCESSORS_ONLN 2>/dev/null || nproc 2>/dev/null || echo 1)"
fi
echo "cores=${cores}" >> "$GITHUB_OUTPUT"
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yaml around lines 243 - 249, The "Detect available
cores" step is calling a non-existent Python function os.process_cpu_count();
update the step (id: cores) to avoid Python and use bash instead: detect CPU
count with a POSIX-safe command such as nproc or getconf _NPROCESSORS_ONLN
(falling back to 2 if detection fails), assign the result to the cores variable
and then echo "cores=${cores}" >> $GITHUB_OUTPUT; ensure the step retains shell:
bash and preserves the output key name and id (cores) so downstream jobs still
read the value.

- name: Run CPython tests
run: |
target/release/rustpython -m test -j ${{ steps.cores.outputs.cores }} ${{ join(matrix.extra_test_args, ' ') }} --slowest --fail-env-changed --timeout 600 -v -x ${{ env.FLAKY_MP_TESTS }} ${{ join(matrix.skips, ' ') }}
timeout-minutes: ${{ matrix.timeout }}
Comment on lines +251 to +254

coderabbitai Bot Mar 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

⚠️ Potential issue | 🟠 Major

This no longer isolates the -j experiment.

By excluding only FLAKY_MP_TESTS here, the workflow is no longer preserving the earlier large platform-independent split and now changes coverage shape at the same time it changes worker count. That makes the CI runtime/stability signal noisy, because this PR is measuring a wider suite distribution and a different parallelism policy in one shot. If the goal is to evaluate max-process CI in isolation, keep the old split for now and only vary -j in this PR.

Also applies to: 258-260

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yaml around lines 251 - 254, The workflow change
altered which tests run while also changing parallelism; revert the test
invocation in the "Run CPython tests" step so the matrix-driven test split is
preserved and only the -j value is varied: keep the original joins/skip logic
(the use of matrix.skips and matrix.extra_test_args) exactly as before and do
not narrow the suite to only exclude FLAKY_MP_TESTS here; instead only change
the -j argument coming from steps.cores.outputs.cores so this PR isolates the
max-process (‑j) experiment without changing the test coverage shape.

ShaharNaveh Mar 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

That's not the goal. it's fine

coderabbitai Bot Mar 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@ShaharNaveh, understood — the goal is simply to reduce CI runtime by running with max processes, not to run a controlled experiment. Apologies for the noise.

🧠 Learnings used 
Learnt from: reactive-firewall
Repo: RustPython/RustPython PR: 0
File: :0-0
Timestamp: 2026-01-19T20:10:58.864Z
Learning: In the RustPython project's CI/CD workflows (PR `#6176`), bash scripts are preferred over Python or Rust for test orchestration and result aggregation to maintain hermetic testing—avoiding circular dependencies where Python tools would depend on the RustPython interpreter being tested, and avoiding visibility loss into Rust runtime issues when using Rust tooling.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-06T05:30:53.056Z
Learning: When comparing behavior with CPython or running test suites, explicitly use the `python` command for CPython and `cargo run --` for RustPython

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.

env:
RUSTPYTHON_SKIP_ENV_POLLUTERS: true

- name: run cpython platform-dependent tests
- name: Run flaky MP CPython tests
run: |
target/release/rustpython -m test -j 1 ${{ join(matrix.extra_test_args, ' ') }} --slowest --fail-env-changed --timeout 600 -v -x ${{ env.PLATFORM_INDEPENDENT_TESTS }} ${{ join(matrix.skips, ' ') }}
target/release/rustpython -m test -j 1 ${{ join(matrix.extra_test_args, ' ') }} --slowest --fail-env-changed --timeout 600 -v ${{ env.FLAKY_MP_TESTS }}
timeout-minutes: ${{ matrix.timeout }}
env:
RUSTPYTHON_SKIP_ENV_POLLUTERS: true

- name: run cpython tests to check if env polluters have stopped polluting
shell: bash
run: |

Check warning on line 267 in .github/workflows/ci.yaml

View workflow job for this annotation

GitHub Actions / format_check 

[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:16:10: Double quote to prevent globbing and word splitting [shellcheck]

Raw Output:
i:.github/workflows/ci.yaml:267:9: shellcheck reported issue in this script: SC2086:info:16:10: Double quote to prevent globbing and word splitting [shellcheck]

Check warning on line 267 in .github/workflows/ci.yaml

View workflow job for this annotation

GitHub Actions / format_check 

[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:12:8: Double quote to prevent globbing and word splitting [shellcheck]

Raw Output:
i:.github/workflows/ci.yaml:267:9: shellcheck reported issue in this script: SC2086:info:12:8: Double quote to prevent globbing and word splitting [shellcheck]
for thing in ${{ join(matrix.env_polluting_tests, ' ') }}; do
for i in $(seq 1 10); do
set +e
Expand Down Expand Up @@ -554,9 +497,9 @@
- name: build rustpython
run: cargo build --release --target wasm32-wasip1 --features freeze-stdlib,stdlib --verbose
- name: run snippets
run: wasmer run --dir $(pwd) target/wasm32-wasip1/release/rustpython.wasm -- "$(pwd)/extra_tests/snippets/stdlib_random.py"

Check warning on line 500 in .github/workflows/ci.yaml

View workflow job for this annotation

GitHub Actions / format_check 

[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2046:warning:1:18: Quote this to prevent word splitting [shellcheck]

Raw Output:
w:.github/workflows/ci.yaml:500:9: shellcheck reported issue in this script: SC2046:warning:1:18: Quote this to prevent word splitting [shellcheck]
- name: run cpython unittest
run: wasmer run --dir $(pwd) target/wasm32-wasip1/release/rustpython.wasm -- "$(pwd)/Lib/test/test_int.py"

Check warning on line 502 in .github/workflows/ci.yaml

View workflow job for this annotation

GitHub Actions / format_check 

[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2046:warning:1:18: Quote this to prevent word splitting [shellcheck]

Raw Output:
w:.github/workflows/ci.yaml:502:9: shellcheck reported issue in this script: SC2046:warning:1:18: Quote this to prevent word splitting [shellcheck]

cargo-shear:
name: cargo shear
Expand Down
Loading
Back | FazBrowse Home | New Git URL