Conversation
NSagan271
left a comment
There was a problem hiding this comment.
One comment, and also something that came to mind for a next step in the testing restructure: a lot of the tests have to mock components of the system (the engine, the api server data worker, the worker, etc.), an a centralized system for dealing with those would be good to have.
| run: | | ||
| python -m pytest -q -ra --strict-markers --durations=20 \ | ||
| --junitxml=cpu-core.xml \ | ||
| test/modular/test_graph_io.py \ |
There was a problem hiding this comment.
I would prefer something more automated for adding more tests to the core set, either a test/core directory, or a file listing all of the core tests.
| test/modular/test_admit_failure_handling.py \ | ||
| test/modular/test_micro_scheduler.py \ | ||
| test/modular/test_worker_drain.py \ | ||
| test/modular/test_ragged_attention_cpu.py |
There was a problem hiding this comment.
What was the rationale for the core set of tests? I think most seem fundamental, but test_ragged_attention_cpu a test for a specific, not heavily-used resource. And maybe some others should be added, like test/modular/test_shm_tensor_comm.py, test/modular/test_worker_speculation_via_graph_api.py could be added, though coming up with an actual set of core tests would involve heavily cleaning up the tests that we have.
There was a problem hiding this comment.
Besides test_ragged_attention_cpu, the other are fundamental and should be there. test_ragged_attention_cpu is the first attempt to disentangle CPU vs GPU tests (previously it was a bit of a mash for this test). There isn't a special reason this is here, but it's first in a sequence of more refactorings. We can decide to keep or remove bunch of tests once we have a clean CPU suite.
That makes sense to me. I will assign myself an issue to deal with this once I determine a full core set of CPU tests. |
Summary
test_ragged_attention.pyintotest_ragged_attention_cpu.py, so they run without the original module's CUDA requirement. Preserve all moved assertions and all remaining GPU tests.The job uses the existing PR open/update/reopen triggers. Its explicit six-module list avoids collecting unrelated GPU-dependent tests. Making CPU Core a required merge check remains a separate repository setting.
Files changed
.github/workflows/ci.ymlCONTRIBUTING.mdtest/modular/test_ragged_attention.pytest/modular/test_ragged_attention_cpu.pyLocal validation
Fresh disposable Linux aarch64 container, Python 3.12.14, CPU PyTorch 2.9.1.
python -m pip check HF_HUB_OFFLINE=1 PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 \ python -m pytest -q -ra --strict-markers --durations=20 \ test/modular/test_ragged_attention_cpu.py HF_HUB_OFFLINE=1 PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 \ python -m pytest -q -ra --strict-markers --durations=20 \ --junitxml=cpu-core.xml \ test/modular/test_graph_io.py \ test/modular/test_resource_runner.py \ test/modular/test_admit_failure_handling.py \ test/modular/test_micro_scheduler.py \ test/modular/test_worker_drain.py \ test/modular/test_ragged_attention_cpu.py python -m ruff check . git diff --checkpip check, Ruff, and diff whitespace checks passed.bash -n.GitHub validation
CPU Core passed on GitHub-hosted Ubuntu for commit
1bb8ffb71dd3ef988578e07c021b07b6aa3110ff:These are timings from one run, not a latency guarantee. All four CI jobs passed: CPU Core, Ruff/build, Dynamo (48 tests), and Rust transport (17 native tests and 35 Python interoperability tests, plus both builds).