Remove the use of resource autodeletion in unit tests¶
Launchpad blueprint:
https://blueprints.launchpad.net/neutron/+spec/remove-unit-test-autodeletion
This blueprint describes the rational for removing resource autodeletion in the unit tests.
Problem description¶
The Neutron unit tests currently decorate resource creation methods with contextlib.contextmanager to allow resources to be automatically deleted when they are no longer needed. This strategy is a legacy of a time when this explicit cleanup was required to ensure isolation between tests, but is now unnecessary since db state is automatically thrown away after each test. The strategy has the following negative impacts on much of Neutron’s unit test suite:
an execution penalty, since deletion of every created resource is always done and is performed through the wsgi layer
a debugging penalty, since contextmanagers are generators with the consequent loss of stack context
a readability penalty, since the test code is riddled with unnecessary ‘with’ blocks around resource creation
a coverage penalty, since implicit deletion testing is likely to hide gaps in testing of resource deletion
Proposed change¶
Resource creation methods should be modified to not perform autodeletion, as follows:
remove the contextlib.contextmanager decorator
change ‘yield’ statements to ‘return’
remove the do_delete or no_delete parameter
remove the conditional block that deletes the resource(s)
In addition, all clients of the resource creation methods should be updated to no longer pass a do_delete/no_delete parameter and perform a regular call (e.g. create_resource()) rather than creating a with block (e.g. with create_resource():). It also may make sense to update the names of resource creation methods, since they are often of the form ‘resourcename’ rather than ‘create_resourcename’.
Care will have to be taken to ensure that explicit tests for deletion are added if they do not already exist for a given resource type. Autodeletion ensured that any resource whose creation was tested also had its deletion validated to some extent, but this validation will have to be explicit from now on. This is arguably desirable, since relying on the ‘magic’ of autodeletion to provide test coverage can potentially lull developers into a false sense of security.
There may also be tests that create multiple resources in a way that depended on autodeltion to prevent uniqueness constraint violation. It will be necessary to perform explicit deletion in those cases where removal of autodeletion provokes test failures.
Alternatives¶
The alternative to not removing autodeletion is continuing to live with the penalties it imposes.
Data model impact¶
None
REST API impact¶
None
Security impact¶
None
Notifications impact¶
None
Other end user impact¶
None
Performance Impact¶
Removal of autodeletion should have a positive impact on the execution time of the unit test suite.
Other deployer impact¶
None
Developer impact¶
Developers and reviewers familiar with the current contextmanager-based model of resource creation in unit tests will have to be educated as to the move to simpler creation methods and the need for explicit testing of deletion.
Once concern voiced at the removal of the contextmanager decorator was that it would disallow the convenience of managing the lifecycle of multiple resources via contextlib.nested. If necessary, this capability can be replicated by creating a helper method that accepts a list of creation functions and ensures that the resources created are cleaned up.
Implementation¶
Assignee(s)¶
- Primary assignee:
kevinbenton
- Other contributors:
marun
Work Items¶
Add explicit tests for deletion for resource types that have previously relied on autodeletion to perform implicit testing of deletion.
Disable autodeletion in each resource creation method (e.g. NeutronDbPluginV2TestCase.network) in the unit test suite and fix any test failures that result.
Remove support for autodeletion (parameters, deletion calls) from the resource creation methods and update their callers.
Remove the contextmanager decorator from resource creation methods, rename the creation methods (e.g. network -> create_network), and update their clients.
Dependencies¶
None
Testing¶
None
Documentation Impact¶
None
References¶
None
