MAINT: Ensure limited set of tests are skipped (#13053)

Co-authored-by: Daniel McCloy <dan@mccloy.info>

Author: larsoner

.github/workflows/tests.yml

@@ -93,6 +93,7 @@ jobs:
         with:
           qt: true
           pyvista: false
+          wm: false
       # Python (if pip)
       - uses: actions/setup-python@v5
         with:
@@ -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/
@@ -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 }}

.pre-commit-config.yaml

@@ -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

azure-pipelines.yml

@@ -88,6 +88,7 @@ stages:
         variables:
           DISPLAY: ':99'
           OPENBLAS_NUM_THREADS: '1'
+          MNE_TEST_ALLOW_SKIP: '^.*(PySide6 causes segfaults).*$'
         steps:
           - bash: |
               set -e
@@ -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'
@@ -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)
@@ -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'
@@ -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
@@ -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:
@@ -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'
@@ -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'

environment.yml

@@ -59,5 +59,5 @@ dependencies:
   - trame
   - trame-vtk
   - trame-vuetify
-  - vtk >=9.2
+  - vtk =9.3.1=qt_*
   - xlrd

mne/conftest.py

@@ -6,6 +6,7 @@
 import inspect
 import os
 import os.path as op
+import re
 import shutil
 import sys
 import warnings
@@ -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 (
@@ -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")
 
@@ -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")

mne/decoding/tests/test_ssd.py

@@ -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)

mne/tests/test_parallel.py

@@ -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

mne/viz/_brain/tests/test_brain.py

@@ -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)

mne/viz/tests/test_evoked.py

@@ -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")

mne/viz/tests/test_raw.py

@@ -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],