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

Isaku Yamahata isaku.yamahata at gmail.com
Mon Jan 15 16:07:52 UTC 2018


Hello. Any comments/thoughts?

This issue is generic to neutron. not specific to networking-odl.
So this should be addressed as Neutron wide, and neutron and sub-project should
be addressed uniformly.

Thanks,

On Mon, Dec 18, 2017 at 12:52:37PM +0200,
Mike Kolesnik <mkolesni at redhat.com> wrote:

> 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

> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


-- 
Isaku Yamahata <isaku.yamahata at gmail.com>



More information about the OpenStack-dev mailing list