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

4.0.x: 'ValueError: Sentinels must not start with _' and others #487

Closed
2uasimojo opened this issue Feb 17, 2020 · 7 comments
Closed

4.0.x: 'ValueError: Sentinels must not start with _' and others #487

2uasimojo opened this issue Feb 17, 2020 · 7 comments

Comments

@2uasimojo
Copy link

2uasimojo commented Feb 17, 2020

With 4.0.0 and 4.0.1, tests start failing in several OpenStack projects. Example 1:

    b'Traceback (most recent call last):'
    b'  File "/home/efried/openstack/nova/nova/tests/unit/virt/libvirt/test_host.py", line 589, in test_cpu_features_bug_1217630'
    b"    with mock.patch('nova.virt.libvirt.host.libvirt') as mock_libvirt:"
    b'  File "/home/efried/openstack/nova/.tox/py37/lib/python3.7/site-packages/oslotest/mock_fixture.py", line 171, in __enter__'
    b'    _lazy_autospec_method(mocked_method, original_attr, eat_self)'
    b'  File "/home/efried/openstack/nova/.tox/py37/lib/python3.7/site-packages/oslotest/mock_fixture.py", line 27, in _lazy_autospec_method'
    b'    _lazy_autospec = mock.create_autospec(original_method)'
    b'  File "/home/efried/openstack/nova/.tox/py37/lib/python3.7/site-packages/mock/mock.py", line 2679, in create_autospec'
    b'    name=_name, **_kwargs)'
    b'  File "/home/efried/openstack/nova/.tox/py37/lib/python3.7/site-packages/mock/mock.py", line 2076, in __init__'
    b'    _safe_super(MagicMixin, self).__init__(*args, **kw)'
    b'  File "/home/efried/openstack/nova/.tox/py37/lib/python3.7/site-packages/mock/mock.py", line 439, in __init__'
    b'    self._mock_add_spec(spec, spec_set, _spec_as_instance, _eat_self)'
    b'  File "/home/efried/openstack/nova/.tox/py37/lib/python3.7/site-packages/mock/mock.py", line 494, in _mock_add_spec'
    b'    if iscoroutinefunction(getattr(spec, attr, None)):'
    b'  File "/home/efried/openstack/nova/.tox/py37/lib/python3.7/site-packages/mock/backports.py", line 34, in iscoroutinefunction'
    b"    getattr(obj, '_is_coroutine', None) is _is_coroutine"
    b'  File "/home/efried/openstack/nova/.tox/py37/lib/python3.7/site-packages/oslo_utils/fixture.py", line 82, in __getattr__'
    b"    raise ValueError('Sentinels must not start with _')"
    b'ValueError: Sentinels must not start with _'

