This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Enum.py : Enum.__new__() : Test Coverage
Type: enhancement Stage: resolved
Components: Extension Modules, Tests Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ethan.furman Nosy List: CliffM, ethan.furman, python-dev
Priority: normal Keywords: patch

Created on 2013-10-13 20:51 by CliffM, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
enum.patch CliffM, 2013-10-13 20:51 review
Messages (7)
msg199800 - (view) Author: CliffM (CliffM) Date: 2013-10-13 20:51
test_nonhash_value tests the lookup of an Enum Member from a value.

The current test does not exercise the iteration over values completely -- the if-clause being redundant wrt the existing test.

My patch adds an additional test which requires the if-clause.
msg199827 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-10-14 00:07
I'm not sure what you are talking about.  Here's the code:

    try:
        if value in cls._value2member_map_:
            return cls._value2member_map_[value]
    except TypeError:
        # not there, now do long search -- O(n) behavior
        for member in cls._member_map_.values():
            if member.value == value:
                return member

Here's the test:

    self.assertEqual(ColorInAList([1]), ColorInAList.red)

In order for that test to work, the first if (in the try) raises a TypeError, then execution falls into the except block and each member is tested for equality until one matches (via the if) -- so what is your added test doing that isn't already being done?
msg199853 - (view) Author: CliffM (CliffM) Date: 2013-10-14 08:53
Sorry -- I could have been clearer :

The conditional:

 if member.value == value:

Is redundant as the tests stand.  If you comment it out -- everything works. So therefore we are missing a test.

The current test works, as red is the first value to pop out of the value() list.  

This makes the if-clause fragile for future maintenance.  So we need another test to ensure the loop is covered.

It's a coverage issue -- where although the code is executed by the test, and the code is correct, the test is not complete enough for the code.
msg199889 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-10-14 14:08
CliffM added the comment:
>
> Sorry -- I could have been clearer :
>
> The conditional:
>
>   if member.value == value:
>
> Is redundant as the tests stand.  If you comment it out -- everything works. So therefore we are missing a test.

Are you saying that you are commenting out the if test, but leaving in the return member?

> This makes the if-clause fragile for future maintenance.  So we need another test to ensure the loop is covered.

In case Python for loops suddenly stop working?

Sorry to be so dense, but I am not understanding the point you are trying to make... ahhhhhh!  Are trying to guard 
against the possibility that in the future someone might accidentally delete the if test, and the unit test won't catch 
it?  That certainly is a good reason to test values further into the loop.
msg199949 - (view) Author: CliffM (CliffM) Date: 2013-10-14 20:44
Yes it's purely a coverage issue.  I must try to be more explicit (rather than implicit).
msg199952 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-10-14 21:06
Well, I would say it's half a coverage issue, half a guard against accidental deletion of the if test.  :)
msg200102 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-10-17 02:09
New changeset 89f6abc2e115 by Ethan Furman in branch 'default':
Close #19252: better test coverage for Enum.  Thanks, CliffM
http://hg.python.org/cpython/rev/89f6abc2e115
History
Date User Action Args
2022-04-11 14:57:51adminsetgithub: 63451
2013-10-17 02:09:57python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg200102

resolution: fixed
stage: resolved
2013-10-14 21:06:19ethan.furmansetmessages: + msg199952
2013-10-14 20:44:17CliffMsetmessages: + msg199949
2013-10-14 14:08:36ethan.furmansetmessages: + msg199889
2013-10-14 08:53:27CliffMsetmessages: + msg199853
2013-10-14 00:07:11ethan.furmansetassignee: ethan.furman

messages: + msg199827
nosy: + ethan.furman
2013-10-13 20:51:42CliffMcreate