settingsLogin | Registersettings

[openstack-dev] [all][tests] Fix it friday! [mock failure in CI]

0 votes

Good news everybody, mock 1.1.0 is now out. This backports all the
improvements over the last couple of years, making it fully
synchronised with cPython master. Yay.

Bad news. Lots of unit tests jobs have suffered falled from this.

But - none of the things I've looked into so far are bugs in mock 1.1.0.

One of the improvements in mock is to fail when a bad method is called.

Consider this: https://review.openstack.org/#/c/200384/1/taskflow/tests/unit/test_engine_helpers.py

Note the old method: mockimport.assertcalledonecwith(name)

That method never existed. onec is a typo :).

mock 1.0.1 silently accepts that - thats part of its job. But, its a
very fragile API.

1.1.0 makes that an error, for methods with assert prefixes - unless
unsafe is specifically requested. So a big chunk of the failing tests
are tests that were not testing anything at all.
One common exampled of that is 'assert_called' - another method that
never ever existed. All our tests using that were testing nothing at
all.

Neutron is failing on a bunch of tests that accessed a private
function inside mock. I'm surprised reviewers didn't spot this, but
_patch isn't part of the public API, and never was.

Tempest seems to be failing due to a different object being returned -
I haven't dug deep enough to describe the cause in more detail.

We can probably pin mock back to 1.0.1 locally within projects to gain
breathing room, but given the API improvements in 1.1.0 (see below[1]
as readthedocs is failing to build it due to having an old pip in
their virtualenvs :/) - I think we'll be much better off adopting it
as quickly as we can. NB: with this release we don't need to use
'unittest.mock' for Python3.4 - we can just standardise on using
'mock' across the board, which is much simpler and easier to reason
about (same codebase everywhere[2])

[2]: Python 2.6 support was dropped in 1.1.0, so we need to use
markers to select 1.0.1 for the remaining 2.6 gate jobs. (We should
kill those of asap).
[1]:
- Issue #23310: Fix MagicMock's initializer to work with methods, just
like configure_mock(). Patch by Kasia Jachim.

  • Issue #23568: Add rdivmod support to MagicMock() objects.
    Patch by Håkan Lövdahl.

  • Issue #23581: Add matmul support to MagicMock. Patch by Håkan Lövdahl.

  • Issue #23326: Removed ne implementations. Since fixing default ne
    implementation in issue #21408 they are redundant. *** NOT BACKPORTED ***

  • Issue #21270: We now override tuple methods in mock.call objects so that
    they can be used as normal call attributes.

  • Issue #21256: Printout of keyword args should be in deterministic order in
    a mock function call. This will help to write better doctests.

  • Issue #21262: New method assertnotcalled for Mock.
    It raises AssertionError if the mock has been called.

  • Issue #21238: New keyword argument unsafe to Mock. It raises
    AttributeError incase of an attribute startswith assert or assret.

  • Issue #21239: patch.stopall() didn't work deterministically when the same
    name was patched more than once.

  • Issue #21222: Passing name keyword argument to mock.create_autospec now
    works.

  • Issue #17826: setting an iterable sideeffect on a mock function created by
    create
    autospec now works. Patch by Kushal Das.

  • Issue #17826: setting an iterable sideeffect on a mock function created by
    create
    autospec now works. Patch by Kushal Das.

  • Issue #20968: unittest.mock.MagicMock now supports division.
    Patch by Johannes Baiter.

  • Issue #20189: unittest.mock now no longer assumes that any object for
    which it could get an inspect.Signature is a callable written in Python.
    Fix courtesy of Michael Foord.

  • Issue #17467: add readline and readlines support to mock_open in
    unittest.mock.

  • Issue #17015: When it has a spec, a Mock object now inspects its signature
    when matching calls, so that arguments can be matched positionally or
    by name.

  • Issue #15323: improve failure message of Mock.assertcalledonce_with

  • Issue #14857: fix regression in references to PEP 3135 implicit class
    closure variable (Reopens issue #12370)

  • Issue #14295: Add unittest.mock

-Rob

--
Robert Collins rbtcollins@hp.com
Distinguished Technologist
HP Converged Cloud


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
asked Jul 10, 2015 in openstack-dev by Robert_Collins (27,200 points)   4 6 12

25 Responses

0 votes

Robert Collins wrote:
Good news everybody, mock 1.1.0 is now out. This backports all the
improvements over the last couple of years, making it fully
synchronised with cPython master. Yay.

Bad news. Lots of unit tests jobs have suffered falled from this.

But - none of the things I've looked into so far are bugs in mock 1.1.0.

One of the improvements in mock is to fail when a bad method is called.

Consider this: https://review.openstack.org/#/c/200384/1/taskflow/tests/unit/test_engine_helpers.py

Note the old method: mockimport.assertcalledonecwith(name)

That method never existed. onec is a typo :).

