Open Stack

On Wed, Dec 13, 2017 at 2:30 PM, Michel Peterson <michel at redhat.com> wrote:

> 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.
>

​i talked to a few guys from networking-ovn which are now processing this
info so they could chime in, but from what I've understood the issue wasn't
given much thought in networking-ovn (and I suspect other mechanism
drivers).
​

>
> 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.
>

​This sounds like a good plan to me.
​

>
> 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.
>

​Agreed.
​

>
>
>
> [1]: http://codesearch.openstack.org/?q=class%20.*\(.*TestMl2
> <http://codesearch.openstack.org/?q=class%20.*%5C(.*TestMl2>
> [2]: something that was happening in networking-odl and addressed by
> https://review.openstack.org/#/c/523934
>
> _______________________________________________
> neutron-dev mailing list
> neutron-dev at lists.opendaylight.org
> https://lists.opendaylight.org/mailman/listinfo/neutron-dev
>
>


-- 
Regards,
Mike
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20171218/55a9980c/attachment.html>