Move dict test matchers into testtools

Bug #1199536 reported by Ghe Rivero
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ironic
Fix Released
Low
Unassigned
OpenStack Compute (nova)
Wishlist
Unassigned
OpenStack DBaaS (Trove)
Triaged
Wishlist
Unassigned
OpenStack Identity (keystone)
Wishlist
Unassigned
oslo-incubator
Undecided
Ghe Rivero
testtools
Undecided
Jonathan Lange

Bug Description

Reduce code duplication by pulling DictKeysMismatch, DictMismatch and DictMatches from glanceclient/tests/matchers.py into a library (e.g. testtools)

aeva black (tenbrae)
Changed in ironic:
importance: Undecided → Low
status: New → Triaged
Revision history for this message
Michael Davies (mrda) wrote :

mrda@lawrence:~/src/openstack$ find . -name matchers.py
./nova/nova/tests/matchers.py
./keystone/keystone/tests/matchers.py
./ironic/ironic/tests/matchers.py
./trove/trove/tests/unittests/util/matchers.py

nova and trove share almost identical versions of these files, and ironic has content almost identical (includes Dict matches, but excludes XML matches). keystone has a very cut-down version.

Ghe Rivero (ghe.rivero)
Changed in ironic:
assignee: nobody → Ghe Rivero (ghe.rivero)
Ghe Rivero (ghe.rivero)
Changed in oslo:
assignee: nobody → Ghe Rivero (ghe.rivero)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to oslo-incubator (master)

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

Changed in oslo:
status: New → In Progress
Matt Riedemann (mriedem)
tags: added: oslo testing
Joe Gordon (jogo)
Changed in nova:
importance: Undecided → Wishlist
status: New → Incomplete
status: Incomplete → Triaged
Changed in keystone:
assignee: nobody → Mikhail Durnosvistov (mdurnosvistov)
Changed in nova:
assignee: nobody → Mikhail Durnosvistov (mdurnosvistov)
Dolph Mathews (dolph)
Changed in keystone:
importance: Undecided → Wishlist
status: New → Triaged
Revision history for this message
Doug Hellmann (doug-hellmann) wrote : Re: Move dict test matchers into oslo

Should these classes go directly to testtools instead of an oslo library? https://launchpad.net/testtools

Changed in trove:
status: New → Triaged
importance: Undecided → Wishlist
Revision history for this message
Dmitry Tantsur (divius) wrote :

Hi! Any updates on this issue? Ghe, are you working on it? If not, could you please unassign yourself?

Changed in keystone:
assignee: Mikhail Durnosvistov (mdurnosvistov) → nobody
Changed in nova:
assignee: Mikhail Durnosvistov (mdurnosvistov) → nobody
Revision history for this message
Dmitry Tantsur (divius) wrote :

Hi! As there was no new activity on this bug for a while, I'm unassigning it. Feel free to assign yourself, if you're still working on this bug.

Changed in ironic:
assignee: Ghe Rivero (ghe.rivero) → nobody
Revision history for this message
Dmitry Tantsur (divius) wrote :

It's also not clear to me, whether the change actually got to Oslo - review mentioned here is abandoned

Revision history for this message
Ben Nemec (bnemec) wrote :

It doesn't look like this ever merged. It probably belongs in testtools, unless they won't take it for some reason.

Revision history for this message
Doug Hellmann (doug-hellmann) wrote :

The change was submitted to oslotest in https://review.openstack.org/#/c/74861/ and in that review I said:

I discussed this change with lifeless and he indicated that he would be very happy to have something like this (without committing to accepting this exact patch after only a quick review). Please submit a pull request to testtools (https://github.com/testing-cabal/testtools) and when it's committed we can bump the version of testtools OpenStack uses.

Changed in oslo:
status: In Progress → Won't Fix
summary: - Move dict test matchers into oslo
+ Move dict test matchers into testtools
Revision history for this message
Viktor Serhieiev (vsergeyev) wrote :

@Doug,

pulling changes to testtools can take a lot of time (for example, we tried to add thee assertRaisesRegexp() method in February), so IMO, we can put them to the oslotest, and then remove, when we'll get this matchers in testtools

Revision history for this message
Robert Collins (lifeless) wrote :

@Victor we've no waiting-on-review pull requests open in testtools. Please give me a link to your pull request so I can see how the ball has been dropped. We generally have a couple day turnaround on testtools changes that are in good shape.

description: updated
Revision history for this message
Viktor Serhieiev (vsergeyev) wrote :

@Robert,

please see pull request to testtools , which was sent on 3 Feb - [1]. For a some reasons, it was ignored during 6 months.

[1] https://github.com/testing-cabal/testtools/pull/75

Revision history for this message
Robert Collins (lifeless) wrote :

Can you rebase it please as I asked? YEs, sorry about the long delay there.

Revision history for this message
Davanum Srinivas (DIMS) (dims-v) wrote :

This need not be tracked by Nova.

Changed in nova:
status: Triaged → Won't Fix
Revision history for this message
Robert Collins (lifeless) wrote :

Still pending the patch being rebased for testtools.

Changed in testtools:
status: New → Incomplete
Revision history for this message
David Stanek (dstanek) wrote :

Keystone no longer maintains those matchers so there isn't anything to fix anymore. The keystone/test/matcher.py module was removed in https://review.openstack.org/#/c/154242/

Changed in keystone:
status: Triaged → Invalid
Revision history for this message
Jonathan Lange (jml) wrote :

https://github.com/testing-cabal/testtools/pull/185 has a rebased version of the assertRaisesRegexp patch, with tweaks.

Changed in testtools:
status: Incomplete → Triaged
assignee: nobody → Jonathan Lange (jml)
status: Triaged → In Progress
Revision history for this message
Jonathan Lange (jml) wrote :

I've merged https://github.com/testing-cabal/testtools/pull/185. Marking this Fix Committed for testtools.

Changed in testtools:
status: In Progress → Fix Committed
milestone: none → next
Jonathan Lange (jml)
Changed in testtools:
status: Fix Committed → Fix Released
Changed in trove:
assignee: nobody → Gábor Antal (gabor.antal)
Changed in trove:
assignee: Gábor Antal (gabor.antal) → nobody
Revision history for this message
Anup (anup-d-navare) wrote :

This was fixed in Ironic:

commit 9670d2bd26a6228f6150f4698b2c55b4348af395
Author: Pavlo Shchelokovskyy <email address hidden>
Date: Wed Sep 16 22:58:04 2015 +0300

    Remove DictMatches custom matcher from unit tests

    On Py27 assertDictEqual handles nested dictionaries just fine.

    Change-Id: I0695a1249a3c4153ba4cc49405640a3e09a2af04
    Closes-Bug: #1496552

Changed in ironic:
status: Triaged → Fix Committed
Michael Turek (mjturek)
Changed in ironic:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers