ec-jrc / pyPoseidon

Framework for Hydrodynamic simulations
https://pyposeidon.readthedocs.io/
European Union Public License 1.2
20 stars 7 forks source link

Fix pytest warning #144

Open brey opened 1 year ago

brey commented 1 year ago
conftest.py:13
  /Users/brey/Github/pyPoseidon-dev/tests/conftest.py:13: PytestDeprecationWarning: A private pytest class or function was used.
    EMPTY_MARK = Mark("", [], {})

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

This will become an error in pytest=8.

pmav99 commented 1 year ago

Yeah, I've already fixed this locally, but I was waiting for #126 to get merged before I push it. This is the diff if you want to apply it:

diff --git a/tests/conftest.py b/tests/conftest.py
index 671f8c1..88c0847 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -14,6 +14,21 @@ EMPTY_MARK = Mark("", [], {})
 RUNLAST_MARK = "runlast"

+def should_run_last(item) -> bool:
+    closest_marker = item.get_closest_marker(RUNLAST_MARK, default=EMPTY_MARK)
+    return closest_marker is RUNLAST_MARK
+
+
+def move_tests_to_the_end(items):
+    start, end = [], []
+    for item in items:
+        if should_run_last(item):
+            end.append(item)
+        else:
+            start.append(item)
+    return start + end
+
+
 def pytest_addoption(parser):
     parser.addoption("--runschism", action="store_true", default=False, help="run schism tests")
     parser.addoption("--rundelft", action="store_true", default=False, help="run delft tests")
@@ -45,8 +60,7 @@ def pytest_collection_modifyitems(config, items):
         if "viz" in item.keywords and not should_run_viz:
             item.add_marker(skip_viz)

-    run_last = lambda item: item.get_closest_marker(RUNLAST_MARK, default=EMPTY_MARK)
-    items.sort(key=run_last, reverse=False)
+    items[:] = move_tests_to_the_end(items)

Not sure if this is the proper way to fix it, because we are making use pytest internals, but it should be good enough for now.

brey commented 1 year ago

Same warning after the fix. The internals is the issue. As long as we use _pytest.mark we'll have a problem.

See https://docs.pytest.org/en/7.1.x/deprecations.html?highlight=mark#directly-constructing-internal-classes.

pmav99 commented 1 year ago

Oh you are talking just about the warning. The code we have in conftest.py is throwing an exception in pytest 7.3:

...
INTERNALERROR>   File "/home/panos/Prog/poseidon/pyPoseidon/tests/conftest.py", line 49, in pytest_collection_modifyitems
INTERNALERROR>     items.sort(key=run_last, reverse=False)
INTERNALERROR> TypeError: '<' not supported between instances of 'Mark' and 'Mark'

That's what the patch is trying to resolve. I guess you haven't updated pytest that's why you don't see it.

For the warning, we need to add a line in pyproject.toml:

diff --git a/pyproject.toml b/pyproject.toml                                                                                                                                                                                            
index a10bcd2..54d235a 100644                                                                                                                                                                                                           
--- a/pyproject.toml                                                                                                                                                                                                                    
+++ b/pyproject.toml                                                                                                                                                                                                                    
@@ -136,6 +136,7 @@ filterwarnings = [                                                                                                                                                                                                  
     'ignore:`np.*` is a deprecated alias for .* `np.*`.*:DeprecationWarning',
     'ignore:distutils Version classes are deprecated. Use packaging.version instead:DeprecationWarning',
     "ignore:Using or importing the ABCs from 'collections' instead of from 'collections.abc':DeprecationWarning",
+    "ignore:A private pytest class or function was used.:pytest.PytestDeprecationWarning"                                                                                                                                              
 ]
 markers = [
     "schism: mark a test as a Schism based test. Needs --runschism to run",
brey commented 1 year ago

which pytest do you have?

brey commented 1 year ago

btw,

I am getting also

tests/test_tools.py::test_open_dataset[netcdf - pathlib]
  <frozen importlib._bootstrap>:241: RuntimeWarning: numpy.ndarray size changed, may indicate binary incompatibility. Expected 16 from C header, got 96 from PyObject

Any ideas? I have updated numpy to the latest.

pmav99 commented 1 year ago

pytest 7.3.1 requires the patch I posted earlier. pytest 7.2 works ok with what we have.

I also get the numpy warning. Not sure about the proper way to resolve it, though. I will check.