misused assertTrue in unit tests

Bug #1226374 reported by ChangBo Guo(gcb) on 2013-09-17
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Glance
Undecided
ChangBo Guo(gcb)
Ironic
Fix Released
Medium
ChangBo Guo(gcb)
OpenStack Heat
Fix Released
Medium
ChangBo Guo(gcb)
OpenStack Identity (keystone)
Low
ChangBo Guo(gcb)
OpenStack Object Storage (swift)
Undecided
ZhiQiang Fan
oslo-incubator
Medium
ChangBo Guo(gcb)
python-keystoneclient
Low
ChangBo Guo(gcb)
tempest
Undecided
Unassigned

Bug Description

1)signature for assertTure:
    def failUnless(self, expr, msg=None):
        """Fail the test unless the expression is true."""
        if not expr: raise self.failureException, msg
     ....................................
assert_ = assertTrue = failUnless

2) signature for assertEqual:
    def failUnlessEqual(self, first, second, msg=None):
        """Fail if the two objects are unequal as determined by the '=='
           operator.
        """
        if not first == second:
            raise self.failureException, \
                  (msg or '%r != %r' % (first, second))
          ...........................
      assertEqual = assertEquals = failUnlessEqual

assertTrue used to evaluate one expression
assertEqual used to compare two expressions

If passed two expressions to assertTrue , it always evaluate first expression . That 's not expected .

Changed in tempest:
assignee: nobody → ChangBo Guo (guochbo)
status: New → In Progress
Changed in keystone:
assignee: nobody → ChangBo Guo (guochbo)
status: New → In Progress

Fix proposed to branch: master
Review: https://review.openstack.org/46868

Changed in glance:
assignee: nobody → ChangBo Guo (guochbo)
status: New → In Progress

Fix proposed to branch: master
Review: https://review.openstack.org/46906

Changed in heat:
assignee: nobody → ChangBo Guo (guochbo)
status: New → In Progress
Steven Hardy (shardy) on 2013-09-17
Changed in heat:
milestone: none → havana-rc1
importance: Undecided → Medium

Reviewed: https://review.openstack.org/46906
Committed: http://github.com/openstack/heat/commit/c8b142182f38fa0d1752a9f02361e30413d9ccf2
Submitter: Jenkins
Branch: master

commit c8b142182f38fa0d1752a9f02361e30413d9ccf2
Author: Chang Bo Guo <email address hidden>
Date: Tue Sep 17 02:02:50 2013 -0700

    Fix misused assertTrue in unit tests

    Refactored unit tests to use assertEqual instead of assertTrue
    where needed.

    Fixes bug #1226374

    Change-Id: I6826fefbb7c6fed8b0e664da36b144e882fb6d0e

Changed in heat:
status: In Progress → Fix Committed
Dolph Mathews (dolph) wrote :

Found at least one instance of this in python-keystoneclient as well

Changed in python-keystoneclient:
status: New → Triaged
milestone: none → 0.3.3
Changed in keystone:
milestone: none → havana-rc1
Dolph Mathews (dolph) wrote :

There may be more, but here's the few examples I came across:

keystone/tests/test_middleware.py:152: self.assertTrue(req.content_type, 'application/json')
keystone/tests/test_v3_oauth1.py:150: self.assertTrue(resp.result.get('consumer').get('id'), consumer_id)
keystone/tests/test_v3_oauth1.py:272: self.assertTrue(entity['id'], self.access_token.key)
keystone/tests/test_v3_oauth1.py:273: self.assertTrue(entity['consumer_id'], self.consumer.key)
keystone/tests/test_v3_oauth1.py:297: self.assertTrue(entity['id'], self.role_id)

tests/v2_0/test_ec2.py:148: self.assertTrue(len(creds), 2)

Changed in python-keystoneclient:
assignee: nobody → ChangBo Guo (guochbo)
ChangBo Guo(gcb) (glongwave) wrote :

Dolph ,

This is patch for keystone https://review.openstack.org/#/c/46855/

  I will check again . and submit a patch for python-keystoneclient include "tests/v2_0/test_ec2.py:148: self.assertTrue(len(creds), 2)" .

thanks.

Fix proposed to branch: master
Review: https://review.openstack.org/46975

