Skip to content

Commit 8c7799f

Browse files
sfc-gh-mvashishthasfc-gh-mvashishtha
authored
TEST-modin-project#7441: Correctly skip sanity tests if we don't need them. (modin-project#7442)
The "sanity" tests for ray, dask, and unidist are supposed to run a subset of the "test-all" suite. If we detect a change to a particular engine, we're supposed to run the `test-all` job using that engine. However, if we don't detect a change, we're supposed to run the "test-sanity" job using that engine. Prior to this commit, we always run the sanity jobs because the `if` key-value pairs [here](https://github.com/modin-project/modin/blob/4152c95e0b94205015afd4e44563524b2d11f432/.github/workflows/ci.yml#L563-L575) don't do anything. In this commit, use [test matrix exclusions](https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/running-variations-of-jobs-in-a-workflow) to skip executions that we don't need to run. ## Testing I have validated that we only run the expected sanity tests in the following scenarios, which are older commits on the branch I am trying to merge here: - change ray, dask, and unidist => [skip all sanity tests](https://github.com/modin-project/modin/actions/runs/13320322721/job/37203660664?pr=7442) - change ray and unidist only => [run dask sanity test only](https://github.com/modin-project/modin/actions/runs/13335886632/job/37250965225?pr=7442) - change dask and ray only => [run unidist sanity tests only](https://github.com/modin-project/modin/actions/runs/13336232376/job/37252017095?pr=7442) - change dask and unidist only => [run ray sanity tests only](https://github.com/modin-project/modin/actions/runs/13336496671/job/37252848023?pr=7442) - change unidist only => [run ray and dask sanity tests only](https://github.com/modin-project/modin/actions/runs/13335936087/job/37251081720?pr=7442) - change ray only => [run unidist and dask sanity tests only](https://github.com/modin-project/modin/actions/runs/13336041245/job/37251429105?pr=7442) - change dask only => [run ray and unidist sanity tests only](https://github.com/modin-project/modin/actions/runs/13336182248/job/37251864934?pr=7442)
1 parent 4152c95 commit 8c7799f

File tree

1 file changed

+74
-54
lines changed

1 file changed

+74
-54
lines changed

‎.github/workflows/ci.yml

+74-54
Original file line numberDiff line numberDiff line change
@@ -547,41 +547,61 @@ jobs:
547547
# The "sanity" tests run on each pull request to test that a subset of the
548548
# full tests work with the slower engines (ray, dask, and unidist-MPI).
549549
needs: [lint-flake8, execution-filter, python-filter]
550-
if: github.event_name == 'pull_request'
550+
# If we don't need to run any sanity tests, the job matrix that we generate
551+
# here gives a single job with all the matrix fields empty (that is, os,
552+
# execution, etc. are not set, so we treat them as "").
553+
# so, if the matrix is going to be empty, we need to skip this job
554+
# completely. This bizarre behavior is not in the official documentation,
555+
# of GitHub actions matrices, but someone does mention it here:
556+
# https://stackoverflow.com/a/77118991
557+
if: |
558+
github.event_name == 'pull_request' &&
559+
(
560+
needs.execution-filter.outputs.ray != 'true' ||
561+
needs.execution-filter.outputs.dask != 'true' ||
562+
needs.execution-filter.outputs.unidist != 'true'
563+
)
551564
strategy:
552565
matrix:
553566
os:
554567
- ubuntu
555568
- windows
556569
python-version: [ "${{ needs.python-filter.outputs.python-version }}" ]
557-
execution:
558-
- name: ray
559-
shell-ex: "python -m pytest"
560-
# If we're going to run all ray tests because we've detected a
561-
# change to the ray engine, we don't need to run these sanity tests
562-
# on ray.
563-
if: needs.execution-filter.outputs.ray != 'true'
564-
- name: dask
565-
shell-ex: "python -m pytest"
566-
# If we're going to run all dask tests because we've detected a
567-
# change to the dask engine, we don't need to run these sanity tests
568-
# on dask.
569-
if: needs.execution-filter.outputs.dask != 'true'
570-
- name: unidist
571-
shell-ex: "mpiexec -n 1 -genv AWS_ACCESS_KEY_ID foobar_key -genv AWS_SECRET_ACCESS_KEY foobar_secret python -m pytest"
572-
# If we're going to run all unidist tests because we've detected a
573-
# change to the unidist engine, we don't need to run these sanity tests
574-
# on unidist.
575-
if: needs.execution-filter.outputs.unidist != 'true'
570+
running-all-ray-tests: [ "${{ needs.execution-filter.outputs.ray }}" ]
571+
running-all-dask-tests: [ "${{needs.execution-filter.outputs.dask}}" ]
572+
running-all-unidist-tests: [ "${{needs.execution-filter.outputs.unidist}}" ]
573+
execution: [ray, dask, unidist]
574+
# If we're going to run all ray tests because we've detected a
575+
# change to the ray engine, we don't need to run these sanity tests
576+
# on ray. Likewise for dask and unidist.
577+
exclude:
578+
- running-all-ray-tests: 'true'
579+
execution: ray
580+
- running-all-dask-tests: 'true'
581+
execution: dask
582+
- running-all-unidist-tests: 'true'
583+
execution: unidist
576584
runs-on: ${{ matrix.os }}-latest
577585
defaults:
578586
run:
579587
shell: bash -l {0}
580588
env:
581-
MODIN_ENGINE: ${{ matrix.execution.name }}
589+
MODIN_ENGINE: ${{ matrix.execution }}
582590
UNIDIST_BACKEND: "mpi"
583-
PARALLEL: ${{ matrix.execution.name != 'unidist' && matrix.os != 'windows' && '-n 2' || '' }}
584-
name: test-${{ matrix.os }}-sanity (engine ${{ matrix.execution.name }}, python ${{matrix.python-version}})
591+
PARALLEL: ${{ matrix.execution != 'unidist' && matrix.os != 'windows' && '-n 2' || '' }}
592+
PYTEST_COMMAND: >-
593+
${{
594+
(
595+
(matrix.execution == 'ray' || matrix.execution == 'dask') &&
596+
'python -m pytest'
597+
) ||
598+
(
599+
matrix.execution == 'unidist' &&
600+
'mpiexec -n 1 -genv AWS_ACCESS_KEY_ID foobar_key -genv AWS_SECRET_ACCESS_KEY foobar_secret python -m pytest'
601+
) ||
602+
'UNKNOWN_PYTEST_COMMAND'
603+
}}
604+
name: test-${{ matrix.os }}-sanity (engine ${{ matrix.execution }}, python ${{matrix.python-version}})
585605
services:
586606
moto:
587607
image: ${{ matrix.os != 'windows' && 'motoserver/moto:5.0.13' || '' }}
@@ -594,18 +614,18 @@ jobs:
594614
- uses: actions/checkout@v4
595615
- uses: ./.github/actions/mamba-env
596616
with:
597-
environment-file: ${{ matrix.os == 'ubuntu' && matrix.execution.name == 'unidist' && 'requirements/env_unidist_linux.yml' || matrix.os == 'windows' && matrix.execution.name == 'unidist' && 'requirements/env_unidist_win.yml' || 'environment-dev.yml' }}
598-
activate-environment: ${{ matrix.execution.name == 'unidist' && 'modin_on_unidist' || 'modin' }}
617+
environment-file: ${{ matrix.os == 'ubuntu' && matrix.execution == 'unidist' && 'requirements/env_unidist_linux.yml' || matrix.os == 'windows' && matrix.execution == 'unidist' && 'requirements/env_unidist_win.yml' || 'environment-dev.yml' }}
618+
activate-environment: ${{ matrix.execution == 'unidist' && 'modin_on_unidist' || 'modin' }}
599619
python-version: ${{matrix.python-version}}
600620
- name: Install HDF5
601621
run: sudo apt update && sudo apt install -y libhdf5-dev
602622
if: matrix.os != 'windows'
603623
- name: Limit ray memory
604624
run: echo "MODIN_MEMORY=1000000000" >> $GITHUB_ENV
605-
if: matrix.os != 'windows' && matrix.execution.name == 'ray'
625+
if: matrix.os != 'windows' && matrix.execution == 'ray'
606626
- name: Tell Modin to use existing ray cluster
607627
run: echo "MODIN_RAY_CLUSTER=True" >> $GITHUB_ENV
608-
if: matrix.os == 'windows' && matrix.execution.name == 'ray'
628+
if: matrix.os == 'windows' && matrix.execution == 'ray'
609629
- name: Start local ray cluster
610630
# Try a few times to start ray to work around
611631
# https://github.com/modin-project/modin/issues/4562
@@ -614,71 +634,71 @@ jobs:
614634
timeout_minutes: 5
615635
max_attempts: 5
616636
command: ray start --head --port=6379 --object-store-memory=1000000000
617-
if: matrix.os == 'windows' && matrix.execution.name == 'ray'
618-
- run: MODIN_BENCHMARK_MODE=True ${{ matrix.execution.shell-ex }} modin/tests/pandas/internals/test_benchmark_mode.py
619-
- run: ${{ matrix.execution.shell-ex }} $PARALLEL modin/tests/test_partition_api.py
620-
- run: ${{ matrix.execution.shell-ex }} modin/tests/pandas/extensions
637+
if: matrix.os == 'windows' && matrix.execution == 'ray'
638+
- run: MODIN_BENCHMARK_MODE=True $PYTEST_COMMAND modin/tests/pandas/internals/test_benchmark_mode.py
639+
- run: $PYTEST_COMMAND $PARALLEL modin/tests/test_partition_api.py
640+
- run: $PYTEST_COMMAND modin/tests/pandas/extensions
621641
- name: xgboost tests
622642
run: |
623643
# TODO(https://github.com/modin-project/modin/issues/5194): Uncap xgboost
624644
# when we use collective instead of rabit.
625645
mamba install "xgboost>=1.7.1,<2.0.0" scikit-learn -c conda-forge
626-
${{ matrix.execution.shell-ex }} $PARALLEL \
646+
$PYTEST_COMMAND $PARALLEL \
627647
modin/tests/experimental/xgboost/test_default.py \
628648
modin/tests/experimental/xgboost/test_xgboost.py \
629649
modin/tests/experimental/xgboost/test_dmatrix.py
630650
if: matrix.os != 'windows' && needs.execution-filter.outputs.experimental == 'true'
631-
- run: ${{ matrix.execution.shell-ex }} $PARALLEL modin/tests/experimental/test_pipeline.py
632-
if: matrix.os != 'windows' && matrix.execution.name != 'unidist' && needs.execution-filter.outputs.experimental == 'true'
651+
- run: $PYTEST_COMMAND $PARALLEL modin/tests/experimental/test_pipeline.py
652+
if: matrix.os != 'windows' && matrix.execution != 'unidist' && needs.execution-filter.outputs.experimental == 'true'
633653
- name: "test DF: binary, default, iter"
634654
run: |
635-
${{ matrix.execution.shell-ex }} $PARALLEL \
655+
$PYTEST_COMMAND $PARALLEL \
636656
modin/tests/pandas/dataframe/test_binary.py \
637657
modin/tests/pandas/dataframe/test_default.py \
638658
modin/tests/pandas/dataframe/test_iter.py
639659
if: matrix.os != 'windows'
640660
- name: "test DF: reduce, udf, window, pickle"
641661
run: |
642-
${{ matrix.execution.shell-ex }} $PARALLEL \
662+
$PYTEST_COMMAND $PARALLEL \
643663
modin/tests/pandas/dataframe/test_reduce.py \
644664
modin/tests/pandas/dataframe/test_udf.py \
645665
modin/tests/pandas/dataframe/test_window.py \
646666
modin/tests/pandas/dataframe/test_pickle.py
647667
if: matrix.os != 'windows'
648-
- run: ${{ matrix.execution.shell-ex }} modin/tests/pandas/test_series.py
649-
if: matrix.execution.name == 'ray'
650-
- run: ${{ matrix.execution.shell-ex }} -m "not exclude_in_sanity" modin/tests/pandas/test_series.py
651-
if: matrix.execution.name != 'ray'
652-
- run: ${{ matrix.execution.shell-ex }} modin/tests/pandas/dataframe/test_map_metadata.py
653-
if: matrix.execution.name == 'ray'
654-
- run: ${{ matrix.execution.shell-ex }} -m "not exclude_in_sanity" modin/tests/pandas/dataframe/test_map_metadata.py
655-
if: matrix.execution.name != 'ray'
668+
- run: $PYTEST_COMMAND modin/tests/pandas/test_series.py
669+
if: matrix.execution == 'ray'
670+
- run: $PYTEST_COMMAND -m "not exclude_in_sanity" modin/tests/pandas/test_series.py
671+
if: matrix.execution != 'ray'
672+
- run: $PYTEST_COMMAND modin/tests/pandas/dataframe/test_map_metadata.py
673+
if: matrix.execution == 'ray'
674+
- run: $PYTEST_COMMAND -m "not exclude_in_sanity" modin/tests/pandas/dataframe/test_map_metadata.py
675+
if: matrix.execution != 'ray'
656676
- name: "test rolling, expanding, reshape, general, concat"
657677
run: |
658-
${{ matrix.execution.shell-ex }} $PARALLEL \
678+
$PYTEST_COMMAND $PARALLEL \
659679
modin/tests/pandas/test_rolling.py \
660680
modin/tests/pandas/test_expanding.py \
661681
modin/tests/pandas/test_reshape.py \
662682
modin/tests/pandas/test_general.py \
663683
modin/tests/pandas/test_concat.py
664684
if: matrix.os != 'windows'
665-
- run: ${{ matrix.execution.shell-ex }} $PARALLEL modin/tests/numpy
666-
- run: ${{ matrix.execution.shell-ex }} -m "not exclude_in_sanity" modin/tests/pandas/test_io.py --verbose
667-
if: matrix.execution.name != 'unidist'
685+
- run: $PYTEST_COMMAND $PARALLEL modin/tests/numpy
686+
- run: $PYTEST_COMMAND -m "not exclude_in_sanity" modin/tests/pandas/test_io.py --verbose
687+
if: matrix.execution != 'unidist'
668688
- uses: nick-fields/retry@v3
669689
# to avoid issues with non-stable `to_csv` tests for unidist on MPI backend.
670690
# for details see: https://github.com/modin-project/modin/pull/6776
671691
with:
672692
timeout_minutes: 15
673693
max_attempts: 3
674-
command: conda run --no-capture-output -n modin_on_unidist ${{ matrix.execution.shell-ex }} -m "not exclude_in_sanity" modin/tests/pandas/test_io.py --verbose
675-
if: matrix.execution.name == 'unidist'
676-
- run: ${{ matrix.execution.shell-ex }} modin/tests/experimental/test_io_exp.py
677-
- run: ${{ matrix.execution.shell-ex }} $PARALLEL modin/tests/interchange/dataframe_protocol/test_general.py
678-
- run: ${{ matrix.execution.shell-ex }} $PARALLEL modin/tests/interchange/dataframe_protocol/pandas/test_protocol.py
694+
command: conda run --no-capture-output -n modin_on_unidist $PYTEST_COMMAND -m "not exclude_in_sanity" modin/tests/pandas/test_io.py --verbose
695+
if: matrix.execution == 'unidist'
696+
- run: $PYTEST_COMMAND modin/tests/experimental/test_io_exp.py
697+
- run: $PYTEST_COMMAND $PARALLEL modin/tests/interchange/dataframe_protocol/test_general.py
698+
- run: $PYTEST_COMMAND $PARALLEL modin/tests/interchange/dataframe_protocol/pandas/test_protocol.py
679699
- name: Stop local ray cluster
680700
run: ray stop
681-
if: matrix.os == 'windows' && matrix.execution.name == 'ray'
701+
if: matrix.os == 'windows' && matrix.execution == 'ray'
682702
- uses: ./.github/actions/upload-coverage
683703

684704
test-experimental:

0 commit comments

Comments
 (0)