From a3039f509b92928fb07d56abfa2407e9bb8b7984 Mon Sep 17 00:00:00 2001
From: Tom Nicholas <tom@cworthy.org>
Date: Mon, 17 Jun 2024 15:10:49 -0600
Subject: [PATCH] Mark tests which require network access (#144)

* added network marker for tests requiring network access

* applied marker to existing test that requires reading from S3

* ensure network tests are run in CI

* add requires_s3fs decorator and use it

* remove now-unneeded importorskip call

* try to fix calling tests in CI

* add entry to release notes
---
 .github/workflows/main.yml        |  2 +-
 docs/releases.rst                 |  3 +++
 virtualizarr/tests/__init__.py    | 28 +++++++++++++++++++++++++
 virtualizarr/tests/conftest.py    | 18 ++++++++++++++++
 virtualizarr/tests/test_xarray.py | 35 +++++++++++++++++--------------
 5 files changed, 69 insertions(+), 17 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 3ca9cf32..3e63feab 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -41,7 +41,7 @@ jobs:
 
       - name: Running Tests
         run: |
-          python -m pytest --cov=./ --cov-report=xml --verbose
+          python -m pytest ./virtualizarr --run-network-tests --cov=./ --cov-report=xml --verbose
 
       - name: Upload code coverage to Codecov
         uses: codecov/codecov-action@v3.1.4
diff --git a/docs/releases.rst b/docs/releases.rst
index b7ab9273..2255ac10 100644
--- a/docs/releases.rst
+++ b/docs/releases.rst
@@ -29,6 +29,9 @@ Documentation
 Internal Changes
 ~~~~~~~~~~~~~~~~
 
+- Mark tests which require network access so that they are only run when `--run-network-tests` is passed a command-line argument to pytest.
+  (:pull:`144`) By `Tom Nicholas <https://github.com/TomNicholas>`_.
+
 .. _v0.1:
 
 v0.1 (17th June 2024)
diff --git a/virtualizarr/tests/__init__.py b/virtualizarr/tests/__init__.py
index 65486273..d8eecb82 100644
--- a/virtualizarr/tests/__init__.py
+++ b/virtualizarr/tests/__init__.py
@@ -1,11 +1,39 @@
+import importlib
 import itertools
 
 import numpy as np
+import pytest
+from packaging.version import Version
 
 from virtualizarr.manifests import ChunkManifest, ManifestArray
 from virtualizarr.manifests.manifest import join
 from virtualizarr.zarr import ZArray, ceildiv
 
+network = pytest.mark.network
+
+
+def _importorskip(
+    modname: str, minversion: str | None = None
+) -> tuple[bool, pytest.MarkDecorator]:
+    try:
+        mod = importlib.import_module(modname)
+        has = True
+        if minversion is not None:
+            v = getattr(mod, "__version__", "999")
+            if Version(v) < Version(minversion):
+                raise ImportError("Minimum version not satisfied")
+    except ImportError:
+        has = False
+
+    reason = f"requires {modname}"
+    if minversion is not None:
+        reason += f">={minversion}"
+    func = pytest.mark.skipif(not has, reason=reason)
+    return has, func
+
+
+has_s3fs, requires_s3fs = _importorskip("s3fs")
+
 
 def create_manifestarray(
     shape: tuple[int, ...], chunks: tuple[int, ...]
diff --git a/virtualizarr/tests/conftest.py b/virtualizarr/tests/conftest.py
index 08651353..8d44454b 100644
--- a/virtualizarr/tests/conftest.py
+++ b/virtualizarr/tests/conftest.py
@@ -2,6 +2,24 @@
 import xarray as xr
 
 
+def pytest_addoption(parser):
+    """Add command-line flags for pytest."""
+    parser.addoption("--run-flaky", action="store_true", help="runs flaky tests")
+    parser.addoption(
+        "--run-network-tests",
+        action="store_true",
+        help="runs tests requiring a network connection",
+    )
+
+
+def pytest_runtest_setup(item):
+    # based on https://stackoverflow.com/questions/47559524
+    if "network" in item.keywords and not item.config.getoption("--run-network-tests"):
+        pytest.skip(
+            "set --run-network-tests to run tests requiring an internet connection"
+        )
+
+
 @pytest.fixture
 def netcdf4_file(tmpdir):
     # Set up example xarray dataset
diff --git a/virtualizarr/tests/test_xarray.py b/virtualizarr/tests/test_xarray.py
index 695759bd..4c7af400 100644
--- a/virtualizarr/tests/test_xarray.py
+++ b/virtualizarr/tests/test_xarray.py
@@ -9,6 +9,7 @@
 
 from virtualizarr import open_virtual_dataset
 from virtualizarr.manifests import ChunkManifest, ManifestArray
+from virtualizarr.tests import network, requires_s3fs
 from virtualizarr.zarr import ZArray
 
 
@@ -270,22 +271,24 @@ def test_combine_by_coords(self, netcdf4_files):
         assert combined_vds.xindexes["time"].to_pandas_index().is_monotonic_increasing
 
 
-pytest.importorskip("s3fs")
-
-
-@pytest.mark.parametrize(
-    "filetype", ["netcdf4", None], ids=["netcdf4 filetype", "None filetype"]
-)
-@pytest.mark.parametrize("indexes", [None, {}], ids=["None index", "empty dict index"])
-def test_anon_read_s3(filetype, indexes):
-    """Parameterized tests for empty vs supplied indexes and filetypes."""
-    # TODO: Switch away from this s3 url after minIO is implemented.
-    fpath = "s3://carbonplan-share/virtualizarr/local.nc"
-    vds = open_virtual_dataset(fpath, filetype=filetype, indexes=indexes)
-
-    assert vds.dims == {"time": 2920, "lat": 25, "lon": 53}
-    for var in vds.variables:
-        assert isinstance(vds[var].data, ManifestArray), var
+@network
+@requires_s3fs
+class TestReadFromS3:
+    @pytest.mark.parametrize(
+        "filetype", ["netcdf4", None], ids=["netcdf4 filetype", "None filetype"]
+    )
+    @pytest.mark.parametrize(
+        "indexes", [None, {}], ids=["None index", "empty dict index"]
+    )
+    def test_anon_read_s3(self, filetype, indexes):
+        """Parameterized tests for empty vs supplied indexes and filetypes."""
+        # TODO: Switch away from this s3 url after minIO is implemented.
+        fpath = "s3://carbonplan-share/virtualizarr/local.nc"
+        vds = open_virtual_dataset(fpath, filetype=filetype, indexes=indexes)
+
+        assert vds.dims == {"time": 2920, "lat": 25, "lon": 53}
+        for var in vds.variables:
+            assert isinstance(vds[var].data, ManifestArray), var
 
 
 class TestLoadVirtualDataset: