Add spec for removal of autodeletion in tests
This spec proposes the removal of autodeletion of Neutron resources in the unit test suite. bp remove-unit-test-autodeletion Change-Id: Ifbf839d442d658ad74c14fdfde3dd685f9bc9775
This commit is contained in:
parent
13ef7afba5
commit
d1066ef07e
189
specs/juno/remove-unit-test-autodeletion.rst
Normal file
189
specs/juno/remove-unit-test-autodeletion.rst
Normal file
@ -0,0 +1,189 @@
|
||||
..
|
||||
This work is licensed under a Creative Commons Attribution 3.0 Unported
|
||||
License.
|
||||
|
||||
http://creativecommons.org/licenses/by/3.0/legalcode
|
||||
|
||||
=====================================================
|
||||
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
|
||||
----------
|
||||
|
||||
1. Add explicit tests for deletion for resource types that have
|
||||
previously relied on autodeletion to perform implicit testing of
|
||||
deletion.
|
||||
|
||||
2. Disable autodeletion in each resource creation method
|
||||
(e.g. NeutronDbPluginV2TestCase.network) in the unit test suite
|
||||
and fix any test failures that result.
|
||||
|
||||
3. Remove support for autodeletion (parameters, deletion calls) from
|
||||
the resource creation methods and update their callers.
|
||||
|
||||
4. 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
|
Loading…
x
Reference in New Issue
Block a user