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

Make mock library used configurable #49

Closed

Conversation

stephenfin
Copy link
Collaborator

@stephenfin stephenfin commented Nov 11, 2021

mock.Mock != unittest.mock.Mock. This can make some tests in consuming projects more complicated that necessary while they switch from one to the other.

This is intended to make life easier in OpenStack nova, as we pivot from mock to unittest.mock.


This change is Reviewable

mock.Mock != unittest.mock.Mock. This can make some tests in consuming
projects more complicated that necessary while they switch from one to
the other.

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
@jelmer
Copy link
Member

jelmer commented Jan 10, 2022

Is there a good reason to support both mock libraries rather than supporting one (and perhaps moving to the unittest one if that is better for some reason)?

At the very least, this makes testing fixtures itself harder as we need to make sure we don't break with either mock library.

@stephenfin
Copy link
Collaborator Author

stephenfin commented Jan 28, 2022

Apologies for the delayed response. mock is a rolling backport of unittest.mock so it does tend to have useful backports from newer Python versions that people can use/require. However, using unittest.mock means one less dependency. I can understand why people would use either.

Concrete details might be helpful in making my case. In OpenStack land, most projects have switched to unittest.mock and I'm trying to remove mock as a dependency for nova. We have a fixture to poison some platform-dependent libraries and ensure they're correctly mocked (to keep things deterministic). This fixtures attempts to avoid double-mocking by checking if thing is already an instance of mock.Mock. I could change this to use unittest.mock.Mock but because fixtures opportunistically uses mock, I'd also have to check for mock.Mock (because mock.Mock != unittest.mock.Mock) on the off-chance that any of our many dependencies is bringing that library in. This would be weird since, in theory, we're not using mock (it would no longer be present in requirements.txt) and I'd have to add except ImportError logic.

Optionally configuring fixtures to use the mock library we want it to use seems better than removing people's ability to use the mock library entirely or preventing them switching to unittest.mock like we're attempting to do. Also, changing this logic now would break people that are relying on the type of Mock object used by fixtures, like nova does in that fixture. I don't think testing is a concern since mock is an exact backport from newer Python versions, so by simply testing with stdlib unittest.mock on multiple versions, we're effectively testing multiple mock versions.

@jelmer
Copy link
Member

jelmer commented Jan 28, 2022

I'm not a big of using environment variables here - especially as it's really up to the code that calls these functions what mock library is necessary, it's not actually dependent on the environment; could we perhaps just allow the caller to pass in mock.default and/or mock.patch?

@stephenfin
Copy link
Collaborator Author

Fair point. Neither am I. If I pass the mock library used into the fixture then I need to remember to do it for every invocation of the fixtures in fixtures/_fixtures/mockpatch.py. I'd like to avoid that if possible.

How about a global variable that the caller has to configure as part of their test suite set up, e.g.

import fixtures

fixtures.default_mock_library = 'mock'

Still kind of ugly but better than updating every. single. instance. of fixtures.Mock*.

@jelmer
Copy link
Member

jelmer commented Jan 28, 2022

Couldn't you effectively do that today by just assigning a mock package to fixtures.mock ?

@stephenfin
Copy link
Collaborator Author

Oh... 🤦

@stephenfin
Copy link
Collaborator Author

Yeah, that'll work. I'll do that. Sorry for the noise! 🙈 😅

@stephenfin stephenfin closed this Jan 28, 2022
@stephenfin stephenfin deleted the configurable-mock-lib branch January 28, 2022 17:48
@stephenfin stephenfin restored the configurable-mock-lib branch January 28, 2022 18:53
@stephenfin
Copy link
Collaborator Author

Nope, that won't work because of this bad boy. Because new=mock_default is "configured" at import time, even if I monkey patch the mock_default attribute it'll still use mock.DEFAULT rather than unittest.mock.DEFAULT. We need to catch things beforehand and just outright avoid importing mock if not needed, so I suspect it's this or I've to start messing with sys.modules before importing fixtures for the first time (which I really want to avoid).

@stephenfin stephenfin reopened this Jan 28, 2022
@jelmer
Copy link
Member

jelmer commented Jan 28, 2022

What something like #52 ?

@stephenfin stephenfin closed this Jan 29, 2022
@stephenfin stephenfin deleted the configurable-mock-lib branch January 29, 2022 21:12
openstack-mirroring pushed a commit to openstack/openstack that referenced this pull request Aug 1, 2022
* Update nova from branch 'master'
  to 89ef050b8c049b9a6f0e2c70408fc93c826c55e0
  - Use unittest.mock instead of third party mock
    
    Now that we no longer support py27, we can use the standard library
    unittest.mock module instead of the third party mock lib. Most of this
    is autogenerated, as described below, but there is one manual change
    necessary:
    
    nova/tests/functional/regressions/test_bug_1781286.py
      We need to avoid using 'fixtures.MockPatch' since fixtures is using
      'mock' (the library) under the hood and a call to 'mock.patch.stop'
      found in that test will now "stop" mocks from the wrong library. We
      have discussed making this configurable but the option proposed isn't
      that pretty [1] so this is better.
    
    The remainder was auto-generated with the following (hacky) script, with
    one or two manual tweaks after the fact:
    
      import glob
    
      for path in glob.glob('nova/tests/**/*.py', recursive=True):
          with open(path) as fh:
              lines = fh.readlines()
          if 'import mock\n' not in lines:
              continue
          import_group_found = False
          create_first_party_group = False
          for num, line in enumerate(lines):
              line = line.strip()
              if line.startswith('import ') or line.startswith('from '):
                  tokens = line.split()
                  for lib in (
                      'ddt', 'six', 'webob', 'fixtures', 'testtools'
                      'neutron', 'cinder', 'ironic', 'keystone', 'oslo',
                  ):
                      if lib in tokens[1]:
                          create_first_party_group = True
                          break
                  if create_first_party_group:
                      break
                  import_group_found = True
              if not import_group_found:
                  continue
              if line.startswith('import ') or line.startswith('from '):
                  tokens = line.split()
                  if tokens[1] > 'unittest':
                      break
                  elif tokens[1] == 'unittest' and (
                      len(tokens) == 2 or tokens[4] > 'mock'
                  ):
                      break
              elif not line:
                  break
          if create_first_party_group:
              lines.insert(num, 'from unittest import mock\n\n')
          else:
              lines.insert(num, 'from unittest import mock\n')
          del lines[lines.index('import mock\n')]
          with open(path, 'w+') as fh:
              fh.writelines(lines)
    
    Note that we cannot remove mock from our requirements files yet due to
    importing pypowervm unit test code in nova unit tests. This library
    still uses the mock lib, and since we are importing test code and that
    lib (correctly) only declares mock in its test-requirements.txt, mock
    would not otherwise be installed and would cause errors while loading
    nova unit test code.
    
    [1] testing-cabal/fixtures#49
    
    Change-Id: Id5b04cf2f6ca24af8e366d23f15cf0e5cac8e1cc
    Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
openstack-mirroring pushed a commit to openstack/nova that referenced this pull request Aug 1, 2022
Now that we no longer support py27, we can use the standard library
unittest.mock module instead of the third party mock lib. Most of this
is autogenerated, as described below, but there is one manual change
necessary:

nova/tests/functional/regressions/test_bug_1781286.py
  We need to avoid using 'fixtures.MockPatch' since fixtures is using
  'mock' (the library) under the hood and a call to 'mock.patch.stop'
  found in that test will now "stop" mocks from the wrong library. We
  have discussed making this configurable but the option proposed isn't
  that pretty [1] so this is better.

The remainder was auto-generated with the following (hacky) script, with
one or two manual tweaks after the fact:

  import glob

  for path in glob.glob('nova/tests/**/*.py', recursive=True):
      with open(path) as fh:
          lines = fh.readlines()
      if 'import mock\n' not in lines:
          continue
      import_group_found = False
      create_first_party_group = False
      for num, line in enumerate(lines):
          line = line.strip()
          if line.startswith('import ') or line.startswith('from '):
              tokens = line.split()
              for lib in (
                  'ddt', 'six', 'webob', 'fixtures', 'testtools'
                  'neutron', 'cinder', 'ironic', 'keystone', 'oslo',
              ):
                  if lib in tokens[1]:
                      create_first_party_group = True
                      break
              if create_first_party_group:
                  break
              import_group_found = True
          if not import_group_found:
              continue
          if line.startswith('import ') or line.startswith('from '):
              tokens = line.split()
              if tokens[1] > 'unittest':
                  break
              elif tokens[1] == 'unittest' and (
                  len(tokens) == 2 or tokens[4] > 'mock'
              ):
                  break
          elif not line:
              break
      if create_first_party_group:
          lines.insert(num, 'from unittest import mock\n\n')
      else:
          lines.insert(num, 'from unittest import mock\n')
      del lines[lines.index('import mock\n')]
      with open(path, 'w+') as fh:
          fh.writelines(lines)

Note that we cannot remove mock from our requirements files yet due to
importing pypowervm unit test code in nova unit tests. This library
still uses the mock lib, and since we are importing test code and that
lib (correctly) only declares mock in its test-requirements.txt, mock
would not otherwise be installed and would cause errors while loading
nova unit test code.

[1] testing-cabal/fixtures#49

Change-Id: Id5b04cf2f6ca24af8e366d23f15cf0e5cac8e1cc
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
AnishReddyRavula pushed a commit to ChameleonCloud/nova that referenced this pull request Jan 17, 2024
Now that we no longer support py27, we can use the standard library
unittest.mock module instead of the third party mock lib. Most of this
is autogenerated, as described below, but there is one manual change
necessary:

nova/tests/functional/regressions/test_bug_1781286.py
  We need to avoid using 'fixtures.MockPatch' since fixtures is using
  'mock' (the library) under the hood and a call to 'mock.patch.stop'
  found in that test will now "stop" mocks from the wrong library. We
  have discussed making this configurable but the option proposed isn't
  that pretty [1] so this is better.

The remainder was auto-generated with the following (hacky) script, with
one or two manual tweaks after the fact:

  import glob

  for path in glob.glob('nova/tests/**/*.py', recursive=True):
      with open(path) as fh:
          lines = fh.readlines()
      if 'import mock\n' not in lines:
          continue
      import_group_found = False
      create_first_party_group = False
      for num, line in enumerate(lines):
          line = line.strip()
          if line.startswith('import ') or line.startswith('from '):
              tokens = line.split()
              for lib in (
                  'ddt', 'six', 'webob', 'fixtures', 'testtools'
                  'neutron', 'cinder', 'ironic', 'keystone', 'oslo',
              ):
                  if lib in tokens[1]:
                      create_first_party_group = True
                      break
              if create_first_party_group:
                  break
              import_group_found = True
          if not import_group_found:
              continue
          if line.startswith('import ') or line.startswith('from '):
              tokens = line.split()
              if tokens[1] > 'unittest':
                  break
              elif tokens[1] == 'unittest' and (
                  len(tokens) == 2 or tokens[4] > 'mock'
              ):
                  break
          elif not line:
              break
      if create_first_party_group:
          lines.insert(num, 'from unittest import mock\n\n')
      else:
          lines.insert(num, 'from unittest import mock\n')
      del lines[lines.index('import mock\n')]
      with open(path, 'w+') as fh:
          fh.writelines(lines)

Note that we cannot remove mock from our requirements files yet due to
importing pypowervm unit test code in nova unit tests. This library
still uses the mock lib, and since we are importing test code and that
lib (correctly) only declares mock in its test-requirements.txt, mock
would not otherwise be installed and would cause errors while loading
nova unit test code.

[1] testing-cabal/fixtures#49

Change-Id: Id5b04cf2f6ca24af8e366d23f15cf0e5cac8e1cc
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
AnishReddyRavula pushed a commit to ChameleonCloud/nova that referenced this pull request Jan 17, 2024
Now that we no longer support py27, we can use the standard library
unittest.mock module instead of the third party mock lib. Most of this
is autogenerated, as described below, but there is one manual change
necessary:

nova/tests/functional/regressions/test_bug_1781286.py
  We need to avoid using 'fixtures.MockPatch' since fixtures is using
  'mock' (the library) under the hood and a call to 'mock.patch.stop'
  found in that test will now "stop" mocks from the wrong library. We
  have discussed making this configurable but the option proposed isn't
  that pretty [1] so this is better.

The remainder was auto-generated with the following (hacky) script, with
one or two manual tweaks after the fact:

  import glob

  for path in glob.glob('nova/tests/**/*.py', recursive=True):
      with open(path) as fh:
          lines = fh.readlines()
      if 'import mock\n' not in lines:
          continue
      import_group_found = False
      create_first_party_group = False
      for num, line in enumerate(lines):
          line = line.strip()
          if line.startswith('import ') or line.startswith('from '):
              tokens = line.split()
              for lib in (
                  'ddt', 'six', 'webob', 'fixtures', 'testtools'
                  'neutron', 'cinder', 'ironic', 'keystone', 'oslo',
              ):
                  if lib in tokens[1]:
                      create_first_party_group = True
                      break
              if create_first_party_group:
                  break
              import_group_found = True
          if not import_group_found:
              continue
          if line.startswith('import ') or line.startswith('from '):
              tokens = line.split()
              if tokens[1] > 'unittest':
                  break
              elif tokens[1] == 'unittest' and (
                  len(tokens) == 2 or tokens[4] > 'mock'
              ):
                  break
          elif not line:
              break
      if create_first_party_group:
          lines.insert(num, 'from unittest import mock\n\n')
      else:
          lines.insert(num, 'from unittest import mock\n')
      del lines[lines.index('import mock\n')]
      with open(path, 'w+') as fh:
          fh.writelines(lines)

Note that we cannot remove mock from our requirements files yet due to
importing pypowervm unit test code in nova unit tests. This library
still uses the mock lib, and since we are importing test code and that
lib (correctly) only declares mock in its test-requirements.txt, mock
would not otherwise be installed and would cause errors while loading
nova unit test code.

[1] testing-cabal/fixtures#49

Change-Id: Id5b04cf2f6ca24af8e366d23f15cf0e5cac8e1cc
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
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

Successfully merging this pull request may close these issues.

None yet

2 participants