Changed in python-keystoneclient:
status: Triaged → In Progress
Dolph Mathews (dolph) on 2013-09-17
Changed in python-keystoneclient:
importance: Undecided → Low
Changed in keystone:
importance: Undecided → Low

Reviewed: https://review.openstack.org/46855
Committed: http://github.com/openstack/keystone/commit/e72c273d46a8c010baea70211800153b90c458ab
Submitter: Jenkins
Branch: master

commit e72c273d46a8c010baea70211800153b90c458ab
Author: Chang Bo Guo <email address hidden>
Date: Mon Sep 16 18:15:03 2013 -0700

    Fix misused assertTrue in unit tests

    Refactored unit tests to use assertEqual instead of assertTrue
    where needed.

    Fixes bug #1226374

    Change-Id: Ie034536127b13653040e2beaa7247c2ecd13f4f0

Changed in keystone:
status: In Progress → Fix Committed

Fix proposed to branch: master
Review: https://review.openstack.org/47072

Changed in ironic:
assignee: nobody → ChangBo Guo (guochbo)
status: New → In Progress

Fix proposed to branch: master
Review: https://review.openstack.org/47073

Changed in oslo:
assignee: nobody → ChangBo Guo (guochbo)
status: New → In Progress

Fix proposed to branch: master
Review: https://review.openstack.org/47074

Changed in melange:
assignee: nobody → ChangBo Guo (guochbo)
status: New → In Progress
no longer affects: melange
no longer affects: neutron

Reviewed: https://review.openstack.org/47073
Committed: http://github.com/openstack/oslo-incubator/commit/6703c5cc744b183fea69cef42d03d4d3498cd22c
Submitter: Jenkins
Branch: master

commit 6703c5cc744b183fea69cef42d03d4d3498cd22c
Author: Chang Bo Guo <email address hidden>
Date: Tue Sep 17 19:55:35 2013 -0700

    Fix misused assertTrue in unit tests

    Refactored unit tests to use assertEqual instead of assertTrue
    where needed.

    Fixes bug #1226374

    Change-Id: I87308eb7871a55ef0fc49ca466311ed202b62d47

Changed in oslo:
status: In Progress → Fix Committed
ZhiQiang Fan (aji-zqfan) on 2013-09-18
Changed in cinder:
assignee: nobody → ZhiQiang Fan (aji-zqfan)
no longer affects: cinder

Reviewed: https://review.openstack.org/47072
Committed: http://github.com/openstack/ironic/commit/190a0cce25ca9395b58a07512558bdab22207d61
Submitter: Jenkins
Branch: master

commit 190a0cce25ca9395b58a07512558bdab22207d61
Author: Chang Bo Guo <email address hidden>
Date: Tue Sep 17 19:17:29 2013 -0700

    Fix misused assertTrue in unit tests

    Refactored unit tests to use assertEqual instead of assertTrue
    where needed.

    Fixes bug #1226374

    Change-Id: I6ba14f94d49e071c4d2208f946befca915c83d53

Changed in ironic:
status: In Progress → Fix Committed
ZhiQiang Fan (aji-zqfan) wrote :

@Chang Bo Guo
swift:
test/unit/common/test_utils.py:1620 self.assertTrue(iter_file.closed, True)
test/unit/common/test_utils.py:1627 self.assertTrue(iter_file.closed, True)

i think the two lines are not correct too, since the second is a msg, True can be printed too ('True'), but makes no sense here

what's your opinion?

can i fix this? i'm glad to help

ChangBo Guo(gcb) (glongwave) wrote :

@ZhiQiang ,
   go ahead , thanks a lot :)

Reviewed: https://review.openstack.org/46853
Committed: http://github.com/openstack/tempest/commit/4ae51d4133696c6bdf1acde1244f544e40804538
Submitter: Jenkins
Branch: master

commit 4ae51d4133696c6bdf1acde1244f544e40804538
Author: Chang Bo Guo <email address hidden>
Date: Mon Sep 16 18:01:39 2013 -0700

    Fix misused assertTrue in unit tests

    Refactored unit tests to use assertEqual instead of assertTrue
    where needed.

    Fixes bug #1226374

    Change-Id: I6ee1e3671054c4604ebb38ca9ceb52e8aef28540

Changed in tempest:
status: In Progress → Fix Committed

Fix proposed to branch: master
Review: https://review.openstack.org/47600

Changed in swift:
assignee: nobody → ZhiQiang Fan (aji-zqfan)
status: New → In Progress

