Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .github/workflows/canary_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,17 @@ jobs:

- name: Test with pytest
run: pytest

# Opt-in tests to check network connectivity for elephant-data
# repository on GIN, kept out of regular CI because GIN
# may answer runners with HTTP 403.
- name: Test GIN connectivity
if: ${{ !cancelled() }}
timeout-minutes: 10
env:
ELEPHANT_NETWORK_TESTS: "1"
# Warn only: GIN may answer runners with HTTP 403, which is not an
# elephant failure. The warning shows on the run summary.
run: |
pytest -v elephant/test/test_datasets.py::TestGinConnectivity \
|| echo "::warning title=GIN connectivity::GIN connectivity tests failed (e.g. HTTP 403 from GIN). See the step log."
16 changes: 13 additions & 3 deletions elephant/datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@

ELEPHANT_TMP_DIR = Path(tempfile.gettempdir()) / "elephant"

# Default URL of `elephant-data`, used when `ELEPHANT_DATA_LOCATION` not set.
# This redirects to the current location of `elephant-data`
ELEPHANT_DATA_ROOT_URL = "https://datasets.python-elephant.org/"


# Data generation functions

Expand Down Expand Up @@ -395,9 +399,9 @@ def download_datasets(repo_path, filepath=None, checksum=None,

else:
# The user did not set a URL or path in `ELEPHANT_DATA_LOCATION`.
# Use the default root URL, which redirects to the current location of
# `elephant-data`.
url_to_root = "https://datasets.python-elephant.org/"
# Use the default root URL.
# Read from module at call time, so that tests can patch it
url_to_root = ELEPHANT_DATA_ROOT_URL

# Get the final URL to the current version of `elephant data`
# (version of Elephant is equal to version of `elephant-data`).
Expand All @@ -410,6 +414,9 @@ def download_datasets(repo_path, filepath=None, checksum=None,
urlopen(data_location + '/README.md')

except HTTPError as error:
# Only missing version (404) should fall back to master. (PR 721)
if error.code != 404:
raise
# If the corresponding `elephant-data` version is not found,
# use the latest commit of `elephant-data` (`master` branch).
# This is expected for development versions of Elephant, which may
Expand All @@ -430,6 +437,9 @@ def download_datasets(repo_path, filepath=None, checksum=None,
ctx.check_hostname = True
urlopen(data_location + '/README.md', context=ctx)
except HTTPError as unverified_ctx_error: # e.g. 404
# Only missing version (404) fall back to master. (PR 721)
if unverified_ctx_error.code != 404:
raise
# If it still fails, use latest commit of `elephant-data` in
# the `master` branch.
data_location = url_to_root + "raw/master"
Expand Down
243 changes: 222 additions & 21 deletions elephant/test/test_datasets.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
import unittest
import os
import threading
from functools import partial
from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer
from unittest.mock import patch
from pathlib import Path
from tempfile import TemporaryDirectory, gettempdir
import hashlib
from urllib.error import URLError
import warnings
from urllib.error import HTTPError, URLError
from urllib.parse import urlparse
from urllib.request import urlopen

import numpy as np
from numpy.testing import assert_allclose
import neo
from elephant.datasets import download_datasets, load_data, ELEPHANT_DATA
from elephant.datasets import (download_datasets, load_data, ELEPHANT_DATA,
ELEPHANT_DATA_ROOT_URL)


# Most tests are done with mocking the environment variable to test different
Expand All @@ -21,11 +28,84 @@
HAS_CACHE_DIR = ("ELEPHANT_DATA_LOCATION" in os.environ
and Path(os.environ["ELEPHANT_DATA_LOCATION"]).is_dir())

# Opt in flag for tests that contact the real `elephant-data` repository on GIN
# To run set env val ELEPHANT_NETWORK_TESTS=1.
RUN_NETWORK_TESTS = os.environ.get("ELEPHANT_NETWORK_TESTS") == "1"

# Version the mock server answers with HTTP 403, like GIN sometimes
# answers GitHub runners
FORBIDDEN_VERSION = '6.6.6'


class _QuietHandler(SimpleHTTPRequestHandler):
# To avoid logging each request to stderr
def log_message(self, format, *args):
pass

def do_GET(self):
if self.path.startswith(f'/raw/v{FORBIDDEN_VERSION}/'):
self.send_error(403)
return
super().do_GET()


class TestDownloadDatasets(unittest.TestCase):

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': '/invalid/path'},
clear=True)
# Version served under `raw/v{FAKE_VERSION}`, for the version probe tests
FAKE_VERSION = '9.9.9'

@classmethod
def setUpClass(cls):
"""
Serve a minimal copy of `elephant-data` from 127.0.0.1
so tests don't have to contact GIN. The master branch
and one fake version of the GIN dataset are mocked.
"""
# Mockup a dummy dataset location
cls._serve_dir = TemporaryDirectory()
raw = Path(cls._serve_dir.name) / 'raw'
cls.repo_data_path = 'dataset-1/dataset-1.h5'

# Dummy dataset, content is irrelevant for tests
cls.master_payload = bytes(range(256)) * 4
dataset = raw / 'master' / cls.repo_data_path
dataset.parent.mkdir(parents=True)
dataset.write_bytes(cls.master_payload)

# Versioned copy with different content, so tests can tell which one
# was downloaded. `download_datasets` probes the README of the
# version, so only the versioned copy needs one.
cls.versioned_payload = b'versioned' * 100
versioned = raw / f'v{cls.FAKE_VERSION}'
dataset = versioned / cls.repo_data_path
dataset.parent.mkdir(parents=True)
dataset.write_bytes(cls.versioned_payload)
(versioned / 'README.md').write_text('Mock elephant-data\n')

# Port 0 lets the OS pick a free port
handler = partial(_QuietHandler, directory=cls._serve_dir.name)
cls.server = ThreadingHTTPServer(('127.0.0.1', 0), handler)
cls.root_url = f'http://127.0.0.1:{cls.server.server_port}/'
cls._server_thread = threading.Thread(target=cls.server.serve_forever,
daemon=True)
cls._server_thread.start()

@classmethod
def tearDownClass(cls):
Comment thread
CozySocksAlways marked this conversation as resolved.
Resolved
cls.server.shutdown()
cls.server.server_close()
cls._server_thread.join()
cls._serve_dir.cleanup()

def setUp(self):
# Point the default root URL (used when `ELEPHANT_DATA_LOCATION` is
# empty) at the local server, so no test in this class contacts GIN.
patcher = patch('elephant.datasets.ELEPHANT_DATA_ROOT_URL',
self.root_url)
patcher.start()
self.addCleanup(patcher.stop)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': '/invalid/path'})
def test_invalid_path(self):
repo_path = 'some/repo/path'
with self.assertRaises(ValueError) as error:
Expand All @@ -35,8 +115,7 @@ def test_invalid_path(self):
exception_msg)
self.assertIn("/invalid/path", exception_msg)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': 'ftp://invalid.url'},
clear=True)
@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': 'ftp://invalid.url'})
def test_invalid_url(self):
repo_path = 'some/repo/path'
with self.assertRaises(ValueError) as error:
Expand All @@ -46,19 +125,17 @@ def test_invalid_url(self):
exception_msg)
self.assertIn("ftp://invalid.url", exception_msg)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': 'http://valid.url'},
clear=True)
@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': 'http://valid.url'})
def test_valid_url(self):
repo_path = 'some/repo/path'
self.assertRaises(URLError, download_datasets, repo_path)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''}, clear=True)
@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''})
def test_invalid_data(self):
repo_path = 'some/repo/path'
self.assertRaises(URLError, download_datasets, repo_path)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': 'invalid_path_or_url'},
clear=True)
@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': 'invalid_path_or_url'})
def test_invalid_value(self):
repo_path = 'some/repo/path'
with self.assertRaises(ValueError) as error:
Expand All @@ -68,8 +145,7 @@ def test_invalid_value(self):
exception_msg)
self.assertIn("invalid_path_or_url", exception_msg)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''},
clear=True)
@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''})
def test_valid_data(self):
# This is expected to download a file with the same name in GIN to a
# folder 'elephant' within the current system temporary folder
Expand All @@ -84,8 +160,7 @@ def test_valid_data(self):
self.assertTrue(Path(downloaded_file).is_file())
self.assertEqual(expected_file_path, downloaded_file)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''},
clear=True)
@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''})
def test_valid_data_with_path(self):
# This is expected to download a file to the provided path
# (using a temporary directory specific to the test)
Expand All @@ -98,8 +173,7 @@ def test_valid_data_with_path(self):
self.assertTrue(Path(downloaded_file).is_file())
self.assertEqual(target_file_path, downloaded_file)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''},
clear=True)
@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''})
def test_valid_data_with_integrity_check(self):
# This is expected to check the integrity of the downloaded file by
# comparing the MD5 hash. We manually download the file to the current
Expand All @@ -124,8 +198,7 @@ def test_valid_data_with_integrity_check(self):
self.assertTrue(Path(downloaded_file).is_file())
self.assertEqual(target_file_path, downloaded_file)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''},
clear=True)
@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''})
def test_valid_data_existing(self):
# This is expected to avoid downloading the file two times, since
# the file has been previously downloaded and a checksum is provided.
Expand All @@ -148,8 +221,7 @@ def test_valid_data_existing(self):
self.assertEqual(target_file_path, existing_file)
self.assertEqual(downloaded_file, existing_file)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''},
clear=True)
@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''})
def test_valid_data_with_failed_integrity_check(self):
# This forces a failure by setting an invalid checksum for a dataset
# in GIN.
Expand All @@ -166,6 +238,69 @@ def test_valid_data_with_failed_integrity_check(self):
self.assertIn("Data at", exception_msg)
self.assertIn("does not agree with MD5 hash aaaaaa", exception_msg)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''})
def test_valid_data_versioned(self):
"""
Intent: Ensure versioned dataset download path works
Condition: Version of Elephant exists in (mocked) `elephant-data`
Expected: No warning raised
"""
with (
patch('elephant.datasets._get_version',
return_value=self.FAKE_VERSION),
TemporaryDirectory() as temp_dir,
warnings.catch_warnings(record=True) as caught,
):
warnings.simplefilter('always')
target_file_path = Path(temp_dir) / 'target_versioned'
downloaded_file = download_datasets(self.repo_data_path,
filepath=target_file_path)
self.assertEqual(target_file_path, downloaded_file)
self.assertEqual(self.versioned_payload,
Path(downloaded_file).read_bytes())
self.assertFalse([w for w in caught
if issubclass(w.category, UserWarning)])

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''})
def test_valid_data_version_fallback(self):
"""
Intent: Ensure fallback to master branch of dataset if version not
available
Condition: Version of Elephant does not exist in `elephant-data` (404)
Expected: Raises a warning
"""
with (
patch('elephant.datasets._get_version', return_value='0.0.0'),
TemporaryDirectory() as temp_dir,
):
target_file_path = Path(temp_dir) / 'target_fallback'
with self.assertWarns(UserWarning) as warning:
downloaded_file = download_datasets(self.repo_data_path,
filepath=target_file_path)
self.assertIn("No corresponding version", str(warning.warning))
self.assertEqual(target_file_path, downloaded_file)
self.assertEqual(self.master_payload,
Path(downloaded_file).read_bytes())

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''})
def test_valid_data_version_forbidden(self):
"""
Intent: Ensure only a missing version (404) falls back to master
Condition: Version probe of `elephant-data` answers 403
Expected: HTTPError 403 raised, nothing downloaded from master
"""
with (
patch('elephant.datasets._get_version',
return_value=FORBIDDEN_VERSION),
TemporaryDirectory() as temp_dir,
):
target_file_path = Path(temp_dir) / 'target_forbidden'
with self.assertRaises(HTTPError) as error:
download_datasets(self.repo_data_path,
filepath=target_file_path)
self.assertEqual(403, error.exception.code)
self.assertFalse(target_file_path.exists())

@unittest.skipIf(not HAS_CACHE_DIR,
"Not testing since ELEPHANT_DATA_LOCATION is not set to "
"a folder")
Expand Down Expand Up @@ -290,6 +425,72 @@ def test_download_with_cache_dir_invalid_file(self):
self.assertIn(f"'{repo_path}' does not exist in that path",
exception_msg)


@unittest.skipUnless(RUN_NETWORK_TESTS,
"Not testing GIN connectivity since "
"ELEPHANT_NETWORK_TESTS is not set to 1")
class TestGinConnectivity(unittest.TestCase):
"""
Checks that the real `elephant-data` repository on GIN is reachable via
the default root URL.
"""

GIN_HOST = 'gin.g-node.org'
TIMEOUT = 60 # seconds
repo_data_path = 'dataset-1/dataset-1.h5'

def _open(self, url):
# Re-raise HTTP errors with the URL, so a 403 from GIN is obvious
# in the CI log
try:
return urlopen(url, timeout=self.TIMEOUT)
except HTTPError as error:
self.fail(f"GET {url} failed with HTTP {error.code}: {error}")

def test_root_url_redirects_to_gin(self):
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a mixture of comments with #s and docstrings to detail each unit test throughout Elephant. I like the structure of this. However, it introduces a new style. Is this the best practice we should keep? If so, I suggest also documenting this (ADR) so that we have this in mind and can already adopt it in the WIP PRs.

