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

Code block reported missing when provably covered #167

Open
al-niessner opened this issue Aug 1, 2017 · 19 comments
Open

Code block reported missing when provably covered #167

al-niessner opened this issue Aug 1, 2017 · 19 comments
Labels

Comments

@al-niessner
Copy link

It appears as though code covered with pytest is being reported as missing. While it may be a bug in pytest-cov, it may also be a problem with configuration. The configuration is quite simple and is all on the command line -- configuration files are avoided.

Here are the platform details:

$ uname -a
Linux 4.10.0-27-generic #30~16.04.2-Ubuntu SMP Thu Jun 29 16:07:46 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
$ pytest --version
This is pytest version 3.1.3, imported from /usr/local/lib/python3.5/dist-packages/pytest.py
setuptools registered plugins:
  pytest-cov-2.5.1 at /usr/local/lib/python3.5/dist-packages/pytest_cov/plugin.py
$ python3 -m pytest --cov=pix --cov=pixg --cov=pge --cov-branch --cov-report term-missing -v test

If I then run the unit tests, it completes without error and the line that concerns me is:

----------- coverage: platform linux, python 3.5.2-final-0 -----------
Name                         Stmts   Miss Branch BrPart  Cover   Missing
------------------------------------------------------------------------
pix/complete.py                 91      5     55      9    89%   40-47, 71, 32->37, 49->exit, 61->78, 64->76, 66->73, 68->71, 95->106, 99->103, 136->143

The block of code 136-143 is:

                if not all (same.values()):
                    _error ('Reservoir "' + no.name + '" does not match ' +
                            'node "' + f.node + '".\n   ' +
                            '\n   '.join (['{} : {}'.format (k, same[k])
                                           for k in sorted (same)]))
                    result = pix.RuleStatus.fail
                    pass

This block is covered with same.values() all being true and with some false elements. This can be shown by forcing pytest with assert to fail and looking at the output:

starting rule 03
completed rule 03
rule_03 completed with the status: good
starting rule 03
Reservoir "/fred" does not match node "/sally".
   type : False
completed rule 03
rule_03 completed with the status: fail
starting rule 03
Reservoir "/fred" does not match node "/sally".
   type : False
completed rule 03
rule_03 completed with the status: fail
starting rule 03
Reservoir "/fred" does not match node "/sally".
   length : False
   shape : True
   type : True
completed rule 03
rule_03 completed with the status: fail
starting rule 03
Reservoir "/fred" does not match node "/sally".
   length : True
   shape : False
   type : True
completed rule 03
rule_03 completed with the status: fail
starting rule 03
completed rule 03
rule_03 completed with the status: good

While the messages is bit to consume all at once, the important bit is to notice that the block being reported as missing is generating the fail message. I included the good messages to show that they same block is called when all (same.values()) is both True and False.

Hence, I think this shows that the block is being reported as missing when it is provably being executed. I cannot post all of the code for someone in this group to test, but with some help I am willing to work through the pytest-cov code to help find and isolate the problem (looking for hints as how and where to start). With the best of luck someone will recognize that I did not type my command line correctly.

@ionelmc
Copy link
Member

ionelmc commented Aug 1, 2017

Are subprocesses or multiprocess involved? I'm not sure what to make of this.

Try disabling branch coverage.

@al-niessner
Copy link
Author

No subprocesses or multiprocess. All unit tests are single threaded unless pytest is multithreaded.

Turned off branch coverage (removed --cov-branch) and it seems to be working correctly now.

@al-niessner
Copy link
Author

I would like to have branch coverage active, so if you want to point me at the pytest-cov code to start at, then can work through it to see if an error pops out. If one does, then a simple test can be generated to demonstrate then fix it.

@ionelmc
Copy link
Member

ionelmc commented Aug 1, 2017

You're probably not running all the branches (you got no test where rules are all True). See https://coverage.readthedocs.io/en/coverage-4.4.1/branch.html

I don't see how this is a problem with coverage or pytest-cov (a mere wrapper around coverage) ...

@al-niessner
Copy link
Author

As shown in the output text, when all (same) is True, the function _error() is not called. When it is not true, then it is called and you get a message about reservoirs and nodes with a list of the checks done for sameness and whether or not the individual item was true or false. Since this is the only block that generates this message (you do not have all 10000 lines of code so there is some trust here) then we can say from the output that both parts of the branch are exercised. Therefore, logic dictates that pytest-cov declaring it as missing is incorrect.

I tried calling it an odd number of times as well as even. I changed the order to make the positive test first then the negative test first. Tried a bunch of silly things to diagnose further but they all resulted in the same missing block.

@al-niessner
Copy link
Author

In terms of your docs

It seems that pytest-cov might be interpreting the if as not a branch. The paragraph on exclusion as an if-else construct and says that because the else is excluded that the if does not look like a branch because of how counting is done.

How can that be true? First, if with out an else is still a branch as doing nothing over something is still different behavior. Second, there are still two possible outcomes in terms of counting. The statement when True and the statement when False. Even if the else is no covered the line after the else block should be and so it would look like the next line. Hence the if is still a branch.

@al-niessner
Copy link
Author

Quick test shows that your documentation is correct. This code block produces same result:

                if not all (same.values()):
                    _error ('Reservoir "' + no.name + '" does not match ' +
                            'node "' + f.node + '".\n   ' +
                            '\n   '.join (['{} : {}'.format (k, same[k])
                                           for k in sorted (same)]))
                    result = pix.RuleStatus.fail
                else: pass

