On Tue, Feb 13, 2018 at 05:48:32PM -0500, Assaf Muller <[email protected]> wrote:
> On Wed, Dec 13, 2017 at 7:30 AM, Michel Peterson <[email protected]> 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. > > > > 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. > > An interesting exercise would be to add 'raise ValueError' type > exceptions in various ODL ML2 mech driver flows and seeing which tests > fail. Basically, if a test passes without the ODL mech driver loaded, > or with a faulty ODL mech driver, then you don't need to run the test > for networking-odl changes. I'd be hesitant to remove all tests > though, it's a good investment of time to figure out which tests are > valuable to you. Mike and Michel should raise it at the PTG for discussion. I know Mike will attend. thanks, -- Isaku Yamahata <[email protected]> __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
