[BBPBGLIB-1097] Reorganization of unit/integration tests (#83)

## Context
This PR addresses a long-standing issue of internal tests all being
mixed (apart from scientific ones).

It meant that tests which required more dependencies needed a special
annotation so that they wouldn't run in the CI job for unit tests. It
also meant that the list of loaded modules for these system tests was
hardcoded, and in particular the NGV test had a pretty elaborated hack
around that.

## Scope
We finally segregate the tests according to their nature and
dependencies, and created the main test dirs:

 - tests/unit
 - tests/integration
 - tests/integration-e2e
 - tests/scientific
 - tests/scientific-ngv

With more groups it is also possible to trivially run tests in parallel,
and therefore we dropped pytest-xdist.

Unit tests notably have very little dependencies, even creating and
exposing a NEURON mock for the individual tests.

Also note that `integration-e2e` contain tests that actually run
end-to-end simulations and may rely on blue brain models. However it's
strongly encouraged future integration tests to only rely on NEURON,
which can be placed in `integration` tests.

This PR
 - Moved the tests to their respective group
 - Adapted tests so they don't have any special annotations
 - Adapted testing system and CI, using three TOX groups:
   - unit
   - integration
   - bbp-model
 - Updated contribution docs

## Review
* [x] PR description is complete
* [x] Coding style (imports, function length, New functions, classes or
files) are good
* [x] Updated Readme, in-code, developer documentation

.gitlab-ci.yml

@@ -24,22 +24,43 @@ variables:
     value: $BLUECONFIGS_BRANCH
     description: 'Name of the blueconfigs branch to test against'
 
-python3-base:
+test-unit:
   extends: .tox-template
   variables:
-    TOXENV: flake8, py3
+    TOXENV: flake8, unit
 
-py38-full-spack:
+test-integration:
   extends: .tox-template
   variables:
-    TOXENV: bb5
+    TOXENV: integration
+
+test-integration-e2e:
+  extends: .tox-template
+  variables:
+    TOXENV: bbp-model
+    TOX_OPTIONS: "-- tests/integration-e2e"
+    # modules need to be loaded in this certain order to avoid mixing of libraries
+    EXTRA_MODULES:
+      unstable:neurodamus-neocortex
+
+test-scientific:
+  extends: .tox-template
+  variables:
+    TOXENV: bbp-model
+    TOX_OPTIONS: "-- tests/scientific"
     # modules need to be loaded in this certain order to avoid mixing of libraries
     EXTRA_MODULES:
-      unstable:py-psutil
       unstable:intel-oneapi-compilers
       unstable:py-bluepy
       unstable:neurodamus-neocortex
-      unstable:py-neurodamus  # needed for dependencies
+
+test-scientific-ngv:
+  extends: .tox-template
+  variables:
+    TOXENV: bbp-model
+    TOX_OPTIONS: "-- tests/scientific-ngv"
+    EXTRA_MODULES:
+      unstable:neurodamus-neocortex-multiscale
 
 set_alt_branches:
   script:

CONTRIBUTING.md

@@ -22,7 +22,7 @@ to our [GitHub Repository][github]. Even better, you can [submit a Pull Request]
 #  Missing a Feature?
 
 You can *request* a new feature by [submitting an issue](#issues) to our GitHub Repository.
-If you would like to *implement* a new feature, please submit an issue with a proposal for your 
+If you would like to *implement* a new feature, please submit an issue with a proposal for your
 work first, to be sure that we can use it.
 
 Please consider what kind of change it is:
@@ -118,23 +118,65 @@ This section applies to both Python versions 2 and 3.
 
 It is recommended to use `virtualenv` to develop in a sandbox environment:
 
-```
+```sh
 virtualenv venv
 . venv/bin/activate
-pip install -r tests/requirement_tests.txt
+
+# Install neurodamus in development mode
+pip install -e .
+
+# Install test requirements
+pip install -r tests/requirements.txt
+```
+
+## Testing
+
+There are several test groups in Neurodamus, from plain unit tests to integration and system tests.
+
+While developing you may want to run unit tests very frequently and thus we suggest running the base
+tests using pytest directly from the dev environment.
+```sh
+pytest tests/unit
 ```
 
-## Build
+For the next stage testing we suggest using the provided tox environments
 
-Run the following command to build incrementally the project: `pip install -e .`
+```sh
+# Integration tests
+tox -e integration
+```
+
+System and scientific tests require Blue Brain models. They therefore depend on neurodamus-neocortex
+`special` builds and should be launched as follows:
 
-## Test
+```sh
+module load unstable neurodamus-neocortex py-neurodamus
+# Integration-e2e tests
+tox -e bbp-model -- tests/integration-e2e
+
+# Scientific tests
+tox -e bbp-model -- tests/scientific
+```
 
-Run the following command to run the Python unit-tests: `pytest tests`
+### Adding more tests
+
+We kindly ask contributors to add tests alongside their new features or enhancements. With the
+previous setup in mind, consider adding test to one or more groups:
+
+ - `tests/unit`: For unit tests of functions with little dependencies. Tests get a few shared mocks,
+namely for NEURON and MPI.
+ - `tests/integration`: For integration tests. Please place here tests around a component which
+   might depend on a number of functions. Tests here can rely on NEURON and the other base
+   dependencies. Additionally tests are provided a `special` with synapse mechanisms so that
+   Neurodamus can be fully initialized.
+ - `tests/integration-e2e`: Place tests here that require launching a top-level Neurodamus instance.
+   Examples of it might be testing modes of operation, parameter handling, or simply larger
+   integration tests which are validated according to the results.
+ - `tests/scientific[-ngv]`: Should contain tests which validate essential scientific features
+   implemented in Neurodamus, namely creation of synapses, replay, NGV, neurodamulation, etc.
 
 ## Coding conventions
 
 The code coverage of the Python unit-tests may not decrease over time.
 It means that every change must go with their corresponding Python unit-tests to
-validate the library behavior as well as to demonstrate the API usage. 
-
+validate the library behavior as well as to demonstrate the API usage.

ci/build_ndcore.sh

@@ -12,7 +12,11 @@ fi
 
 # Get the common synapses
 COMMON_DIR=_common
-git clone [email protected]:hpc/sim/models/common.git $COMMON_DIR  --depth=1
+if [ -d "$COMMON_DIR" ]; then
+    ( cd "$COMMON_DIR" && git pull --quiet )
+else
+    git clone [email protected]:hpc/sim/models/common.git $COMMON_DIR  --depth=1
+fi
 
 MOD_BUILD_DIR="mods.tmp"
 mkdir -p $MOD_BUILD_DIR

setup.cfg

@@ -17,7 +17,7 @@ classifier =
     License :: Other/Proprietary License
 
 [tool:pytest]
-addopts = tests --verbose
+addopts = --verbose
 markers = slow: marks tests as slow
 
 [aliases]
@@ -36,6 +36,6 @@ formats = bdist_wheel
 
 [flake8]
 exclude = .*, __pycache__, .eggs, *.egg, build, dist, docs, venv, *.egg-info, _benchmarks, core
-ignore = E127, E221, E226, E701, W503, W504, E731
+ignore = E127, E221, E226, E701, W503, W504, E731, PT001, PT023
 max-line-length = 100
 # max-complexity = 12

tests/conftest.py

@@ -4,6 +4,16 @@
 USECASE3 = Path(__file__).parent.absolute() / "simulations" / "usecase3"
 
 
+@pytest.fixture(scope="session")
+def rootdir(request):
+    return request.config.rootdir
+
+
+@pytest.fixture(scope="session", name="USECASE3")
+def usecase3_path():
+    return USECASE3
+
+
 @pytest.fixture
 def sonata_config():
     return dict(

tests/integration-e2e/conftest.py

@@ -0,0 +1,8 @@
+import os
+import pytest
+
+assert os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"), "Test requires loading a neocortex model to run"
+
+pytestmark = [
+    pytest.mark.forked,
+]

tests/test_cli_opts.py → tests/integration-e2e/test_cli_opts.py

@@ -1,20 +1,16 @@
 import json
-import os
 from pathlib import Path
 import pytest
 import shutil
 import subprocess
 import tempfile
 
 
-SIM_DIR = Path(__file__).parent.absolute() / "simulations" / "v5_sonata"
+SIM_DIR = Path(__file__).parent.parent.absolute() / "simulations" / "v5_sonata"
 CONFIG_FILE_MINI = "simulation_config_mini.json"
 
 
 @pytest.mark.slow
-@pytest.mark.skipif(
-    not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-    reason="Test requires loading a neocortex model to run")
 def test_save_restore_cli():
     with open(SIM_DIR / CONFIG_FILE_MINI, "r") as f:
         sim_config_data = json.load(f)
@@ -47,9 +43,6 @@ def test_save_restore_cli():
                 subprocess.run(command, check=True)
 
 
-@pytest.mark.skipif(
-    not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-    reason="Test requires loading a neocortex model to run")
 def test_cli_prcellgid():
     test_folder = tempfile.TemporaryDirectory("cli-test-prcellgid")  # auto removed
     test_folder_path = Path(test_folder.name)

tests/test_connection.py → tests/integration-e2e/test_connection.py

@@ -1,18 +1,14 @@
-import os
 import pytest
 from pathlib import Path
 from neurodamus.io.synapse_reader import SynapseParameters
 from neurodamus.node import Node
 
 
-SIM_DIR = Path(__file__).parent.absolute() / "simulations" / "v5_sonata"
+SIM_DIR = Path(__file__).parent.parent.absolute() / "simulations" / "v5_sonata"
 CONFIG_FILE_MINI = SIM_DIR / "simulation_config_mini.json"
 
 
 @pytest.mark.slow
-@pytest.mark.skipif(
-    not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-    reason="Test requires loading a neocortex model to run")
 # This test is to mimic the error reported in HPCTM-1687 during connection.add_syanpses()
 # when detecting conn._synapse_params with more than one element is not None
 def test_add_synapses():

tests/integration-e2e/test_dry_run_worflow.py

@@ -0,0 +1,25 @@
+
+def test_dry_run_workflow(USECASE3):
+    """
+    Test that the dry run mode works
+    """
+
+    from neurodamus import Neurodamus
+    nd = Neurodamus(
+        str(USECASE3 / "simulation_sonata.json"),
+        dry_run=True
+    )
+
+    nd.run()
+
+    assert 20.0 <= nd._dry_run_stats.cell_memory_total <= 30.0
+    assert 0.0 <= nd._dry_run_stats.synapse_memory_total <= 1.0
+    assert 80.0 <= nd._dry_run_stats.base_memory <= 120.0
+    expected_items = {
+        'L4_PC-dSTUT': 2,
+        'L4_MC-dSTUT': 1,
+        'L4_MC-dNAC': 1,
+        'L5_PC-dSTUT': 1
+    }
+    assert nd._dry_run_stats.metype_counts == expected_items
+    assert nd._dry_run_stats.suggest_nodes(0.3) > 0

tests/test_error_handling.py → tests/integration-e2e/test_error_handling.py

@@ -1,21 +1,8 @@
-import os
 import pytest
-from pathlib import Path
 
 
-USECASE3 = Path(__file__).parent.absolute() / "simulations" / "usecase3"
-SONATA_CONF_FILE = str(USECASE3 / "simulation_sonata.json")
-
-pytestmark = [
-    pytest.mark.forked,
-    pytest.mark.skipif(
-        not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-        reason="Test requires loading a neocortex model to run"
-    )
-]
-
-
-def test_handling_neuron_exceptions():
+def test_handling_neuron_exceptions(USECASE3):
+    SONATA_CONF_FILE = str(USECASE3 / "simulation_sonata.json")
     from neurodamus import Node
     n = Node(SONATA_CONF_FILE)
     n.load_targets()

tests/test_loadbalance.py → tests/integration-e2e/test_loadbalance.py

@@ -1,13 +1,13 @@
+"""Tests load balance."""
+# Since a good deal of load balance tests are e2e we put all of them together in this group
 import logging
 import os
 import pytest
 import shutil
 import tempfile
 from pathlib import Path
 
-SIM_DIR = Path(__file__).parent.absolute() / "simulations"
-
-pytestmark = pytest.mark.forked
+SIM_DIR = Path(__file__).parent.parent.absolute() / "simulations"
 
 
 @pytest.fixture
@@ -69,10 +69,6 @@ def circuit_conf():
     )
 
 
-@pytest.mark.skipif(
-    not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-    reason="Test requires loading a neocortex model to run"
-)
 def test_load_balance_integrated(target_manager_hoc, circuit_conf):
     """Comprehensive test using real cells and deriving cx for a sub-target
     """
@@ -102,10 +98,6 @@ def test_load_balance_integrated(target_manager_hoc, circuit_conf):
     assert not lbal._reuse_cell_complexity(TargetSpec(None))
 
 
-@pytest.mark.skipif(
-    not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-    reason="Test requires loading a neocortex model to run"
-)
 def test_multisplit(target_manager_hoc, circuit_conf, capsys):
     """Comprehensive test using real cells, multi-split and complexity derivation
     """
@@ -146,10 +138,6 @@ def test_multisplit(target_manager_hoc, circuit_conf, capsys):
     assert "Target VerySmall is a subset of the target Small" in captured.out
 
 
-@pytest.mark.skipif(
-    not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-    reason="Test requires loading a neocortex model to run"
-)
 def test_loadbal_integration():
     """Ensure given the right files are in the lbal dir, the correct situation is detected
     """

tests/test_multicycle_runs.py → tests/integration-e2e/test_multicycle_runs.py

@@ -3,18 +3,14 @@
 import pytest
 from pathlib import Path
 
-SIM_DIR = Path(__file__).parent.absolute() / "simulations"
+SIM_DIR = Path(__file__).parent.parent.absolute() / "simulations"
 
 
 @pytest.fixture(autouse=True)
-def change_test_dir(monkeypatch, tmp_path):
+def _change_test_dir(monkeypatch, tmp_path):
     monkeypatch.chdir(str(SIM_DIR / "usecase3"))
 
 
-@pytest.mark.forked
-@pytest.mark.skipif(
-    not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-    reason="Test initialize internal structure and clashes with other tests")
 def test_nodeset_target_generate_subtargets():
     from neurodamus.core.nodeset import NodeSet
     from neurodamus.target_manager import NodesetTarget
@@ -45,10 +41,6 @@ def test_nodeset_target_generate_subtargets():
     assert np.array_equal(subtargets[2][1].get_gids(), np.array([1002]))
 
 
-@pytest.mark.forked
-@pytest.mark.skipif(
-    not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-    reason="Test initialize internal structure and clashes with other tests")
 def test_hoc_target_generate_subtargets():
     from neurodamus.target_manager import _HocTarget
 
@@ -94,10 +86,6 @@ def _read_sonata_spike_file(spike_file):
     return timestamps, spike_gids
 
 
-@pytest.mark.forked
-@pytest.mark.skipif(
-    not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-    reason="Test requires loading a neocortex model to run")
 def test_v5_sonata_multisteps():
     import numpy.testing as npt
     from neurodamus import Neurodamus
@@ -126,10 +114,6 @@ def test_v5_sonata_multisteps():
     npt.assert_allclose(timestamps, obtained_timestamps)
 
 
-@pytest.mark.forked
-@pytest.mark.skipif(
-    not os.environ.get("NEURODAMUS_NEOCORTEX_ROOT"),
-    reason="Test requires loading a neocortex model to run")
 def test_usecase3_sonata_multisteps():
     import numpy.testing as npt
     from neurodamus import Neurodamus