This code block produces the expected coverage (136-143 are not missing):

                if not all (same.values()):
                    _error ('Reservoir "' + no.name + '" does not match ' +
                            'node "' + f.node + '".\n   ' +
                            '\n   '.join (['{} : {}'.format (k, same[k])
                                           for k in sorted (same)]))
                    result = pix.RuleStatus.fail
                else: print ('nothing')

All three cases should produce the same results as they are exactly the same behavior (minus some output to the terminal) with respect to branching. At least there is a tremendously simple test case that shows the problem.

@ionelmc
Copy link
Member

ionelmc commented Aug 1, 2017

In terms of your docs

Coverage is a different project, those aren't my docs. I don't maintain them and I don't know the internals or particulars of branch coverage. Perhaps you should take this to it's bug tracker: https://bitbucket.org/ned/coveragepy/issues?status=new&status=open

@al-niessner
Copy link
Author

Sorry, but I am getting confused...

  1. I am running pytest --cov
  2. I said your docs referring to your reference to the document. I did not mean to say or imply that you were fully responsible for it.
  3. I am saying that pytest --cov is generating spurious results and think that I have shown it. I can produce a small sample that I can post.

With those bullets, are you saying that this is the wrong forum and that I should take this to specified bitbucket? If this is the wrong forum, then, for my education, what is the relationship between pytest-cov and coverage.py?

@al-niessner
Copy link
Author

Hmmm, it (branch detection) seems to have problems with recursion as well...

I have a recursive function that where I changed the if to be more like what the document wants and it is failing. I am tempted to replace all of my if blocks with dictionaries of functions...

@ionelmc
Copy link
Member

ionelmc commented Aug 1, 2017

pytest-cov uses coverage.py and only does pytest command line integration, and supports xdist/subprocesses/multiprocessing out of the box (with bare coverage.py it's very hard to get those right).

It's not that it's the wrong forum, feel free to ping Ned right here, I don't mind. It's just that your bug report assumes that your tests can't be wrong and that I should know what's wrong without seeing a reproducer. See http://sscce.org/

@al-niessner
Copy link
Author

Fair enough. Let me do a quick small test where I can post the whole unit and then ping Ned.

@al-niessner
Copy link
Author

al-niessner commented Aug 2, 2017

Here is the test. It is not at all what I thought! Someone should let Ned know to look at it (I did not find him in the watchers list or let me know and I will try submitting to bitbucket):

I used the same if block for testing and then wrapped it with various stuff that made it more and more like what I really have. Those functions are foo -> foobar inclusive. I then made two more edition, sna and snafu that extend fooba and foobar respectively to show that adding and else changes the branch testing.

Here is my command line:
python3 -m pytest --cov=cov_test --cov-branch --cov-report term-missing PyScripts/cov_test.py

Here is my result:

============================= test session starts ==============================
platform linux -- Python 3.5.2, pytest-3.1.3, py-1.4.34, pluggy-0.4.0
rootdir: /home/niessner, inifile:
plugins: cov-2.5.1
collected 1 item s

PyScripts/cov_test.py .

----------- coverage: platform linux, python 3.5.2-final-0 -----------
Name                    Stmts   Miss Branch BrPart  Cover   Missing
-------------------------------------------------------------------
PyScripts/cov_test.py      48      0     20      2    97%   18->21, 27->30


=========================== 1 passed in 0.03 seconds ===========================

Here is the code being tested:

def foo(states:[bool]):
    if all(states):
        print ('foo')
        pass
    return True

def foob (states:[bool]):
    if True:
        if all(states):
            print ('foob')
            pass
        pass
    return True

def fooba (states:[bool]):
    for i in range(10):
        if all(states):
            print ('fooba')
            pass
        pass
    return True

def foobar (states:[bool]):
    if True:
        for i in range(10):
            if all(states):
                print ('fooba')
                pass
            pass
        pass
    return True

def sna (states:[bool]):
    for i in range(10):
        if all(states):
            print ('sna')
        else: print ('ans')
        pass
    return True

def snafu (states:[bool]):
    if True:
        for i in range(10):
            if all(states):
                print ('snafu')
            else: print ('ufans')
            pass
        pass
    return True

def test():
    assert foo([True,False])
    assert foo([True,True])
    assert foob([True,False])
    assert foob([True,True])
    assert fooba([True,False])
    assert fooba([True,True])
    assert foobar([True,False])
    assert foobar([True,True])
    assert sna([True,False])
    assert sna([True,True])
    assert snafu([True,False])
    assert snafu([True,True])
    return

Sorry if it is more than you wanted, but I was trying to be complete.

@ionelmc
Copy link
Member

ionelmc commented Aug 2, 2017

In hindsight now I think it's best to log a bug report in coverage's bugtracker (maybe bug is already there).

@al-niessner
Copy link
Author

I already looked through the open tickets and did not see it. I will log the ticket there. Just a shame to go through the whole explanation again.

Thanks for your help. It has been a pleasure.

@al-niessner
Copy link
Author

reported

@nedbat
Copy link
Collaborator

nedbat commented Aug 2, 2017

This is a limitation of Python's: it optimizes away jumps to jumps, and you cannot disable the optimization.

@al-niessner
Copy link
Author

Since converage.py already supports pragma for code exclusion, would it be possible to add pragmas for inclusion? For instance, 'pragma cov-if' or 'pragma cov-for' etc to circumvent the optimization limitations.

@webknjaz
Copy link
Member

@ionelmc This should probably be closed as it's an upstream issue nedbat/coveragepy#594 (along with the duplicate nedbat/coveragepy#198) are closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants