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 <mkole...@redhat.com> wrote: > On Wed, Dec 13, 2017 at 2:30 PM, Michel Peterson <mic...@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-...@lists.opendaylight.org > > https://lists.opendaylight.org/mailman/listinfo/neutron-dev > > > > > > > -- > Regards, > Mike > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Isaku Yamahata <isaku.yamah...@gmail.com> __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev