Added checks for mocking best practices #27
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
mocking
checker[back to top]
R8918
:explicit-dependency-required
Obscure implicit test dependency with mock.patch(XXX). Rewrite to inject dependencies through constructor.. Using
patch
to mock dependencies in unit tests can introduce implicitdependencies within a class, making it unclear to other developers. Constructor arguments, on the other hand,
explicitly declare dependencies, enhancing code readability and maintainability. However, reliance on
patch
for testing may lead to issues during refactoring, as updates to underlying implementations would necessitate
changes across multiple unrelated unit tests. Moreover, the use of hard-coded strings in
patch
can obscurewhich unit tests require modification, as they lack strongly typed references. This coupling of the class
under test to concrete classes signifies a code smell, and such code is not easily portable to statically typed
languages where monkey patching isn't feasible without significant effort. In essence, extensive patching of
external clients suggests a need for refactoring, with experienced engineers recognizing the potential for
dependency inversion in such scenarios.
To address this issue, refactor the code to inject dependencies through the constructor. This approach
explicitly declares dependencies, enhancing code readability and maintainability. Moreover, it allows for
dependency inversion, enabling the use of interfaces to decouple the class under test from concrete classes.
This decoupling facilitates unit testing, as it allows for the substitution of mock objects for concrete
implementations, ensuring that the class under test behaves as expected. By following this approach, you can
create more robust and maintainable unit tests, improving the overall quality of your codebase.
Use
require-explicit-dependency
option to specify the package names that contain code for your project.[back to top]
R8919
:obscure-mock
Obscure implicit test dependency with MagicMock(). Rewrite with create_autospec(ConcreteType).. Using
MagicMock
to mock dependencies in unit tests can introduce implicit dependencieswithin a class, making it unclear to other developers. create_autospec(ConcreteType) is a better alternative, as it
automatically creates a mock object with the same attributes and methods as the concrete class. This
approach ensures that the mock object behaves like the concrete class, allowing for more robust and
maintainable unit tests. Moreover, reliance on
MagicMock
for testing leads to issues during refactoring,as updates to underlying implementations would necessitate changes across multiple unrelated unit tests.
[back to top]