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