mock 1.0.1 silently accepts that - thats part of its job. But, its a
very fragile API.

1.1.0 makes that an error, for methods with assert prefixes - unless
unsafe is specifically requested. So a big chunk of the failing tests
are tests that were not testing anything at all.
One common exampled of that is 'assert_called' - another method that
never ever existed. All our tests using that were testing nothing at
all.

It's still non-trivial to fix, especially as fixing it may expose a real
bug in the never-actually-tested thing.

Releasing this on a Friday sounds like bad timing, especially without
advance notice that we'd have to rush to fix the dozens of new issues it
would expose.

Now that we have data[1] showing what fails, it's not completely crazy
to pin it and buy us some time ?

[1]
http://lists.openstack.org/pipermail/openstack-stable-maint/2015-July/thread.html

--
Thierry Carrez (ttx)


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Jul 10, 2015 by Thierry_Carrez (57,480 points)   3 8 13
0 votes

How do we test to see what is failing in each project with the new version?

Also, I'm responsible for the reference to the private mock method in
Neutron. That particular reference is to prevent people from patching the
same target twice because mock.patch.stopall() unwinds patches in a
non-deterministic order in < py34 which leads to leftover monkey patches.
These caused completely unrelated unit tests to randomly and inexplicably
explode later. More details here:
https://github.com/openstack/neutron/commit/1b60df85ba3ad442c2e4e7e52538e1b9a1bf9378

Cheers,
Kevin Benton

On Fri, Jul 10, 2015 at 12:45 AM, Robert Collins robertc@robertcollins.net
wrote:

Good news everybody, mock 1.1.0 is now out. This backports all the
improvements over the last couple of years, making it fully
synchronised with cPython master. Yay.

Bad news. Lots of unit tests jobs have suffered falled from this.

But - none of the things I've looked into so far are bugs in mock 1.1.0.

One of the improvements in mock is to fail when a bad method is called.

Consider this:
https://review.openstack.org/#/c/200384/1/taskflow/tests/unit/test_engine_helpers.py

Note the old method: mockimport.assertcalledonecwith(name)

That method never existed. onec is a typo :).

mock 1.0.1 silently accepts that - thats part of its job. But, its a
very fragile API.

1.1.0 makes that an error, for methods with assert prefixes - unless
unsafe is specifically requested. So a big chunk of the failing tests
are tests that were not testing anything at all.
One common exampled of that is 'assert_called' - another method that
never ever existed. All our tests using that were testing nothing at
all.

Neutron is failing on a bunch of tests that accessed a private
function inside mock. I'm surprised reviewers didn't spot this, but
_patch isn't part of the public API, and never was.

Tempest seems to be failing due to a different object being returned -
I haven't dug deep enough to describe the cause in more detail.

We can probably pin mock back to 1.0.1 locally within projects to gain
breathing room, but given the API improvements in 1.1.0 (see below[1]
as readthedocs is failing to build it due to having an old pip in
their virtualenvs :/) - I think we'll be much better off adopting it
as quickly as we can. NB: with this release we don't need to use
'unittest.mock' for Python3.4 - we can just standardise on using
'mock' across the board, which is much simpler and easier to reason
about (same codebase everywhere[2])