Reviewed: https://review.openstack.org/47600
Committed: http://github.com/openstack/swift/commit/92ae497800d9e66795346019cf284026a751597e
Submitter: Jenkins
Branch: master

commit 92ae497800d9e66795346019cf284026a751597e
Author: ZhiQiang Fan <email address hidden>
Date: Fri Sep 20 23:34:06 2013 +0800

    Fix unsuitable assertTrue

    assertTrue accepts a parameter msg which will be printed when
    assertion fails, usually msg is a str. This patch fixes unsuitable
    usage of assertTrue which set msg to bool type True.

    Change-Id: I731f8ea553c935eba0e112ffded16f41a5ea86c0
    Fixes-Bug: #1226374

Changed in swift:
status: In Progress → Fix Committed
no longer affects: cinder
Changed in ironic:
importance: Undecided → Medium
Mark McLoughlin (markmc) on 2013-09-24
Changed in oslo:
importance: Undecided → Low
importance: Low → Medium

Reviewed: https://review.openstack.org/46975
Committed: http://github.com/openstack/python-keystoneclient/commit/baa949017a42368ec1e92e49490086364befe5da
Submitter: Jenkins
Branch: master

commit baa949017a42368ec1e92e49490086364befe5da
Author: Chang Bo Guo <email address hidden>
Date: Tue Sep 17 08:41:51 2013 -0700

    Fix misused assertTrue in unit tests

    Refactored unit tests to use assertEqual instead of assertTrue
    where needed.

    Fixes bug #1226374

    Change-Id: I678b2e7fcc522c8776c7fc0a554c1fc229ab781e

Changed in python-keystoneclient:
status: In Progress → Fix Committed
Download full text (17.5 KiB)

Reviewed: https://review.openstack.org/48990
Committed: http://github.com/openstack/swift/commit/175a837befd57b520e28623f0d708d883c023532
Submitter: Jenkins
Branch: feature/ec

commit 4c4a8abaa500d0d3940d81a4eb5ac21215ddc07a
Author: Kun Huang <email address hidden>
Date: Fri Sep 27 15:25:53 2013 +0800

    improve bulk document

    This a very small change which just tell users request url of bulk
    delete request. In original docstrings, it just states the request
    parameters, request body and request method but not request url.

    Change-Id: I0bbc302a0e072910bb58e4814614d7f761433b10

commit df39602c41605c4c68a47c6532a466ccc1a6633d
Author: David Goetz <email address hidden>
Date: Thu Sep 12 07:38:23 2013 -0700

    bulk delete bug with trailing whitespace

    Change-Id: Ia48224a1a187a8ed6b0c9a3c72cac06f084a6fc8

commit d8e0492ea80adae990f35930465d6e905a3be061
Author: Samuel Merritt <email address hidden>
Date: Tue Aug 27 18:00:04 2013 -0700

    Fix internal swift.source tracking.

    In 1.8.0 (Grizzly), your proxy logs would indicate which middleware
    was responsible for an internal request, e.g. TU for tempurl or BD for
    bulk delete. At some point, those all turned into GET_INFO, which does
    not give you any idea which specific middleware was responsible, only
    that it came from a get_account_info/get_container_info call.

    This commit puts it back to how it was in 1.8.0. Also, the
    new-since-1.8.0 function get_object_info() got swift_source plumbing
    added to it, so source tracking for the quota middlewares'
    get_object_info() calls will happen now too.

    Note that due to the new-since-1.8.0 in-environment caching of
    account/container info, you may not see as many lines in the proxy log
    as you would with 1.8.0. This is because there are actually fewer
    internal requests being made.

    Change-Id: I2b2ff7823c612dc7ed7f268da979c4500bbbe911

commit d9d7b2135a7020cdf43172ea4fcf0b1020f49101
Author: Samuel Merritt <email address hidden>
Date: Tue Sep 24 16:43:33 2013 -0700

    Install libffi-dev in SAIO docs.

    If you don't, then newer versions of xattr won't install, and since
    our xattr requirement is simply ">= 0.4" in requirements.txt, this
    affects anyone setting up a new SAIO.

    This happened with xattr 0.7, which was released on 2013-07-19.

    Change-Id: Iaf335fa25a2908953d1fd218158ebedf5d01cc27

commit ce5e810fed8c453f4cd41c3c32162f47cde48f10
Author: Samuel Merritt <email address hidden>
Date: Tue Sep 24 16:20:28 2013 -0700

    Update SAIO doc to have double proxy-logging in pipeline.

    Change-Id: I0a034ca1420761cbf4e35dcea1d9cd18a92f90bd

commit 3e6f9293b8882cecb151e87fe5bfbe24e605b847
Author: Brian D. Burns <email address hidden>
Date: Thu Aug 1 14:50:03 2013 -0400

    update SLO delete error handling

    * ensure all responses are 200 OK
    * report missing sub-SLO manifests or other error messages in bulk
      delete response

    Change-Id: Iaf88c94bc7114ff3c9751f9f31f8f748de911f8a

commit 92ae497800d9e66795346019cf284026a751597e
Author: ZhiQiang Fan <email address hidden>
Date: Fri Sep 20 23:34:06 201...

Thierry Carrez (ttx) on 2013-10-02
Changed in oslo:
milestone: none → havana-rc1
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-10-02
Changed in keystone:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-10-03
Changed in heat:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-10-09
Changed in swift:
milestone: none → 1.10.0-rc1
status: Fix Committed → Fix Released

Reviewed: https://review.openstack.org/46868
Committed: http://github.com/openstack/glance/commit/b040c1aecc989db14134f0a3843de3b4abe7ba8c
Submitter: Jenkins
Branch: master

commit b040c1aecc989db14134f0a3843de3b4abe7ba8c
Author: Chang Bo Guo <email address hidden>
Date: Mon Sep 16 20:41:24 2013 -0700

    Fix misused assertTrue in unit tests

    Refactored unit tests to use assertEqual instead of assertTrue
    where needed.

    Fixes bug #1226374

    Change-Id: Iac5117c7a8dd3769aa6e303c6830aad291d5a0f7

Changed in glance:
status: In Progress → Fix Committed
Dolph Mathews (dolph) on 2013-10-09
Changed in python-keystoneclient:
milestone: 0.3.3 → 0.4.0
Dolph Mathews (dolph) on 2013-10-09
Changed in python-keystoneclient:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-10-17
Changed in oslo:
milestone: havana-rc1 → 2013.2
Thierry Carrez (ttx) on 2013-10-17
Changed in heat:
milestone: havana-rc1 → 2013.2
Thierry Carrez (ttx) on 2013-10-17
Changed in swift:
milestone: 1.10.0-rc1 → 1.10.0
Thierry Carrez (ttx) on 2013-10-17
Changed in keystone:
milestone: havana-rc1 → 2013.2
Thierry Carrez (ttx) on 2013-12-04
Changed in glance:
milestone: none → icehouse-1
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-12-09
Changed in ironic:
milestone: none → icehouse-1
status: Fix Committed → Fix Released
Sean Dague (sdague) on 2013-12-12
Changed in tempest:
status: Fix Committed → Fix Released
assignee: ChangBo Guo (guochbo) → nobody

Reviewed: https://review.openstack.org/61816
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=aa51f82c9d05ff581cd35cb476c7900a74a839b8
Submitter: Jenkins
Branch: master

commit aa51f82c9d05ff581cd35cb476c7900a74a839b8
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Fri Dec 13 13:07:41 2013 +0200

    Fix misused assertTrue in unit tests

    one additional case of misuse of assertTrue was found

    Change-Id: If62f5800566921433fc098dbba0188f69fbb6eb1
    Closes-Bug: #1226374

Reviewed: https://review.openstack.org/63089
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=480289776ece24f25baae47952917269e6355a4b
Submitter: Jenkins
Branch: master

commit 480289776ece24f25baae47952917269e6355a4b
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Thu Dec 19 13:23:06 2013 +0200

    Fix misused assertTrue in unit tests

    The patch merged under review https://review.openstack.org/#/c/61816/3
    still uses assertTrue instead of assertEqual.

    This patch properly corrects this.

    Change-Id: I09fd4b7355dd721b513febd0669cb1368b5cda19
    Closes-Bug: #1226374

Thierry Carrez (ttx) on 2014-04-17
Changed in glance:
milestone: icehouse-1 → 2014.1
Thierry Carrez (ttx) on 2014-04-17
Changed in ironic:
milestone: icehouse-1 → 2014.1
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers