Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure no unexpected warnings are emitted during test suite runs #1363

Open
lbianchi-lbl opened this issue Mar 1, 2024 · 3 comments
Open
Assignees
Labels
Priority:Normal Normal Priority Issue or PR warnings

Comments

@lbianchi-lbl
Copy link
Contributor

  • Removing the -W ignore flag in Restore ability to run pytest on individual files or directories #1362 caused warnings emitted during pytest run to be shown again (see below for a list)
  • Ideally, this list would be empty:
    • Expected warnings should be inspected within the testing code using pytest.warns() (which as I understand is equivalent to pytest.raises() used for exceptions)
    • Unexpected warnings should be addressed in the code being tested

General strategies

SyntaxWarning invalid escape sequence

  • Most often found in pytest.raises(match=expected_str):
  • If expected_str is supposed to be a plain string (i.e. containing no regexes), use re.escape()
  • If expected_str contains both regex constructs and regex symbols that should not be interpreted as regexes, define expected_str as raw literals using r"...", e.g. r"[a-z]+.my_dict\[key\]"

PytestCollectionWarning: cannot collect test class 'TestXYZ' because it has a init constructor

  • If the TestXYZ class is supposed to be a pytest test class (i.e. its methods should be collected as pytest test functions), remove the __init__() definition
  • If the TestXYZ class is not supposed to be a pytest class, change its name so that it doesn't collide with the Test prefix used by pytest, e.g.
    • SampleXYZ
    • XYZForTesting

PytestReturnNotNoneWarning: Expected None, but <test-function> returned <object>, which will be an error in a future version of pytest. Did you mean to use assert instead of return?

  • Test functions in pytest are not supposed to be called directly
  • Basically all the tests in idaes/apps/matopt have this

idaes/apps/grid_integration/tests/util.py:25
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/grid_integration/tests/util.py:25: PytestCollectionWarning: cannot collect test class 'TestingModel' because it has a __init__ constructor (from: idaes/apps/grid_integration/tests/test_bidder.py)
    class TestingModel:
idaes/apps/grid_integration/tests/util.py:267
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/grid_integration/tests/util.py:267: PytestCollectionWarning: cannot collect test class 'TestingForecaster' because it has a __init__ constructor (from: idaes/apps/grid_integration/tests/test_bidder.py)
    class TestingForecaster(AbstractPrescientPriceForecaster):
idaes/apps/grid_integration/tests/test_bidder.py:25
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/grid_integration/tests/test_bidder.py:25: PytestCollectionWarning: cannot collect test class 'TestMissingModel' because it has a __init__ constructor (from: idaes/apps/grid_integration/tests/test_bidder.py)
    class TestMissingModel:
idaes/apps/grid_integration/tests/util.py:25
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/grid_integration/tests/util.py:25: PytestCollectionWarning: cannot collect test class 'TestingModel' because it has a __init__ constructor (from: idaes/apps/grid_integration/tests/test_coordinator.py)
    class TestingModel:
idaes/apps/grid_integration/tests/util.py:267
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/grid_integration/tests/util.py:267: PytestCollectionWarning: cannot collect test class 'TestingForecaster' because it has a __init__ constructor (from: idaes/apps/grid_integration/tests/test_coordinator.py)
    class TestingForecaster(AbstractPrescientPriceForecaster):
idaes/apps/grid_integration/tests/util.py:25
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/grid_integration/tests/util.py:25: PytestCollectionWarning: cannot collect test class 'TestingModel' because it has a __init__ constructor (from: idaes/apps/grid_integration/tests/test_tracker.py)
    class TestingModel:
idaes/apps/grid_integration/tests/test_tracker.py:19
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/grid_integration/tests/test_tracker.py:19: PytestCollectionWarning: cannot collect test class 'TestMissingModel' because it has a __init__ constructor (from: idaes/apps/grid_integration/tests/test_tracker.py)
    class TestMissingModel:
idaes/apps/matopt/opt/pyomo_modeling.py:1465
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/matopt/../matopt/opt/pyomo_modeling.py:1465: SyntaxWarning: "is not" with 'int' literal. Did you mean "!="?
    if PosZic is not 0:
idaes/apps/matopt/opt/pyomo_modeling.py:1469
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/matopt/../matopt/opt/pyomo_modeling.py:1469: SyntaxWarning: "is not" with 'int' literal. Did you mean "!="?
    if NegZic is not 0:
idaes/apps/matopt/opt/pyomo_modeling.py:1587
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/matopt/../matopt/opt/pyomo_modeling.py:1587: SyntaxWarning: "is not" with 'int' literal. Did you mean "!="?
    if PosZic is not 0:
idaes/apps/matopt/opt/pyomo_modeling.py:1591
  /home/runner/work/idaes-pse/idaes-pse/idaes/apps/matopt/../matopt/opt/pyomo_modeling.py:1591: SyntaxWarning: "is not" with 'int' literal. Did you mean "!="?
    if NegZic is not 0:
<frozen importlib._bootstrap>:530: 10 warnings
  <frozen importlib._bootstrap>:530: DeprecationWarning: the load_module() method is deprecated and slated for removal in Python 3.12; use exec_module() instead
idaes/core/util/env_info.py:22
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/util/env_info.py:22: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
    import pkg_resources
../../../../../usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/pkg_resources/__init__.py:2871
../../../../../usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/pkg_resources/__init__.py:2871
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/pkg_resources/__init__.py:2871: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('sphinxcontrib')`.
  Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
    declare_namespace(pkg)
idaes/core/base/tests/test_components.py:88
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_components.py:88: SyntaxWarning: invalid escape sequence '\('
    "support is_solute\(\) method. Use a Solvent or "
idaes/core/base/tests/test_components.py:98
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_components.py:98: SyntaxWarning: invalid escape sequence '\('
    "support is_solvent\(\) method. Use a Solvent or "
idaes/core/base/tests/test_control_volume_1d.py:421
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_control_volume_1d.py:421: SyntaxWarning: invalid escape sequence '\('
    match="fs.cv length_var must be a scalar \(unindexed\) component.",
idaes/core/base/tests/test_control_volume_1d.py:2314
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_control_volume_1d.py:2314: SyntaxWarning: invalid escape sequence '\('
    "list of inherent reactions \(inherent_reaction_idx\), "
idaes/core/base/tests/test_flowsheet_model.py:225
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_flowsheet_model.py:225: SyntaxWarning: invalid escape sequence '\('
    "least two values \(start and end\).",
idaes/core/base/tests/test_property_meta.py:31
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_property_meta.py:31: SyntaxWarning: invalid escape sequence '\('
    match="Unrecognized units of measurement for quantity TIME \(foo\)",
idaes/core/base/tests/test_property_meta.py:41
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_property_meta.py:41: SyntaxWarning: invalid escape sequence '\('
    match="Invalid units of measurement for quantity LENGTH \(s\). "
idaes/core/base/tests/test_property_meta.py:52
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_property_meta.py:52: SyntaxWarning: invalid escape sequence '\('
    match="Invalid units of measurement for quantity MASS \(m\). "
idaes/core/base/tests/test_property_meta.py:63
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_property_meta.py:63: SyntaxWarning: invalid escape sequence '\('
    match="Invalid units of measurement for quantity AMOUNT \(m\). "
idaes/core/base/tests/test_property_meta.py:74
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_property_meta.py:74: SyntaxWarning: invalid escape sequence '\('
    match="Invalid units of measurement for quantity TEMPERATURE \(m\). "
idaes/core/base/tests/test_property_meta.py:85
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_property_meta.py:85: SyntaxWarning: invalid escape sequence '\('
    match="Invalid units of measurement for quantity CURRENT \(m\). "
idaes/core/base/tests/test_property_meta.py:96
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_property_meta.py:96: SyntaxWarning: invalid escape sequence '\('
    match="Invalid units of measurement for quantity LUMINOUS_INTENSITY \(m\). "
idaes/core/base/tests/test_property_meta.py:107
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_property_meta.py:107: SyntaxWarning: invalid escape sequence '\('
    match="Invalid units of measurement for quantity TIME \(m\). "
idaes/core/base/tests/test_unit_model.py:201
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_unit_model.py:201: SyntaxWarning: invalid escape sequence '\('
    "instance of a StateBlock object \(does not have a build_port method\).",
idaes/core/base/tests/test_unit_model.py:322
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_unit_model.py:322: SyntaxWarning: invalid escape sequence '\('
    " but no default ControlVolume exists \(control_volume\). "
idaes/core/base/tests/test_unit_model.py:530
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_unit_model.py:530: SyntaxWarning: invalid escape sequence '\('
    "\(control_volume\). Please provide block to which the "
idaes/core/base/tests/test_var_like_expression.py:40
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_var_like_expression.py:40: SyntaxWarning: invalid escape sequence '\('
    "attribute. Use the 'value\(\)' method instead.",
idaes/core/base/tests/test_var_like_expression.py:118
  /home/runner/work/idaes-pse/idaes-pse/idaes/core/base/tests/test_var_like_expression.py:118: SyntaxWarning: invalid escape sequence '\['
idaes/models/control/tests/test_antiwindup.py::test_setpoint_change_windup
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but idaes/models/control/tests/test_antiwindup.py::test_setpoint_change_windup returned (<pyomo.core.base.PyomoModel.ConcreteModel object at 0x7f479e799270>, <pyomo.solvers.plugins.solvers.IPOPT.IPOPT object at 0x7f479dbd36e0>, <idaes.core.solvers.petsc.PetscTrajectory object at 0x7f47a0652660>), which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
    warnings.warn(
idaes/models/control/tests/test_antiwindup.py::test_setpoint_change_conditional_integration
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but idaes/models/control/tests/test_antiwindup.py::test_setpoint_change_conditional_integration returned (<pyomo.core.base.PyomoModel.ConcreteModel object at 0x7f479dfb8910>, <pyomo.solvers.plugins.solvers.IPOPT.IPOPT object at 0x7f479f16d880>, <idaes.core.solvers.petsc.PetscTrajectory object at 0x7f479e2ff410>), which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
    warnings.warn(
idaes/models/control/tests/test_antiwindup.py::test_setpoint_change_back_calculation
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but idaes/models/control/tests/test_antiwindup.py::test_setpoint_change_back_calculation returned (<pyomo.core.base.PyomoModel.ConcreteModel object at 0x7f479f5e14f0>, <pyomo.solvers.plugins.solvers.IPOPT.IPOPT object at 0x7f47a0ad28d0>, <idaes.core.solvers.petsc.PetscTrajectory object at 0x7f479cb69a60>), which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
    warnings.warn(
idaes/models_extra/power_generation/costing/tests/test_CCS_capcost.py::test_ccs_units_costing
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but idaes/models_extra/power_generation/costing/tests/test_CCS_capcost.py::test_ccs_units_costing returned <pyomo.core.base.PyomoModel.ConcreteModel object at 0x7f479b857250>, which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
    warnings.warn(
idaes/models_extra/power_generation/costing/tests/test_power_plant_capcost.py::test_PP_costing
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but idaes/models_extra/power_generation/costing/tests/test_power_plant_capcost.py::test_PP_costing returned <pyomo.core.base.PyomoModel.ConcreteModel object at 0x7f479ba7f7a0>, which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
    warnings.warn(
idaes/models_extra/power_generation/costing/tests/test_power_plant_capcost.py::test_power_plant_costing
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but idaes/models_extra/power_generation/costing/tests/test_power_plant_capcost.py::test_power_plant_costing returned <pyomo.core.base.PyomoModel.ConcreteModel object at 0x7f47a45eb1b0>, which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
    warnings.warn(
idaes/models_extra/power_generation/costing/tests/test_power_plant_capcost.py::test_sCO2_costing
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but idaes/models_extra/power_generation/costing/tests/test_power_plant_capcost.py::test_sCO2_costing returned <pyomo.core.base.PyomoModel.ConcreteModel object at 0x7f479ebda710>, which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
    warnings.warn(
idaes/models_extra/power_generation/costing/tests/test_power_plant_capcost.py::test_ASU_costing
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but idaes/models_extra/power_generation/costing/tests/test_power_plant_capcost.py::test_ASU_costing returned <pyomo.core.base.PyomoModel.ConcreteModel object at 0x7f479ed1bc50>, which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
    warnings.warn(
idaes/models_extra/power_generation/unit_models/helm/tests/test_turbine_multistage.py::test_initialize
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but idaes/models_extra/power_generation/unit_models/helm/tests/test_turbine_multistage.py::test_initialize returned <pyomo.core.base.PyomoModel.ConcreteModel object at 0x7f47834d1fe0>, which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
    warnings.warn(
idaes/models_extra/power_generation/unit_models/helm/tests/test_turbine_multistage.py::test_initialize_calc_cf
  /usr/share/miniconda/envs/idaes-pse-dev/lib/python3.12/site-packages/_pytest/python.py:198: PytestReturnNotNoneWarning: Expected None, but idaes/models_extra/power_generation/unit_models/helm/tests/test_turbine_multistage.py::test_initialize_calc_cf returned <pyomo.core.base.PyomoModel.ConcreteModel object at 0x7f479dc1f750>, which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?
    warnings.warn(
idaes/tests/prescient/test_prescient.py::Test5Bus::test_data_path_available
idaes/tests/prescient/test_prescient.py::Test5Bus::test_simulation_results
  /home/runner/work/idaes-pse/idaes-pse/idaes/tests/prescient/test_prescient.py:36: DeprecationWarning: path is deprecated. Use files() instead. Refer to https://importlib-resources.readthedocs.io/en/latest/using.html#migrating-from-legacy for migration advice.
    with resources.path("idaes.tests.prescient.5bus", "__init__.py") as pkg_file:
@lbianchi-lbl lbianchi-lbl added the Priority:Normal Normal Priority Issue or PR label Mar 1, 2024
@lbianchi-lbl
Copy link
Contributor Author

Example from

with pytest.raises(
TypeError,
match="e is an Expression and does not have a value "
"attribute. Use the 'value\(\)' method instead.",
):
assert m.e.value == 42

import re
...
    with pytest.raises(
        TypeError,
        match=re.escape(
            "e is an Expression and does not have a value "
            "attribute. Use the 'value()' method instead."
        ),
    ):
        assert m.e.value == 42

@ksbeattie
Copy link
Member

It appears the way to address these warnings is to deprecate and/or remove older (unmaintained) parts of the code base.

@lbianchi-lbl
Copy link
Contributor Author

  • Most of the remaining warnings come from the way MatOpt tests are implemented
  • Since there's no way to address the warnings without rewriting the entire MatOpt test suite from scratch, our plan is to resolve the warnings by moving the MatOpt code to a separate repository (being tracked in Move MatOpt code to own repository #1508)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:Normal Normal Priority Issue or PR warnings
Projects
None yet
Development

No branches or pull requests

5 participants