Intent: Ensure the default root URL reaches GIN
Condition: Receive README of the master branch requested
Expected: HTTP 200, final URL is on GIN
"""
url = ELEPHANT_DATA_ROOT_URL + 'raw/master/README.md'
with self._open(url) as response:
self.assertEqual(200, response.status)
self.assertEqual(self.GIN_HOST,
urlparse(response.geturl()).hostname)

def test_missing_version_probe_is_404(self):
"""
Intent: Ensure the version probe of `download_datasets` gets a real
"not found" for a missing version, not e.g. a 403 (Forbidden)
Condition: README of a version that does not exist is requested
Expected: HTTPError with code 404
"""
url = ELEPHANT_DATA_ROOT_URL + 'raw/v0.0.0/README.md'
with self.assertRaises(HTTPError) as error:
urlopen(url, timeout=self.TIMEOUT)
self.assertEqual(404, error.exception.code)

@patch.dict(os.environ, {'ELEPHANT_DATA_LOCATION': ''})
def test_download_default_url(self):
"""
Intent: Ensure `download_datasets` works end to end against GIN
Condition: `ELEPHANT_DATA_LOCATION` is empty, so the default root URL
and the version probe are used
Expected: The file is downloaded to the requested path without error
"""
# TODO: Discuss with the team what other checks can be done to verify
Comment thread
CozySocksAlways marked this conversation as resolved.
Resolved
# the end-to-end download from GIN (e.g. content/checksum, given that
# the downloaded version may differ from any cached copy).
with TemporaryDirectory() as temp_dir:
target_file_path = Path(temp_dir) / 'dataset-1.h5'
downloaded_file = download_datasets(self.repo_data_path,
filepath=target_file_path,
verbose=False)
self.assertTrue(Path(downloaded_file).is_file())
self.assertEqual(target_file_path, downloaded_file)


class TestLoadData(unittest.TestCase):
Comment thread
CozySocksAlways marked this conversation as resolved.
Resolved

def test_load_data_invalid(self):
Expand Down
Loading