[2]: Python 2.6 support was dropped in 1.1.0, so we need to use
markers to select 1.0.1 for the remaining 2.6 gate jobs. (We should
kill those of asap).
[1]:
- Issue #23310: Fix MagicMock's initializer to work with methods, just
like configure_mock(). Patch by Kasia Jachim.

  • Issue #23568: Add rdivmod support to MagicMock() objects.
    Patch by Håkan Lövdahl.

  • Issue #23581: Add matmul support to MagicMock. Patch by Håkan Lövdahl.

  • Issue #23326: Removed ne implementations. Since fixing default
    ne
    implementation in issue #21408 they are redundant. *** NOT BACKPORTED ***

  • Issue #21270: We now override tuple methods in mock.call objects so that
    they can be used as normal call attributes.

  • Issue #21256: Printout of keyword args should be in deterministic order
    in
    a mock function call. This will help to write better doctests.

  • Issue #21262: New method assertnotcalled for Mock.
    It raises AssertionError if the mock has been called.

  • Issue #21238: New keyword argument unsafe to Mock. It raises
    AttributeError incase of an attribute startswith assert or assret.

  • Issue #21239: patch.stopall() didn't work deterministically when the same
    name was patched more than once.

  • Issue #21222: Passing name keyword argument to mock.create_autospec now
    works.

  • Issue #17826: setting an iterable sideeffect on a mock function created
    by
    create
    autospec now works. Patch by Kushal Das.

  • Issue #17826: setting an iterable sideeffect on a mock function created
    by
    create
    autospec now works. Patch by Kushal Das.

  • Issue #20968: unittest.mock.MagicMock now supports division.
    Patch by Johannes Baiter.

  • Issue #20189: unittest.mock now no longer assumes that any object for
    which it could get an inspect.Signature is a callable written in Python.
    Fix courtesy of Michael Foord.

  • Issue #17467: add readline and readlines support to mock_open in
    unittest.mock.

  • Issue #17015: When it has a spec, a Mock object now inspects its
    signature
    when matching calls, so that arguments can be matched positionally or
    by name.

  • Issue #15323: improve failure message of Mock.assertcalledonce_with

  • Issue #14857: fix regression in references to PEP 3135 implicit class
    closure variable (Reopens issue #12370)

  • Issue #14295: Add unittest.mock

-Rob

--
Robert Collins rbtcollins@hp.com
Distinguished Technologist
HP Converged Cloud


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

--
Kevin Benton


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Jul 10, 2015 by Kevin_Benton (24,800 points)   3 5 7
0 votes

On 10 July 2015 at 20:23, Kevin Benton blak111@gmail.com wrote:
How do we test to see what is failing in each project with the new version?

Look at any CI failure in the last 5 hours or so.

Or run tox :).

Also, I'm responsible for the reference to the private mock method in
Neutron. That particular reference is to prevent people from patching the
same target twice because mock.patch.stopall() unwinds patches in a
non-deterministic order in < py34 which leads to leftover monkey patches.
These caused completely unrelated unit tests to randomly and inexplicably
explode later. More details here:
https://github.com/openstack/neutron/commit/1b60df85ba3ad442c2e4e7e52538e1b9a1bf9378

Thats fixed in 1.1.0. So you should be able to unwind that. If you
need a short term workaround, its in mock.mock now.

-Rob

--
Robert Collins rbtcollins@hp.com
Distinguished Technologist
HP Converged Cloud


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Jul 10, 2015 by Robert_Collins (27,200 points)   4 6 12
0 votes

Kevin Benton wrote:
How do we test to see what is failing in each project with the new version?

Just watch one of the thousands tests currently failing in zuul:
http://status.openstack.org/zuul/

Or see the recent periodic stable maint jobs fail reports at:
http://lists.openstack.org/pipermail/openstack-stable-maint/2015-July/thread.html

(the stable/kilo stuff is relatively recent and likely to apply to cover
95% of the master issues as well)

--
Thierry Carrez (ttx)


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Jul 10, 2015 by Thierry_Carrez (57,480 points)   3 8 13
0 votes

On 10 July 2015 at 20:18, Thierry Carrez thierry@openstack.org wrote:
Robert Collins wrote:

Good news everybody, mock 1.1.0 is now out. This backports all the
...
1.1.0 makes that an error, for methods with assert prefixes - unless
unsafe is specifically requested. So a big chunk of the failing tests
are tests that were not testing anything at all.
One common exampled of that is 'assert_called' - another method that
never ever existed. All our tests using that were testing nothing at
all.

