From 079afdc8028ab4daad2487f8bfb75635e8d6f81d Mon Sep 17 00:00:00 2001
From: Ludovico Bianchi <lbianchi@lbl.gov>
Date: Fri, 18 Aug 2023 16:44:44 -0500
Subject: [PATCH] Make pytest the only requirement for running test suite
 without errors (#1088)

* Switch to find_packages() and add missing __init__.py files

* Add missing pyyaml dependency

* Handle requests as optional dependency

* Handle JSON schema dependencies as optional

* Handle mongomock dependency as optional

* Remove spurious undefined pytest marker

* Remove redundant notebook tests

* Add missing requests requirement for parameter_sweep

* Change CI job for noneditable install to only require pytest

* Address unused imports pylint failures in unchanged files

Probably they have been ignored until now because not in a package?

* Add yet another WET import guard for mongomock

* Add missing __init__.py files for package data to work correctly

* Address spurious Pylint failure

* Modify "Getting Started" docs to install pytest instead

* Replace most import guards with Pyomo's attempt_import()

* Address Pylint failure
---
 .github/workflows/checks.yml                  |   2 +-
 docs/getting_started.rst                      |   4 +-
 setup.py                                      |   6 +-
 watertap/edb/commands.py                      |  14 ++-
 watertap/edb/data/__init__.py                 |   0
 watertap/edb/tests/test_commands.py           |   4 +-
 watertap/edb/tests/test_edb.py                |   8 +-
 watertap/edb/tests/test_schemas.py            |   4 +
 watertap/edb/tests/util.py                    |   8 +-
 watertap/examples/chemistry/tests/conftest.py |   8 +-
 .../chemistry/tests/test_notebooks.py         | 109 ------------------
 .../chemistry/tests/test_tutorials.py         |  55 ---------
 watertap/examples/edb/tests/conftest.py       |   8 +-
 .../paper_analysis_baselines/__init__.py      |   0
 .../parameter_sweep_baselines/__init__.py     |   0
 watertap/tools/parallel/__init__.py           |   0
 .../tools/parameter_sweep/parameter_sweep.py  |   9 +-
 .../tests/test_parameter_sweep.py             |   5 +-
 .../unit_models/tests/test_cstr_injection.py  |   1 -
 .../unit_models/tests/test_electroNP_ZO.py    |   1 -
 watertap/unit_models/translators/__init__.py  |   0
 .../translators/translator_asm1_adm1.py       |   1 -
 .../translators/translator_asm2d_adm1.py      |   1 -
 23 files changed, 64 insertions(+), 184 deletions(-)
 create mode 100644 watertap/edb/data/__init__.py
 delete mode 100644 watertap/examples/chemistry/tests/test_notebooks.py
 delete mode 100644 watertap/examples/chemistry/tests/test_tutorials.py
 create mode 100644 watertap/examples/flowsheets/lsrro/tests/paper_analysis_baselines/__init__.py
 create mode 100644 watertap/examples/flowsheets/lsrro/tests/parameter_sweep_baselines/__init__.py
 create mode 100644 watertap/tools/parallel/__init__.py
 create mode 100644 watertap/unit_models/translators/__init__.py

diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml
index dff736ef8c..717f42c8ac 100644
--- a/.github/workflows/checks.yml
+++ b/.github/workflows/checks.yml
@@ -204,7 +204,7 @@ jobs:
     - name: Install watertap and testing dependencies
       run: |
         echo '::group::Output of "pip install" commands'
-        pip install --progress-bar off "watertap[testing] @ ${_install_url}"
+        pip install --progress-bar off "watertap @ ${_install_url}" pytest
         echo '::endgroup::'
         echo '::group::Display installed packages'
         conda list
diff --git a/docs/getting_started.rst b/docs/getting_started.rst
index 91d96a7170..8d2e10030f 100644
--- a/docs/getting_started.rst
+++ b/docs/getting_started.rst
@@ -112,11 +112,11 @@ Next, we can obtain Ipopt and CBC from conda-forge:
 Running the test suite
 ----------------------
 
-To run the WaterTAP test suite, first install the optional testing dependencies using pip:
+To run the WaterTAP test suite, first install the ``pytest`` test framework:
 
 .. code-block:: shell
 
-   pip install "watertap[testing]"
+   pip install pytest
 
 Then, run the following command to run the complete WaterTAP test suite:
 
diff --git a/setup.py b/setup.py
index 5d24e1f908..6764c098f4 100644
--- a/setup.py
+++ b/setup.py
@@ -14,7 +14,7 @@
 """
 
 # Always prefer setuptools over distutils
-from setuptools import setup, find_namespace_packages
+from setuptools import setup, find_packages
 from pathlib import Path
 
 cwd = Path(__file__).parent
@@ -72,7 +72,7 @@
     ],
     keywords="water systems, chemical engineering, process modeling, filtration, desalination, nawi",
     # just include watertap and everything under it
-    packages=find_namespace_packages(
+    packages=find_packages(
         include=("watertap*",),
     ),
     python_requires=">=3.7",
@@ -81,6 +81,7 @@
         # maintainers: switch to SPECIAL_DEPENDENCIES_FOR_RELEASE when cutting a release of watertap
         *SPECIAL_DEPENDENCIES_FOR_PRERELEASE,
         "pyomo>=6.6.1",  # (also needed for units in electrolyte database (edb))
+        "pyyaml",  # watertap.core.wt_database
         # the following requirements are for the electrolyte database (edb)
         "pymongo>3",  # database interface
         "fastjsonschema",  # schema validation
@@ -90,6 +91,7 @@
         "scipy",
         # for parameter_sweep
         "h5py",
+        "requests",
         # for watertap.ui.api_model (though may be generally useful)
         "pydantic < 2",
         "numpy",
diff --git a/watertap/edb/commands.py b/watertap/edb/commands.py
index 707323f129..f0f114f078 100644
--- a/watertap/edb/commands.py
+++ b/watertap/edb/commands.py
@@ -22,9 +22,10 @@
 
 # third-party
 import click
-from json_schema_for_humans import generate as schema_gen
-from json_schema_for_humans.generation_configuration import GenerationConfiguration
 from pymongo.errors import ConnectionFailure
+from pyomo.common.dependencies import attempt_import
+
+_, json_schema_available = attempt_import("json_schema_for_humans")
 
 # package
 from .db_api import ElectrolyteDB
@@ -399,6 +400,15 @@ def drop_database(url, database, yes):
     "-d", "--database", help="Database name", default=ElectrolyteDB.DEFAULT_DB
 )
 def schema(output_file, output_format, data_type, url, database):
+    if not json_schema_available:
+        raise ImportError(
+            "json-schema-for-humans (EDB optional dependency) not installed"
+        )
+    from json_schema_for_humans import generate as schema_gen
+    from json_schema_for_humans.generation_configuration import (
+        GenerationConfiguration,
+    )
+
     print_messages = _log.isEnabledFor(logging.ERROR)
     if output_file:
         stream = output_file
diff --git a/watertap/edb/data/__init__.py b/watertap/edb/data/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/watertap/edb/tests/test_commands.py b/watertap/edb/tests/test_commands.py
index 8791756dae..ec8c836d65 100644
--- a/watertap/edb/tests/test_commands.py
+++ b/watertap/edb/tests/test_commands.py
@@ -107,7 +107,9 @@ def __call__(self, url=None, **kwargs) -> EDBClient:
 
 @pytest.fixture(scope="function")
 def mock_edb(monkeypatch) -> EDBClientFactory:
-    import mongomock
+    mongomock = pytest.importorskip(
+        "mongomock", reason="mongomock (EDB optional dependency) not available"
+    )
 
     # NOTE since mongomock clients store data in memory,
     # the same MongoClient instance must be kept and used for the lifetime of the fixture
diff --git a/watertap/edb/tests/test_edb.py b/watertap/edb/tests/test_edb.py
index 487f9683dd..f70ab1945b 100644
--- a/watertap/edb/tests/test_edb.py
+++ b/watertap/edb/tests/test_edb.py
@@ -15,15 +15,21 @@
 import json
 import os
 import pytest
-import mongomock
+
+from pyomo.common.dependencies import attempt_import
 
 from watertap.edb import commands
 from watertap.edb.db_api import ElectrolyteDB
 from watertap.edb.validate import validate
 
+mongomock, mongomock_available = attempt_import("mongomock")
+
 
 class MockDB(ElectrolyteDB):
     def __init__(self, db="foo", **kwargs):
+        if not mongomock_available:
+            pytest.skip(reason="mongomock (EDB optional dependency) not available")
+
         self._client = mongomock.MongoClient()
         self._db = getattr(self._client, db)
         # note: don't call superclass!
diff --git a/watertap/edb/tests/test_schemas.py b/watertap/edb/tests/test_schemas.py
index f006b90766..886e4325e7 100644
--- a/watertap/edb/tests/test_schemas.py
+++ b/watertap/edb/tests/test_schemas.py
@@ -13,6 +13,10 @@
 Test schemas module
 """
 import pytest
+
+pytest.importorskip(
+    "watertap.edb.schemas", reason="Missing optional dependencies for EDB schemas"
+)
 from ..schemas import schemas
 from ..data_model import Reaction
 
diff --git a/watertap/edb/tests/util.py b/watertap/edb/tests/util.py
index dd198f9fdb..f161e35bf1 100644
--- a/watertap/edb/tests/util.py
+++ b/watertap/edb/tests/util.py
@@ -13,12 +13,18 @@
 Utility functions for EDB tests
 """
 import pytest
-import mongomock
+from pyomo.common.dependencies import attempt_import
+
 from watertap.edb.db_api import ElectrolyteDB
 
+mongomock, mongomock_available = attempt_import("mongomock")
+
 
 class MockDB(ElectrolyteDB):
     def __init__(self, db="foo", **kwargs):
+        if not mongomock_available:
+            pytest.skip(reason="mongomock (EDB optional dependency) not available")
+
         self._client = mongomock.MongoClient()
         self._db = getattr(self._client, db)
         # note: don't call superclass!
diff --git a/watertap/examples/chemistry/tests/conftest.py b/watertap/examples/chemistry/tests/conftest.py
index 00cbe31a8f..afa45f33a7 100644
--- a/watertap/examples/chemistry/tests/conftest.py
+++ b/watertap/examples/chemistry/tests/conftest.py
@@ -11,16 +11,20 @@
 #################################################################################
 import pytest
 from _pytest.config import Config
+from pyomo.common.dependencies import attempt_import
 
 from watertap.edb import ElectrolyteDB
 from watertap.edb.commands import _load_bootstrap
 
+mongomock, mongomock_available = attempt_import("mongomock")
+
 
 class MockDB(ElectrolyteDB):
     def __init__(self, db="foo", **kwargs):
-        from mongomock import MongoClient
+        if not mongomock_available:
+            pytest.skip(reason="mongomock (EDB optional dependency) not available")
 
-        self._client = MongoClient()
+        self._client = mongomock.MongoClient()
         self._db = getattr(self._client, db)
         # note: don't call superclass!
         self._database_name = db
diff --git a/watertap/examples/chemistry/tests/test_notebooks.py b/watertap/examples/chemistry/tests/test_notebooks.py
deleted file mode 100644
index 2c269beefc..0000000000
--- a/watertap/examples/chemistry/tests/test_notebooks.py
+++ /dev/null
@@ -1,109 +0,0 @@
-#################################################################################
-# WaterTAP Copyright (c) 2020-2023, The Regents of the University of California,
-# through Lawrence Berkeley National Laboratory, Oak Ridge National Laboratory,
-# National Renewable Energy Laboratory, and National Energy Technology
-# Laboratory (subject to receipt of any required approvals from the U.S. Dept.
-# of Energy). All rights reserved.
-#
-# Please see the files COPYRIGHT.md and LICENSE.md for full copyright and license
-# information, respectively. These files are also available online at the URL
-# "https://github.com/watertap-org/watertap/"
-#################################################################################
-"""
-Test Jupyter notebooks in the docs.
-"""
-# stdlib
-import logging
-from pathlib import Path
-import time
-
-# deps
-import nbformat
-from nbconvert.preprocessors import ExecutePreprocessor, CellExecutionError
-import pytest
-
-# package
-from watertap.edb import ElectrolyteDB
-
-# Logging setup
-
-_log = logging.getLogger(__name__)
-
-# Constants
-
-JUPYTER_NB_VERSION = 4
-EXECUTE_TIMEOUT_SEC = 600
-
-# Exceptions
-
-
-class NotebookError(Exception):
-    pass
-
-
-# Utility
-# =======
-
-
-def run_notebook(path: Path):
-    # parse
-    _log.debug(f"begin: parsing '{path}'")
-    with path.open("r", encoding="utf-8") as f:
-        nb = nbformat.read(f, as_version=JUPYTER_NB_VERSION)
-    _log.debug(f"end: parsing '{path}'")
-
-    # execute
-    pp = ExecutePreprocessor(timeout=EXECUTE_TIMEOUT_SEC)
-    _log.debug(f"begin: executing '{path}'")
-    t0 = time.time()
-    try:
-        metadata = {"metadata": {"path": path.parent}}
-        pp.preprocess(nb, metadata)
-    except (CellExecutionError, NameError) as err:
-        _log.warning(f"Failed: {path} :: {err}")
-        raise NotebookError(f"execution error for '{path}': {err}")
-    except TimeoutError as err:
-        _log.warning(f"Timeout: {path} :: {err}")
-        dur, timeout = time.time() - t0, EXECUTE_TIMEOUT_SEC
-        raise NotebookError(f"timeout for '{path}': {dur}s > {timeout}s")
-    dur = time.time() - t0
-    _log.debug(f"end: executing '{path}' duration={dur}s")
-
-
-def find_notebooks(path: Path):
-    return path.glob("**/*.ipynb")
-
-
-@pytest.fixture(scope="session")
-def docs_root(dirname="docs"):
-    p = Path(__file__).parent
-    while not (p / dirname).exists():
-        pp = p.parent
-        if pp == p:
-            raise RuntimeError(f"Could not find '{dirname}' dir")
-        p = pp
-    yield p / dirname
-
-
-def mongodb():
-    try:
-        edb = ElectrolyteDB()  # will fail if no DB
-        edb.get_base()  # will fail if DB is not loaded
-    except Exception as err:
-        _log.warning(f"Could not connect to MongoDB: {err}")
-    return False
-
-
-# Tests
-# =====
-
-
-@pytest.mark.skipif(not mongodb(), reason="MongoDB is required")
-@pytest.mark.component
-def test_edb_notebooks(docs_root):
-    print("\n")
-    for nb_path in find_notebooks(docs_root / "examples" / "edb"):
-        if ".ipynb_checkpoints" in nb_path.parts:
-            continue
-        print(f"run notebook: {nb_path}")
-        run_notebook(nb_path)
diff --git a/watertap/examples/chemistry/tests/test_tutorials.py b/watertap/examples/chemistry/tests/test_tutorials.py
deleted file mode 100644
index 79f969b8a8..0000000000
--- a/watertap/examples/chemistry/tests/test_tutorials.py
+++ /dev/null
@@ -1,55 +0,0 @@
-#################################################################################
-# WaterTAP Copyright (c) 2020-2023, The Regents of the University of California,
-# through Lawrence Berkeley National Laboratory, Oak Ridge National Laboratory,
-# National Renewable Energy Laboratory, and National Energy Technology
-# Laboratory (subject to receipt of any required approvals from the U.S. Dept.
-# of Energy). All rights reserved.
-#
-# Please see the files COPYRIGHT.md and LICENSE.md for full copyright and license
-# information, respectively. These files are also available online at the URL
-# "https://github.com/watertap-org/watertap/"
-#################################################################################
-"""
-This test is for the tutorials in the ../../tutorials/ directory.
-For now, it merely checks that the tutorials execute without raising
-an exception, it does not check for correctness.
-
-- dang 07202022: Made purely advisory for now
-"""
-
-import nbformat
-import unittest
-import glob
-import os.path
-from nbconvert.preprocessors import ExecutePreprocessor
-import traceback
-import warnings
-
-_tutorials_dir = os.path.join(
-    os.path.dirname(os.path.abspath(__file__)), "../../..", "..", "tutorials"
-)
-_notebooks = glob.glob(os.path.join(_tutorials_dir, "**", "*.ipynb"), recursive=True)
-
-
-class TestTutorials(unittest.TestCase):
-    def test_tutorials(self):
-        for notebook_filename in _notebooks:
-            with self.subTest(notebook=os.path.basename(notebook_filename)):
-                with open(notebook_filename) as f:
-                    nb = nbformat.read(f, as_version=4)
-                ep = ExecutePreprocessor()
-                try:
-                    ep.preprocess(
-                        nb,
-                        {"metadata": {"path": f"{os.path.dirname(notebook_filename)}"}},
-                    )
-                except Exception:
-                    warnings.warn(
-                        f"Error executing notebook `{notebook_filename}"
-                        f"\n{traceback.format_exc()}"
-                    )
-                    # continue
-
-
-if __name__ == "__main__":
-    unittest.main()
diff --git a/watertap/examples/edb/tests/conftest.py b/watertap/examples/edb/tests/conftest.py
index 00cbe31a8f..afa45f33a7 100644
--- a/watertap/examples/edb/tests/conftest.py
+++ b/watertap/examples/edb/tests/conftest.py
@@ -11,16 +11,20 @@
 #################################################################################
 import pytest
 from _pytest.config import Config
+from pyomo.common.dependencies import attempt_import
 
 from watertap.edb import ElectrolyteDB
 from watertap.edb.commands import _load_bootstrap
 
+mongomock, mongomock_available = attempt_import("mongomock")
+
 
 class MockDB(ElectrolyteDB):
     def __init__(self, db="foo", **kwargs):
-        from mongomock import MongoClient
+        if not mongomock_available:
+            pytest.skip(reason="mongomock (EDB optional dependency) not available")
 
-        self._client = MongoClient()
+        self._client = mongomock.MongoClient()
         self._db = getattr(self._client, db)
         # note: don't call superclass!
         self._database_name = db
diff --git a/watertap/examples/flowsheets/lsrro/tests/paper_analysis_baselines/__init__.py b/watertap/examples/flowsheets/lsrro/tests/paper_analysis_baselines/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/watertap/examples/flowsheets/lsrro/tests/parameter_sweep_baselines/__init__.py b/watertap/examples/flowsheets/lsrro/tests/parameter_sweep_baselines/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/watertap/tools/parallel/__init__.py b/watertap/tools/parallel/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/watertap/tools/parameter_sweep/parameter_sweep.py b/watertap/tools/parameter_sweep/parameter_sweep.py
index a099b4bb8e..2116993951 100644
--- a/watertap/tools/parameter_sweep/parameter_sweep.py
+++ b/watertap/tools/parameter_sweep/parameter_sweep.py
@@ -13,7 +13,6 @@
 import pyomo.environ as pyo
 import warnings
 import copy
-import requests
 import time
 
 from abc import abstractmethod, ABC
@@ -27,6 +26,9 @@
 from pyomo.common.modeling import unique_component_name
 from pyomo.core.base import _VarData, _ExpressionData
 from pyomo.core.base.param import _ParamData
+from pyomo.common.dependencies import attempt_import
+
+requests, requests_available = attempt_import("requests")
 
 from watertap.tools.parameter_sweep.parameter_sweep_writer import ParameterSweepWriter
 from watertap.tools.parameter_sweep.sampling_types import SamplingType, LinearSample
@@ -191,6 +193,11 @@ def assign_variable_names(model, outputs):
                 outputs[output_name] = exprs[output_name]
 
     def _publish_updates(self, iteration, solve_status, solve_time):
+        if not requests_available:
+            raise ImportError(
+                "requests (parameter_sweep optional dependency) not installed"
+            )
+
         if self.config.publish_progress:
             publish_dict = {
                 "worker_number": self.parallel_manager.get_rank(),
diff --git a/watertap/tools/parameter_sweep/tests/test_parameter_sweep.py b/watertap/tools/parameter_sweep/tests/test_parameter_sweep.py
index 41b371e979..61701d7afa 100644
--- a/watertap/tools/parameter_sweep/tests/test_parameter_sweep.py
+++ b/watertap/tools/parameter_sweep/tests/test_parameter_sweep.py
@@ -13,7 +13,6 @@
 import pytest
 import os
 import numpy as np
-import requests
 import pyomo.environ as pyo
 
 from pyomo.environ import value
@@ -208,6 +207,10 @@ def test_reverse_geom_build_combinations(self):
 
     @pytest.mark.component
     def test_status_publishing(self):
+        requests = pytest.importorskip(
+            "requests",
+            reason="requests (parameter_sweep optional dependency) not available",
+        )
         ps = ParameterSweep(
             publish_progress=True, publish_address="http://localhost:8888"
         )
diff --git a/watertap/unit_models/tests/test_cstr_injection.py b/watertap/unit_models/tests/test_cstr_injection.py
index bf9c1bb017..369febd8dd 100644
--- a/watertap/unit_models/tests/test_cstr_injection.py
+++ b/watertap/unit_models/tests/test_cstr_injection.py
@@ -250,7 +250,6 @@ def test_conservation(self, sapon):
             <= 1e-3
         )
 
-    @pytest.mark.ui
     @pytest.mark.unit
     def test_get_performance_contents(self, sapon):
         perf_dict = sapon.fs.unit._get_performance_contents()
diff --git a/watertap/unit_models/tests/test_electroNP_ZO.py b/watertap/unit_models/tests/test_electroNP_ZO.py
index 12cd71fc21..49d414ae36 100644
--- a/watertap/unit_models/tests/test_electroNP_ZO.py
+++ b/watertap/unit_models/tests/test_electroNP_ZO.py
@@ -16,7 +16,6 @@
     assert_optimal_termination,
     value,
     units,
-    Var,
 )
 from idaes.core import FlowsheetBlock
 from watertap.unit_models.electroNP_ZO import ElectroNPZO
diff --git a/watertap/unit_models/translators/__init__.py b/watertap/unit_models/translators/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/watertap/unit_models/translators/translator_asm1_adm1.py b/watertap/unit_models/translators/translator_asm1_adm1.py
index ee6f5a7f55..71187bb619 100644
--- a/watertap/unit_models/translators/translator_asm1_adm1.py
+++ b/watertap/unit_models/translators/translator_asm1_adm1.py
@@ -40,7 +40,6 @@
 
 from pyomo.environ import (
     Param,
-    PositiveReals,
     NonNegativeReals,
     Var,
     units as pyunits,
diff --git a/watertap/unit_models/translators/translator_asm2d_adm1.py b/watertap/unit_models/translators/translator_asm2d_adm1.py
index e9e07dd306..752fff49cf 100644
--- a/watertap/unit_models/translators/translator_asm2d_adm1.py
+++ b/watertap/unit_models/translators/translator_asm2d_adm1.py
@@ -37,7 +37,6 @@
 import idaes.logger as idaeslog
 
 from pyomo.environ import (
-    PositiveReals,
     Param,
     units as pyunits,
     check_optimal_termination,