Skip to content

Commit

Permalink
MAINT: Ensure limited set of tests are skipped (#13053)
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel McCloy <[email protected]>
  • Loading branch information
larsoner and drammock authored Jan 10, 2025
1 parent 47ea360 commit dedb392
Show file tree
Hide file tree
Showing 18 changed files with 129 additions and 77 deletions.
13 changes: 7 additions & 6 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ jobs:
with:
qt: true
pyvista: false
wm: false
# Python (if pip)
- uses: actions/setup-python@v5
with:
Expand All @@ -115,13 +116,13 @@ jobs:
create-args: >-
python=${{ env.PYTHON_VERSION }}
if: ${{ !startswith(matrix.kind, 'pip') }}
- run: ./tools/github_actions_dependencies.sh
- run: bash ./tools/github_actions_dependencies.sh
# Minimal commands on Linux (macOS stalls)
- run: ./tools/get_minimal_commands.sh
- run: bash ./tools/get_minimal_commands.sh
if: startswith(matrix.os, 'ubuntu') && matrix.kind != 'minimal' && matrix.kind != 'old'
- run: ./tools/github_actions_infos.sh
- run: bash ./tools/github_actions_infos.sh
# Check Qt
- run: ./tools/check_qt_import.sh $MNE_QT_BACKEND
- run: bash ./tools/check_qt_import.sh $MNE_QT_BACKEND
if: env.MNE_QT_BACKEND != ''
- name: Run tests with no testing data
run: MNE_SKIP_TESTING_DATASET_TESTS=true pytest -m "not (ultraslowtest or pgtest)" --tb=short --cov=mne --cov-report xml -vv -rfE mne/
Expand All @@ -131,8 +132,8 @@ jobs:
with:
key: ${{ env.TESTING_VERSION }}
path: ~/mne_data
- run: ./tools/github_actions_download.sh
- run: ./tools/github_actions_test.sh
- run: bash ./tools/github_actions_download.sh
- run: bash ./tools/github_actions_test.sh # for some reason on macOS we need to run "bash X" in order for a failed test run to show up
- uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
Expand Down
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ repos:
name: Copy dependency changes from pyproject.toml to environment.yml
language: python
entry: ./tools/hooks/update_environment_file.py
files: pyproject.toml
files: '^(pyproject.toml|tools/hooks/update_environment_file.py)$'
- repo: local
hooks:
- id: dependency-sync
name: Copy core dependencies from pyproject.toml to README.rst
language: python
entry: ./tools/hooks/sync_dependencies.py
files: pyproject.toml
files: '^(pyproject.toml|tools/hooks/sync_dependencies.py)$'
additional_dependencies: ["mne==1.9.0"]

# zizmor
Expand Down
23 changes: 10 additions & 13 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ stages:
variables:
DISPLAY: ':99'
OPENBLAS_NUM_THREADS: '1'
MNE_TEST_ALLOW_SKIP: '^.*(PySide6 causes segfaults).*$'
steps:
- bash: |
set -e
Expand All @@ -111,7 +112,7 @@ stages:
- bash: |
set -e
python -m pip install --progress-bar off --upgrade pip
python -m pip install --progress-bar off "mne-qt-browser[opengl] @ git+https://github.com/mne-tools/mne-qt-browser.git@main" pyvista scikit-learn pytest-error-for-skips python-picard qtpy nibabel sphinx-gallery "PySide6!=6.8.0,!=6.8.0.1"
python -m pip install --progress-bar off "mne-qt-browser[opengl] @ git+https://github.com/mne-tools/mne-qt-browser.git@main" pyvista scikit-learn python-picard qtpy nibabel sphinx-gallery "PySide6!=6.8.0,!=6.8.0.1" pandas neo pymatreader antio defusedxml
python -m pip uninstall -yq mne
python -m pip install --progress-bar off --upgrade -e .[test]
displayName: 'Install dependencies with pip'
Expand All @@ -132,7 +133,7 @@ stages:
displayName: 'Cache testing data'
- script: python -c "import mne; mne.datasets.testing.data_path(verbose=True)"
displayName: 'Get test data'
- script: pytest --error-for-skips -m "ultraslowtest or pgtest" --tb=short --cov=mne --cov-report=xml --cov-report=html -vv mne
- script: pytest -m "ultraslowtest or pgtest" --tb=short --cov=mne --cov-report=xml -vv mne
displayName: 'slow and mne-qt-browser tests'
# Coverage
- bash: bash <(curl -s https://codecov.io/bash)
Expand All @@ -144,19 +145,18 @@ stages:
testRunTitle: 'Publish test results for $(Agent.JobName)'
failTaskOnFailedTests: true
condition: succeededOrFailed()
- task: PublishCodeCoverageResults@1
- task: PublishCodeCoverageResults@2
inputs:
codeCoverageTool: Cobertura
summaryFileLocation: '$(System.DefaultWorkingDirectory)/**/coverage.xml'
reportDirectory: '$(System.DefaultWorkingDirectory)/**/htmlcov'

- job: Qt
pool:
vmImage: 'ubuntu-22.04'
variables:
DISPLAY: ':99'
OPENBLAS_NUM_THREADS: '1'
TEST_OPTIONS: "--tb=short --cov=mne --cov-report=xml --cov-report=html --cov-append -vv mne/viz/_brain mne/viz/backends mne/viz/tests/test_evoked.py mne/gui mne/report"
TEST_OPTIONS: "--tb=short --cov=mne --cov-report=xml --cov-append -vv mne/viz/_brain mne/viz/backends mne/viz/tests/test_evoked.py mne/gui mne/report"
MNE_TEST_ALLOW_SKIP: '^.*(PySide6 causes segfaults).*$'
steps:
- bash: ./tools/setup_xvfb.sh
displayName: 'Install Ubuntu dependencies'
Expand Down Expand Up @@ -192,6 +192,7 @@ stages:
set -eo pipefail
python -m pip install PyQt6
LD_DEBUG=libs python -c "from PyQt6.QtWidgets import QApplication, QWidget; app = QApplication([]); import matplotlib; matplotlib.use('QtAgg'); import matplotlib.pyplot as plt; plt.figure()"
displayName: 'Check Qt import'
- bash: |
set -eo pipefail
mne sys_info -pd
Expand Down Expand Up @@ -226,11 +227,9 @@ stages:
testRunTitle: 'Publish test results for $(Agent.JobName)'
failTaskOnFailedTests: true
condition: succeededOrFailed()
- task: PublishCodeCoverageResults@1
- task: PublishCodeCoverageResults@2
inputs:
codeCoverageTool: Cobertura
summaryFileLocation: '$(System.DefaultWorkingDirectory)/**/coverage.xml'
reportDirectory: '$(System.DefaultWorkingDirectory)/**/htmlcov'

- job: Windows
pool:
Expand Down Expand Up @@ -285,7 +284,7 @@ stages:
displayName: 'Cache testing data'
- script: python -c "import mne; mne.datasets.testing.data_path(verbose=True)"
displayName: 'Get test data'
- script: pytest -m "not (slowtest or pgtest)" --tb=short --cov=mne --cov-report=xml --cov-report=html -vv mne
- script: pytest -m "not (slowtest or pgtest)" --tb=short --cov=mne --cov-report=xml -vv mne
displayName: 'Run tests'
- bash: bash <(curl -s https://codecov.io/bash)
displayName: 'Codecov'
Expand All @@ -296,8 +295,6 @@ stages:
testRunTitle: 'Publish test results for $(Agent.JobName) $(TEST_MODE) $(PYTHON_VERSION)'
failTaskOnFailedTests: true
condition: succeededOrFailed()
- task: PublishCodeCoverageResults@1
- task: PublishCodeCoverageResults@2
inputs:
codeCoverageTool: Cobertura
summaryFileLocation: '$(System.DefaultWorkingDirectory)/**/coverage.xml'
reportDirectory: '$(System.DefaultWorkingDirectory)/**/htmlcov'
2 changes: 1 addition & 1 deletion environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,5 @@ dependencies:
- trame
- trame-vtk
- trame-vuetify
- vtk >=9.2
- vtk =9.3.1=qt_*
- xlrd
57 changes: 54 additions & 3 deletions mne/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import inspect
import os
import os.path as op
import re
import shutil
import sys
import warnings
Expand Down Expand Up @@ -79,7 +80,7 @@
collect_ignore = ["export/_brainvision.py", "export/_eeglab.py", "export/_edf.py"]


def pytest_configure(config):
def pytest_configure(config: pytest.Config):
"""Configure pytest options."""
# Markers
for marker in (
Expand Down Expand Up @@ -650,6 +651,11 @@ def _check_skip_backend(name):
pytest.skip("Test skipped, requires Qt.")
else:
assert name == "notebook", name
pytest.importorskip("jupyter")
pytest.importorskip("ipympl")
pytest.importorskip("trame")
pytest.importorskip("trame_vtk")
pytest.importorskip("trame_vuetify")
if not _notebook_vtk_works():
pytest.skip("Test skipped, requires working notebook vtk")

Expand Down Expand Up @@ -1178,10 +1184,55 @@ def qt_windows_closed(request):

@pytest.hookimpl(tryfirst=True, hookwrapper=True)
def pytest_runtest_makereport(item, call):
"""Stash the status of each item."""
"""Stash the status of each item and turn unexpected skips into errors."""
outcome = yield
rep = outcome.get_result()
rep: pytest.TestReport = outcome.get_result()
item.stash.setdefault(_phase_report_key, {})[rep.when] = rep
_modify_report_skips(rep)
return rep


@pytest.hookimpl(tryfirst=True, hookwrapper=True)
def pytest_make_collect_report(collector: pytest.Collector):
"""Turn unexpected skips during collection (e.g., module-level) into errors."""
outcome = yield
rep: pytest.CollectReport = outcome.get_result()
_modify_report_skips(rep)
return rep


# Default means "allow all skips". Can use something like "$." to mean
# "never match", i.e., "treat all skips as errors"
_valid_skips_re = re.compile(os.getenv("MNE_TEST_ALLOW_SKIP", ".*"))


# To turn unexpected skips into errors, we need to look both at the collection phase
# (for decorated tests) and the call phase (for things like `importorskip`
# within the test body). code adapted from pytest-error-for-skips
def _modify_report_skips(report: pytest.TestReport | pytest.CollectReport):
if not report.skipped:
return
if isinstance(report.longrepr, tuple):
file, lineno, reason = report.longrepr
else:
file, lineno, reason = "<unknown>", 1, str(report.longrepr)
if _valid_skips_re.match(reason):
return
assert isinstance(report, pytest.TestReport | pytest.CollectReport), type(report)
if file.endswith("doctest.py"): # _python/doctest.py
return
# xfail tests aren't true "skips" but show up as skipped in reports
if getattr(report, "keywords", {}).get("xfail", False):
return
# the above only catches marks, so we need to actually parse the report to catch
# an xfail based on the traceback
if " pytest.xfail( " in reason:
return
if reason.startswith("Skipped: "):
reason = reason[9:]
report.longrepr = f"{file}:{lineno}: UNEXPECTED SKIP: {reason}"
# Make it show up as an error in the report
report.outcome = "error" if isinstance(report, pytest.TestReport) else "failed"


@pytest.fixture(scope="function")
Expand Down
2 changes: 1 addition & 1 deletion mne/decoding/tests/test_ssd.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,5 +474,5 @@ def test_non_full_rank_data():

ssd = SSD(info, filt_params_signal, filt_params_noise)
if sys.platform == "darwin":
pytest.skip("Unknown linalg bug (Accelerate?)")
pytest.xfail("Unknown linalg bug (Accelerate?)")
ssd.fit(X)
2 changes: 1 addition & 1 deletion mne/tests/test_parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def test_parallel_func(n_jobs):
"""Test Parallel wrapping."""
joblib = pytest.importorskip("joblib")
if os.getenv("MNE_FORCE_SERIAL", "").lower() in ("true", "1"):
pytest.skip("MNE_FORCE_SERIAL cannot be set")
pytest.skip("MNE_FORCE_SERIAL is set")

def fun(x):
return x * 2
Expand Down
8 changes: 0 additions & 8 deletions mne/viz/_brain/tests/test_brain.py
Original file line number Diff line number Diff line change
Expand Up @@ -850,14 +850,6 @@ def tiny(tmp_path):
def test_brain_screenshot(renderer_interactive_pyvistaqt, tmp_path, brain_gc):
"""Test time viewer screenshot."""
# This is broken on Conda + GHA for some reason
from qtpy import API_NAME

if (
os.getenv("CONDA_PREFIX", "") != ""
and os.getenv("GITHUB_ACTIONS", "") == "true"
or API_NAME.lower() == "pyside6"
):
pytest.skip("Test is unreliable on GitHub Actions conda runs and pyside6")
tiny_brain, ratio = tiny(tmp_path)
img_nv = tiny_brain.screenshot(time_viewer=False)
want = (_TINY_SIZE[1] * ratio, _TINY_SIZE[0] * ratio, 3)
Expand Down
6 changes: 4 additions & 2 deletions mne/viz/tests/test_evoked.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,10 @@ def test_plot_evoked_image():

ch_names = evoked.ch_names[3:5]
picks = [evoked.ch_names.index(ch) for ch in ch_names]
evoked.plot_image(show_names="all", time_unit="s", picks=picks)
yticklabels = plt.gca().get_yticklabels()
fig = evoked.plot_image(show_names="all", time_unit="s", picks=picks)
fig.canvas.draw_idle()
yticklabels = fig.axes[0].get_yticklabels()
assert len(yticklabels) == len(ch_names)
for tick_target, tick_observed in zip(ch_names, yticklabels):
assert tick_target in str(tick_observed)
evoked.plot_image(show_names=True, time_unit="s")
Expand Down
6 changes: 2 additions & 4 deletions mne/viz/tests/test_raw.py
Original file line number Diff line number Diff line change
Expand Up @@ -862,16 +862,14 @@ def test_remove_annotations(raw, hide_which, browser_backend):
assert len(raw.annotations) == len(hide_which)


def test_merge_annotations(raw, browser_backend):
def test_merge_annotations(raw, pg_backend):
"""Test merging of annotations in the Qt backend.
Let's not bother in figuring out on which sample the _fake_click actually
dropped the annotation, especially with the 600.614 Hz weird sampling rate.
-> atol = 10 / raw.info["sfreq"]
"""
if browser_backend.name == "matplotlib":
pytest.skip("The MPL backend does not support draggable annotations.")
elif not check_version("mne_qt_browser", "0.5.3"):
if not check_version("mne_qt_browser", "0.5.3"):
pytest.xfail("mne_qt_browser < 0.5.3 does not merge annotations properly")
annot = Annotations(
onset=[1, 3, 4, 5, 7, 8],
Expand Down
1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ addopts = """--durations=20 --doctest-modules -rfEXs --cov-report= --tb=short \
--ignore=mne/gui/_*.py --ignore=mne/icons --ignore=tools \
--ignore=mne/report/js_and_css \
--color=yes --capture=sys"""
junit_family = "xunit2"

[tool.rstcheck]
ignore_directives = [
Expand Down
1 change: 1 addition & 0 deletions tools/azure_dependencies.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ if [ "${TEST_MODE}" == "pip" ]; then
elif [ "${TEST_MODE}" == "pip-pre" ]; then
${SCRIPT_DIR}/install_pre_requirements.sh
python -m pip install $STD_ARGS --pre -e .[test_extra]
echo "##vso[task.setvariable variable=MNE_TEST_ALLOW_SKIP].*(Requires (spm|brainstorm) dataset|Requires MNE-C|CUDA not|Numba not| on Windows|MNE_FORCE_SERIAL|PySide6 causes segfaults).*"
else
echo "Unknown run type ${TEST_MODE}"
exit 1
Expand Down
10 changes: 5 additions & 5 deletions tools/get_minimal_commands.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export MNE_ROOT="${PWD}/minimal_cmds"
export PATH=${MNE_ROOT}/bin:$PATH
if [ "${GITHUB_ACTIONS}" == "true" ]; then
echo "Setting MNE_ROOT for GHA"
echo "MNE_ROOT=${MNE_ROOT}" >> $GITHUB_ENV;
echo "MNE_ROOT=${MNE_ROOT}" | tee -a $GITHUB_ENV;
echo "${MNE_ROOT}/bin" >> $GITHUB_PATH;
elif [ "${AZURE_CI}" == "true" ]; then
echo "Setting MNE_ROOT for Azure"
Expand All @@ -33,9 +33,9 @@ if [[ "${CI_OS_NAME}" != "macos"* ]]; then
export NEUROMAG2FT_ROOT="${PWD}/minimal_cmds/bin"
export FREESURFER_HOME="${MNE_ROOT}"
if [ "${GITHUB_ACTIONS}" == "true" ]; then
echo "LD_LIBRARY_PATH=${LD_LIBRARY_PATH}" >> "$GITHUB_ENV";
echo "NEUROMAG2FT_ROOT=${NEUROMAG2FT_ROOT}" >> "$GITHUB_ENV";
echo "FREESURFER_HOME=${FREESURFER_HOME}" >> "$GITHUB_ENV";
echo "LD_LIBRARY_PATH=${LD_LIBRARY_PATH}" | tee -a "$GITHUB_ENV";
echo "NEUROMAG2FT_ROOT=${NEUROMAG2FT_ROOT}" | tee -a "$GITHUB_ENV";
echo "FREESURFER_HOME=${FREESURFER_HOME}" | tee -a "$GITHUB_ENV";
fi;
if [ "${AZURE_CI}" == "true" ]; then
echo "##vso[task.setvariable variable=LD_LIBRARY_PATH]${LD_LIBRARY_PATH}"
Expand All @@ -57,7 +57,7 @@ else
export DYLD_LIBRARY_PATH=${MNE_ROOT}/lib:$DYLD_LIBRARY_PATH
if [ "${GITHUB_ACTIONS}" == "true" ]; then
echo "Setting variables for GHA"
echo "DYLD_LIBRARY_PATH=${DYLD_LIBRARY_PATH}" >> "$GITHUB_ENV";
echo "DYLD_LIBRARY_PATH=${DYLD_LIBRARY_PATH}" | tee -a "$GITHUB_ENV";
set -x
wget https://github.com/XQuartz/XQuartz/releases/download/XQuartz-2.7.11/XQuartz-2.7.11.dmg
sudo hdiutil attach XQuartz-2.7.11.dmg
Expand Down
2 changes: 1 addition & 1 deletion tools/get_testing_version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ TESTING_VERSION=`grep -o "testing=\"[0-9.]\+\"" mne/datasets/config.py | cut -d
# This can be incremented to start fresh when the cache misbehaves, e.g.:
# TESTING_VERSION=${TESTING_VERSION}-1
if [ ! -z $GITHUB_ENV ]; then
echo "TESTING_VERSION="$TESTING_VERSION >> $GITHUB_ENV
echo "TESTING_VERSION="$TESTING_VERSION | tee -a $GITHUB_ENV
elif [ ! -z $AZURE_CI ]; then
echo "##vso[task.setvariable variable=testing_version]$TESTING_VERSION"
elif [ ! -z $CIRCLECI ]; then
Expand Down
28 changes: 16 additions & 12 deletions tools/github_actions_env_vars.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,30 @@ set -eo pipefail -x
if [[ "$MNE_CI_KIND" == "pip"* ]]; then
echo "Setting pip env vars for $MNE_CI_KIND"
if [[ "$MNE_CI_KIND" == "pip-pre" ]]; then
echo "MNE_QT_BACKEND=PyQt6" >> $GITHUB_ENV
echo "MNE_QT_BACKEND=PyQt6" | tee -a $GITHUB_ENV
# We should test an eager import somewhere, might as well be here
echo "EAGER_IMPORT=true" >> $GITHUB_ENV
echo "EAGER_IMPORT=true" | tee -a $GITHUB_ENV
# Make sure nothing unexpected is skipped
echo "MNE_TEST_ALLOW_SKIP=.*(Requires (spm|brainstorm) dataset|CUDA not|Numba not|PySide6 causes segfaults).*" | tee -a $GITHUB_ENV
else
echo "MNE_QT_BACKEND=PySide6" >> $GITHUB_ENV
echo "MNE_QT_BACKEND=PySide6" | tee -a $GITHUB_ENV
fi
else # conda-like
echo "Setting conda env vars for $MNE_CI_KIND"
if [[ "$MNE_CI_KIND" == "old" ]]; then
echo "CONDA_ENV=tools/environment_old.yml" >> $GITHUB_ENV
echo "MNE_IGNORE_WARNINGS_IN_TESTS=true" >> $GITHUB_ENV
echo "MNE_SKIP_NETWORK_TESTS=1" >> $GITHUB_ENV
echo "MNE_QT_BACKEND=PyQt5" >> $GITHUB_ENV
echo "CONDA_ENV=tools/environment_old.yml" | tee -a $GITHUB_ENV
echo "MNE_IGNORE_WARNINGS_IN_TESTS=true" | tee -a $GITHUB_ENV
echo "MNE_SKIP_NETWORK_TESTS=1" | tee -a $GITHUB_ENV
echo "MNE_QT_BACKEND=PyQt5" | tee -a $GITHUB_ENV
elif [[ "$MNE_CI_KIND" == "minimal" ]]; then
echo "CONDA_ENV=tools/environment_minimal.yml" >> $GITHUB_ENV
echo "MNE_QT_BACKEND=PySide6" >> $GITHUB_ENV
echo "CONDA_ENV=tools/environment_minimal.yml" | tee -a $GITHUB_ENV
echo "MNE_QT_BACKEND=PySide6" | tee -a $GITHUB_ENV
else # conda, mamba (use warning level for completeness)
echo "CONDA_ENV=environment.yml" >> $GITHUB_ENV
echo "MNE_LOGGING_LEVEL=warning" >> $GITHUB_ENV
echo "MNE_QT_BACKEND=PySide6" >> $GITHUB_ENV
echo "CONDA_ENV=environment.yml" | tee -a $GITHUB_ENV
echo "MNE_LOGGING_LEVEL=warning" | tee -a $GITHUB_ENV
echo "MNE_QT_BACKEND=PySide6" | tee -a $GITHUB_ENV
# TODO: Also need "|unreliable on GitHub Actions conda" on macOS, but omit for now to make sure the failure actually shows up
echo "MNE_TEST_ALLOW_SKIP=.*(Requires (spm|brainstorm) dataset|CUDA not|PySide6 causes segfaults).*" | tee -a $GITHUB_ENV
fi
fi
set +x
Loading

0 comments on commit dedb392

Please sign in to comment.