It's still non-trivial to fix, especially as fixing it may expose a real
bug in the never-actually-tested thing.

Releasing this on a Friday sounds like bad timing, especially without
advance notice that we'd have to rush to fix the dozens of new issues it
would expose.

There would never be a good time to release such things. The whole
point of the constraints system we're rolling out is to make us not
sensitive to the release schedule of upstreams: we can't dictate the
release schedule of the world to be convenient to our cycles.

Further to that we've already rolled out the constraints system for
devstack, so this is not a gate wedge. Its per-project in unit tests,
and it can be fixed one project at a time. There's no rush that makes
this worse for anyone than if it came out on Monday. We don't need [at
least so far as I can tell - maybe the neutron AAS's will be
interlocked - but again, that would happen any day] infra intervention
to force-push stuff in anywhere.

The one thing that we may need, for py26 projects, is for this review:
https://review.openstack.org/#/c/200344/ - to get in asap, for any
projects with python2.6 jobs, since 1.1.0 isn't compatible with 2.6.
That will let folk with 26 jobs update their requirements locally and
easily, without tox.ini hacks.

Now that we have data[1] showing what fails, it's not completely crazy
to pin it and buy us some time ?

Pinning will be per repo, because its unittests not devstack. So its
doable if we have to, but since we have to land a patch in the
repositories anyway, trying for a fix first would be good IMO.

-Rob

--
Robert Collins rbtcollins@hp.com
Distinguished Technologist
HP Converged Cloud


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Jul 10, 2015 by Robert_Collins (27,200 points)   4 6 12
0 votes

Thanks. I didn't realize it was already breaking everything. I thought it
might have been stuck in requirements bump patch somewhere.

Thats fixed in 1.1.0. So you should be able to unwind that. If you need a
short term workaround, its in mock.mock now.

Yeah, I know it's fixed, I reported the upstream bug quite some time ago.
:) I just felt the need to explain because you implied that the Neutron
reviewers missed something when the patch that introduced that was very
intentional due to the many times that bug has caused non-deterministic
failures.

On Fri, Jul 10, 2015 at 1:28 AM, Robert Collins robertc@robertcollins.net
wrote:

On 10 July 2015 at 20:23, Kevin Benton blak111@gmail.com wrote:

How do we test to see what is failing in each project with the new
version?

Look at any CI failure in the last 5 hours or so.

Or run tox :).

Also, I'm responsible for the reference to the private mock method in
Neutron. That particular reference is to prevent people from patching the
same target twice because mock.patch.stopall() unwinds patches in a
non-deterministic order in < py34 which leads to leftover monkey patches.
These caused completely unrelated unit tests to randomly and inexplicably
explode later. More details here:

https://github.com/openstack/neutron/commit/1b60df85ba3ad442c2e4e7e52538e1b9a1bf9378

Thats fixed in 1.1.0. So you should be able to unwind that. If you
need a short term workaround, its in mock.mock now.

-Rob

--
Robert Collins rbtcollins@hp.com
Distinguished Technologist
HP Converged Cloud


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

--
Kevin Benton


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Jul 10, 2015 by Kevin_Benton (24,800 points)   3 5 7
0 votes

On 10 July 2015 at 20:50, Kevin Benton blak111@gmail.com wrote:
Thanks. I didn't realize it was already breaking everything. I thought it
might have been stuck in requirements bump patch somewhere.

Thats fixed in 1.1.0. So you should be able to unwind that. If you need a
short term workaround, its in mock.mock now.

Yeah, I know it's fixed, I reported the upstream bug quite some time ago. :)
I just felt the need to explain because you implied that the Neutron
reviewers missed something when the patch that introduced that was very
intentional due to the many times that bug has caused non-deterministic
failures.

Fair enough; I was feeling a little miffed at breaking so many
projects; sorry :)

-Rob

--
Robert Collins rbtcollins@hp.com
Distinguished Technologist
HP Converged Cloud


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Jul 10, 2015 by Robert_Collins (27,200 points)   4 6 12
0 votes

