Various fixes to tests infrastructure

[simo5]

Fix issues cropping up with newer python tools versions Fixes #245 Fixes #247

[simo5]

@tiran PTAL

[simo5]

@stanislavlevin would you mind reviewing this PR given you proposed #248 and therefore are familiar with the issue ?

[stanislavlevin]

Please, let me check.

[simo5]

@stanislavlevin thanks for the review, sorry for late reply, I was away

[simo5]

-    if skip_servertest and item.get_marker("servertest") is not None:
+    # pytest 4+
+    if hasattr(item, 'get_closest_marker'):
+        get_marker = item.get_closest_marker
+    else:
+        get_marker = item.get_marker
+    if skip_servertest and get_marker("servertest") is not None:

You could use a simpler version of fix with support of pytest < 4 .

Do you have a suggestion ?

[stanislavlevin]

-    if skip_servertest and item.get_marker("servertest") is not None:
+    # pytest 4+
+    if hasattr(item, 'get_closest_marker'):
+        get_marker = item.get_closest_marker
+    else:
+        get_marker = item.get_marker
+    if skip_servertest and get_marker("servertest") is not None:

You could use a simpler version of fix with support of pytest < 4 .

Do you have a suggestion ?

That diff is the suggestion ;-)

[simo5]

Ok rebased with the requested changes, modulo differences in how to handle the get_marker replacement

[stanislavlevin]

-    if skip_servertest and item.get_marker("servertest") is not None:
+    # pytest 4+
+    if hasattr(item, 'get_closest_marker'):
+        get_marker = item.get_closest_marker
+    else:
+        get_marker = item.get_marker
+    if skip_servertest and get_marker("servertest") is not None:

You could use a simpler version of fix with support of pytest < 4 .

Do you have a suggestion ?

Heh, nevermind, I just now realized this was the suggestion ... sometimes the github UI makes it confusing to distinguish actual code quoted from proposed code quoted ...

That said, I am not sure get_closest_marker gives me what I want, I had considered it and decided to check for the specific marker. But what I can do is retain the pytest < 4 code for backwards compat I guess.

Please, take a look at the official docs: https://docs.pytest.org/en/latest/historical-notes.html#update-marker-code http://doc.pytest.org/en/latest/example/markers.html#marking-whole-classes-or-modules

for marker in item.iter_markers():
    if marker.name == "servertest":
        # args has --skip-servertests and test is marked as servertest
        pytest.skip("Skip integration test")

The iteration is extra operation here, because iter_markers iterates over all markers of the item: https://docs.pytest.org/en/latest/reference.html?highlight=get_closest_marker#_pytest.nodes.Node.iter_markers Instead, you could filter marks with name=servertest, but that still returns generator of all the marks applied to the item (marks with the same name could be applied on test function, class or module levels). This can be done like:

if skip_servertest:
    if next(item.iter_markers("servertest"), None) is not None:
        # args has --skip-servertests and test is marked as servertest
        pytest.skip("Skip integration test")

or you could just use the syntax sugar (get_closest_marker).

Internally, get_closest_marker calls iter_markers(name=name) and returns the first mark from generator. https://docs.pytest.org/en/latest/_modules/_pytest/nodes.html#Node.get_closest_marker

Honestly speaking, the custodia test suite has only 1 mark and only one level for this mark, so, it doesn't matter how to handle this one :smile: But...

[stanislavlevin]

Another one point, what is about Python3.7 at Travis? Like this: 0001-py37.patch.txt

[simo5]

Ok let me use get_closer_marker() and if later we have issues we'll fix em later...

As for py37 I think I tried to add it and something else blew up so I backed out :)

[simo5]

Pushed, let's see what happens

[simo5]

@stanislavlevin sounds like all checks pass, can you approve a review? (can't recall if permissions are set up that you can or not, let me know if you can't)

[simo5]

Sigh realized now I had opened this PR from a origin branch ... then kept pushing changes to my personal branch ... oooh well..

[codecov-io]

Codecov Report

Merging #246 into master will increase coverage by 0.41%. The diff coverage is 39.13%.

@@            Coverage Diff             @@
##           master     #246      +/-   ##
==========================================
+ Coverage   73.73%   74.14%   +0.41%     
==========================================
  Files          40       40              
  Lines        4401     4409       +8     
  Branches      447      453       +6     
==========================================
+ Hits         3245     3269      +24     
+ Misses        988      980       -8     
+ Partials      168      160       -8

Impacted Files	Coverage Δ
src/custodia/cli/__init__.py	44.32% <0%> (+1.08%)	:arrow_up:
tests/functional/base.py	86.36% <0%> (ø)	:arrow_up:
src/custodia/store/sqlite.py	80.15% <0%> (ø)	:arrow_up:
src/custodia/ipa/interface.py	80.48% <100%> (ø)	:arrow_up:
tests/conftest.py	66.66% <12.5%> (-21.57%)	:arrow_down:
src/custodia/store/encgen.py	82.81% <50%> (ø)	:arrow_up:
tests/test_custodia.py	95.41% <66.66%> (+0.32%)	:arrow_up:
src/custodia/httpd/server.py	24.92% <0%> (+0.87%)	:arrow_up:
src/custodia/server/config.py	77.88% <0%> (+0.96%)	:arrow_up:
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e3a08ff...8a443ad. Read the comment docs.

[stanislavlevin]

I've checked this PR locally against Python 3.7.3. tox py37 env is OK, unless --skip-servertests.

tox.py3 --sitepackages -e py37 -- --skip-servertests

platform linux -- Python 3.7.3, pytest-5.1.0, py-1.8.0, pluggy-0.12.0
cachedir: .tox/py37/.pytest_cache
rootdir: /usr/src/RPM/BUILD/test/custodia, inifile: tox.ini
collected 162 items                                                                           

tests/test_authenticators.py EE
tests/test_cli.py EEEEE
tests/test_custodia.py EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE
tests/test_httpd.py E
tests/test_ipa.py EEEEEEEEEEEEEEEEEEEEEEEE
tests/test_message_kem.py EEEE
tests/test_misc.py EEE
tests/test_plugins.py EEEE
tests/test_secrets.py EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE
tests/test_server.py EEEE
tests/test_store.py EEE
tests/test_store_sqlite.py EEEEEEEEEEE
tests/functional/test_basics.py EEE
tests/functional/test_container.py EEEEEEEEE
tests/functional/test_key.py EEEEEEEEEEE

=========================================== ERRORS ============================================
_______________________ ERROR at setup of TestAuthenticators.test_cred
...
>           elif item.get_closer_marker("servertest"):
E           AttributeError: 'TestCaseFunction' object has no attribute 'get_closer_marker'

The method should be 'get_closest_marker', not 'get_closer_marker' ;-)

[simo5]

Oh my ... should be fixed now

[stanislavlevin]

Pytest	With Servertests	Python 2.7.16	Python 3.7.3
3.10.1	Y	OK	OK
3.10.1	N	OK	OK
5.1.0	Y	OK	OK
5.1.0	N	OK	OK

[stanislavlevin]

@stanislavlevin sounds like all checks pass, can you approve a review? (can't recall if permissions are set up that you can or not, let me know if you can't)

You could merge directly, aren't you?

[simo5]

Done, thanks a lot for the review!

[stanislavlevin]

Thank you too!