(permalink to that source line: https://opendev.org/openstack/nova/src/commit/e69dbfa0d34d6b3f51282ac0ab51cdab3c2115e4/nova/tests/unit/virt/libvirt/test_host.py#L589)

Switching from import mock to from unittest import mock fixes this one.

We will add comments with other examples.

@2uasimojo
Copy link
Author

2uasimojo commented Feb 17, 2020

Example 2:

    b"2020-02-17 15:51:28,040 INFO [nova.console.websocketproxy] handler exception: Expected int or long, got <class 'mock.mock._AutospecMagicMock'>"
...
    b'Traceback (most recent call last):'
    b'  File "/home/efried/openstack/nova/nova/tests/unit/console/test_websocketproxy.py", line 627, in test_tcp_rst_no_compute_rpcapi'
    b'    self.assertIsNone(self.wh._compute_rpcapi)'
    b'  File "/home/efried/openstack/nova/.tox/py37/lib/python3.7/site-packages/testtools/testcase.py", line 426, in assertIsNone'
    b'    self.assertThat(observed, matcher, message)'
    b'  File "/home/efried/openstack/nova/.tox/py37/lib/python3.7/site-packages/testtools/testcase.py", line 498, in assertThat'
    b'    raise mismatch_error'
    b'testtools.matchers._impl.MismatchError: <nova.compute.rpcapi.ComputeAPI object at 0x7f865ddfa710> is not None'

permalink: https://opendev.org/openstack/nova/src/commit/e69dbfa0d34d6b3f51282ac0ab51cdab3c2115e4/nova/tests/unit/console/test_websocketproxy.py#L627

@2uasimojo 2uasimojo changed the title 4.0.1: 'ValueError: Sentinels must not start with _' 4.0.x: 'ValueError: Sentinels must not start with _' and others Feb 17, 2020
@johnsom
Copy link

johnsom commented Feb 17, 2020

We are seeing something similar:

Captured traceback:

    Traceback (most recent call last):
      File "/home/zuul/src/opendev.org/openstack/octavia/octavia/tests/unit/certificates/manager/test_barbican_legacy.py", line 81, in setUp
        self.secret1 = mock.Mock(spec=secrets.Secret)
      File "/home/zuul/src/opendev.org/openstack/octavia/.tox/py36/lib/python3.6/site-packages/mock/mock.py", line 1084, in __init__
        _spec_state, _new_name, _new_parent, **kwargs
      File "/home/zuul/src/opendev.org/openstack/octavia/.tox/py36/lib/python3.6/site-packages/mock/mock.py", line 439, in __init__
        self._mock_add_spec(spec, spec_set, _spec_as_instance, _eat_self)
      File "/home/zuul/src/opendev.org/openstack/octavia/.tox/py36/lib/python3.6/site-packages/mock/mock.py", line 494, in _mock_add_spec
        if iscoroutinefunction(getattr(spec, attr, None)):
      File "/home/zuul/src/opendev.org/openstack/octavia/.tox/py36/lib/python3.6/site-packages/mock/mock.py", line 2899, in __get__
        return self()
      File "/home/zuul/src/opendev.org/openstack/octavia/.tox/py36/lib/python3.6/site-packages/mock/mock.py", line 1100, in __call__
        return _mock_self._mock_call(*args, **kwargs)
      File "/home/zuul/src/opendev.org/openstack/octavia/.tox/py36/lib/python3.6/site-packages/mock/mock.py", line 1104, in _mock_call
        return _mock_self._execute_mock_call(*args, **kwargs)
      File "/home/zuul/src/opendev.org/openstack/octavia/.tox/py36/lib/python3.6/site-packages/mock/mock.py", line 1161, in _execute_mock_call
        raise effect
    ValueError

openstack-gerrit pushed a commit to openstack/requirements that referenced this issue Feb 18, 2020
testing-cabal/mock#487

Change-Id: I1da8453a5e838a3f9cc730655a3d9d8519d55f36
openstack-gerrit pushed a commit to openstack/openstack that referenced this issue Feb 18, 2020
* Update requirements from branch 'master'
  - blacklist bad mock releases 4.0.0 and 4.0.1
    
    testing-cabal/mock#487
    
    Change-Id: I1da8453a5e838a3f9cc730655a3d9d8519d55f36
@tirkarthi
Copy link
Contributor

@2uasimojo Please add a note on the version of Python. mock 4.0 added support to detect coroutines to return AsyncMock. It's trying to detect it by getting _is_coroutine. oslotest tries to do autospec and has a restriction that getattr cannot be used with _is_coroutine. From the traceback if you are on Python 3.7 this is something that might affect your upgrade to 3.8 too. So in one way I see at as an upstream issue that is detected by the backport.

@tirkarthi
Copy link
Contributor

@johnsom Please add a simplified reproducer for the issue on how to reproduce the failure. From the traceback yours seem to be a different issue. Thanks.

@tirkarthi
Copy link
Contributor

The issue reported by @johnsom can be reproduced as below. Please correct me if I understood the issue incorrectly.

from unittest.mock import Mock, PropertyMock

class Foo:
    prop = PropertyMock(side_effect=ValueError)

mock = Mock(spec=Foo)

In upstream cpython mock with 3.7 there is no error. In Python 3.8 and master it triggers ValueError. The cause in both issues reported is that we rely on getattr to detect coroutine that might have side effects that were not present before. Both will be upstream issues when the projects upgrade to 3.8. I am not sure of a way to get the attribute without triggering the side effect of getattr that could possibly solve the issue.

@cjw296
Copy link
Collaborator

cjw296 commented Feb 18, 2020

@2uasimojo / @johnsom - thanks for the reports, but these are separate issues and would have been better filed as such.

@tirkarthi - thanks for the digging! Sounds like these are all upstream issues and so will be need to be reported and fixed there so they can be backported.

@2uasimojo / @johnsom - please can you report these issues on https://bugs.python.org/, and file each one separately. Feel free to ping the bpo urls here when you have them, and and PRs, so that I can get a new backport release out as soon as the fixes land on cpython master.

@cjw296 cjw296 closed this as completed Feb 18, 2020
@gibizer
Copy link

gibizer commented Jun 26, 2020

I think the original issue reported here ('ValueError: Sentinels must not start with _') caused by the violation of the getattr protocol in oslo utils [1]. I filed a bug in oslo utils [2] and bushed a fix [3].

[1] https://github.com/openstack/oslo.utils/blob/b9938230f992935e8332b6e288937be890724cd2/oslo_utils/fixture.py#L82
[2] https://bugs.launchpad.net/oslo.utils/+bug/1885281
[3] https://review.opendev.org/#/c/738207/

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

No branches or pull requests

5 participants