No prob. The fixes for Neutron were relatively trivial.
https://review.openstack.org/#/c/200420/

The only one that was a bit surprising was the failure of autospec in this
file:
https://review.openstack.org/#/c/200420/4/neutron/tests/unit/services/metering/agents/test_metering_agent.py

It was coming back with argument count mismatch failures. It seems like the
log decorator[1] trips up autospec in the new version.

1.
https://github.com/openstack/neutron/blob/master/neutron/services/metering/drivers/noop/noop_driver.py#L22

On Fri, Jul 10, 2015 at 2:16 AM, Robert Collins robertc@robertcollins.net
wrote:

On 10 July 2015 at 20:50, Kevin Benton blak111@gmail.com wrote:

Thanks. I didn't realize it was already breaking everything. I thought it
might have been stuck in requirements bump patch somewhere.

Thats fixed in 1.1.0. So you should be able to unwind that. If you need a
short term workaround, its in mock.mock now.

Yeah, I know it's fixed, I reported the upstream bug quite some time
ago. :)
I just felt the need to explain because you implied that the Neutron
reviewers missed something when the patch that introduced that was very
intentional due to the many times that bug has caused non-deterministic
failures.

Fair enough; I was feeling a little miffed at breaking so many
projects; sorry :)

-Rob

--
Robert Collins rbtcollins@hp.com
Distinguished Technologist
HP Converged Cloud


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

--
Kevin Benton


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Jul 10, 2015 by Kevin_Benton (24,800 points)   3 5 7
0 votes

On 10 July 2015 at 22:07, Kevin Benton blak111@gmail.com wrote:
No prob. The fixes for Neutron were relatively trivial.
https://review.openstack.org/#/c/200420/

The only one that was a bit surprising was the failure of autospec in this
file:
https://review.openstack.org/#/c/200420/4/neutron/tests/unit/services/metering/agents/test_metering_agent.py

It was coming back with argument count mismatch failures. It seems like the
log decorator[1] trips up autospec in the new version.

1.
https://github.com/openstack/neutron/blob/master/neutron/services/metering/drivers/noop/noop_driver.py#L22

Hmm, that might be a flaw in funcsigs, the inspect.signature backport.

We should figure it out - does it get tripped up in Python 3.5 with
the unittest.mock variant? If so, bugs.python.org for the bug. If not,
then please file the bug (and how you tested - the tighter the
reproduction the better) at
https://github.com/testing-cabal/mock/issues.

Thanks!
-Rob


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Jul 10, 2015 by Robert_Collins (27,200 points)   4 6 12
0 votes

On 07/10/2015 03:45 AM, Robert Collins wrote:
Good news everybody, mock 1.1.0 is now out. This backports all the
improvements over the last couple of years, making it fully
synchronised with cPython master. Yay.

Bad news. Lots of unit tests jobs have suffered falled from this.

But - none of the things I've looked into so far are bugs in mock 1.1.0.

One of the improvements in mock is to fail when a bad method is called.

Consider this: https://review.openstack.org/#/c/200384/1/taskflow/tests/unit/test_engine_helpers.py

Note the old method: mockimport.assertcalledonecwith(name)

That method never existed. onec is a typo :).

mock 1.0.1 silently accepts that - thats part of its job. But, its a
very fragile API.

1.1.0 makes that an error, for methods with assert prefixes - unless
unsafe is specifically requested. So a big chunk of the failing tests
are tests that were not testing anything at all.
One common exampled of that is 'assert_called' - another method that
never ever existed. All our tests using that were testing nothing at
all.

Neutron is failing on a bunch of tests that accessed a private
function inside mock. I'm surprised reviewers didn't spot this, but
_patch isn't part of the public API, and never was.

Tempest seems to be failing due to a different object being returned -
I haven't dug deep enough to describe the cause in more detail.

The Tempest fix is here - https://review.openstack.org/#/c/200449/ and
was pretty straight forward. It was another one of those assert_*
doesn't exist issues. I just approved it though.

-Sean

--
Sean Dague
http://dague.net


OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
responded Jul 10, 2015 by Sean_Dague (66,200 points)   4 10 16
...