[openstack-dev] [neutron] Generalized issues in the unit testing of ML2 mechanism drivers

Michel Peterson michel at redhat.com
Wed Dec 13 12:30:15 UTC 2017


Through my work in networking-odl I've found what I believe is an issue
present in a majority of ML2 drivers. An issue I think needs awareness so
each project can decide a course of action.

The issue stems from the adopted practice of importing
`neutron.tests.unit.plugins.ml2.test_plugin` and creating classes with noop
operation to "inherit" tests for free [1]. The idea behind is nice, you
inherit >600 tests that cover several scenarios.

There are several issues of adopting this pattern, two of which are
paramount:

1. If the mechanism driver is not loaded correctly [2], the tests then
don't test the mechanism driver but still succeed and therefore there is no
indication that there is something wrong with the code. In the case of
networking-odl it wasn't discovered until last week, which means that for
>1 year it this was adding PASSed tests uselessly.

2. It gives a false sense of reassurance. If the code of those tests is
analyzed it's possible to see that the code itself is mostly centered
around testing the REST endpoint of neutron than actually testing that the
mechanism succeeds on the operation it was supposed to test. As a result of
this, there is marginally added value on having those tests. To be clear,
the hooks for the respective operations are called on the mechanism driver,
but the result of the operation is not asserted.

I would love to hear more voices around this, so feel free to comment.

Regarding networking-odl the solution I propose is the following:
  **First**, discard completely the change mentioned in the footnote #2.
  **Second**, create a patch that completely removes the tests that follow
this pattern.
  **Third**, incorporate the neutron tempest plugin into the CI and rely on
that for assuring coverage of the different scenarios.

Also to mention that when discovered this issue in networking-odl we took a
decision not to merge more patches until the PS of footnote #2 was
addressed. I think we can now decide to overrule that decision and proceed
as usual.



[1]: http://codesearch.openstack.org/?q=class%20.*\(.*TestMl2
[2]: something that was happening in networking-odl and addressed by
https://review.openstack.org/#/c/523934
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20171213/b518a24b/attachment.html>


More information about the OpenStack-dev mailing list