smarie / python-pytest-harvest

Store data created during your `pytest` tests execution, and retrieve it at the end of the session, e.g. for applicative benchmarking purposes.
https://smarie.github.io/python-pytest-harvest/
BSD 3-Clause "New" or "Revised" License
61 stars 8 forks source link

BUG: Fix use with doctest + fixtures #41

Closed larsoner closed 3 years ago

larsoner commented 3 years ago

Fixes #42

On master using --doctest-modules with fixtures leads to:

pytest_harvest/results_session.py:167: in get_session_synthesis_dct
    param_dct = get_pytest_params(item)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

item = <DoctestItem pytest_harvest.plugin.dummy>

    def get_pytest_params(item):
        """ Returns a dictionary containing a pytest session item's parameters """

        if isinstance(item, _MinimalItem):
            # Our special _MinimalItem object - when xdist is used and worker states have been saved + restored
            return item.get_pytest_params()
        else:
            param_dct = OrderedDict()
>           for param_name in item.fixturenames:  # note: item.funcargnames gives the exact same list
E           AttributeError: 'DoctestItem' object has no attribute 'fixturenames'

and also

pytest_harvest/results_session.py:234: in <genexpr>
    filtered_items = tuple(item for item in session.items if _pytest_item_matches_filter(item, filterset))
pytest_harvest/results_session.py:327: in _pytest_item_matches_filter
    elif any(item_obj.__module__ == f for f in filterset):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

.0 = <set_iterator object at 0x7ffe00770140>

>   elif any(item_obj.__module__ == f for f in filterset):
E   AttributeError: 'NoneType' object has no attribute '__module__'
  1. Added test that fails on master but passes here, along with dummy doctestable function
  2. Adjusted META test count
  3. Fixed code by adding a conditional that avoids trying to get param names from DoctestItem
  4. Fixed code by adding a conditional for the item_obj is None case
  5. Added pytest_harvest/_version.py to .gitignore (I assume this is useful?)
  6. Removed some trailing whitespace (more PEP8-y; my editor did it automatically; can revert if preferred)

FWIW locally not all tests pass actually, but the same set that fail on this PR also fail on master.

codecov-commenter commented 3 years ago

Codecov Report

Merging #41 into master will decrease coverage by 0.18%. The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
- Coverage   88.63%   88.45%   -0.19%     
==========================================
  Files          31       31              
  Lines        1065     1074       +9     
==========================================
+ Hits          944      950       +6     
- Misses        121      124       +3     
Impacted Files Coverage Δ
pytest_harvest/plugin.py 62.12% <50.00%> (-0.19%) :arrow_down:
pytest_harvest/results_session.py 82.32% <60.00%> (-0.58%) :arrow_down:
...test_harvest/tests_raw/test_get_session_results.py 96.21% <100.00%> (+0.05%) :arrow_up:

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 00a713c...0a90b0a. Read the comment docs.

smarie commented 3 years ago

Thanks @larsoner ! I'll have a look tomorrow

smarie commented 3 years ago

Thanks @larsoner ! Looks very good to me, I made a few comments that are mostly nitpicks really, just for the sake of making maintenance a bit easier on the long run.

All tests pass ok in Travis so I'll be able to merge when you'll have made the updates. (the only failure that I see is in the "raw" tests, and is intended - it is then collected by the META test runner and expected to fail)

larsoner commented 3 years ago

@smarie comments addressed

larsoner commented 3 years ago

There is a wall of red here :)

https://travis-ci.org/github/smarie/python-pytest-harvest/builds/727704884

Let me know if you want my help fixing it

smarie commented 3 years ago

Hi @larsoner I merged manually and fixed the meta-tester, everything is fine now and 1.9.3 is available.

There were 2 issues

Let me know if the released version is ok for you. And thanks again !!

larsoner commented 3 years ago

@smarie yes, just pip install pytest-harvest works now!

smarie commented 3 years ago

great, thanks for the feedback and PR @larsoner !