Run unit tests against all plugins

Bug #1040277 reported by Salvatore Orlando
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Fix Released
High
Salvatore Orlando

Bug Description

As testified by bug 1039393 we need to make sure changes both in the API layer or in the plugins do not break the plugins.
To do so, we should run unit tests against every plugin, at least those unit tests which actually invoke code on the plugin, such as test_db_plugin.

Ensuring that both tox and run_tests cover all plugins is therefore quite important at this stage, especially with several patches targeting the plugin now that we are in the rc timeframe.

As I reviewed, and approved the patch which caused the aforementioned bug I will gladly accept to stand in front of a firing squad.
Well, actually we can come to a sort of agreement... what if I assign this bug to myself.

Revision history for this message
Sumit Naiksatam (snaiksat) wrote :

As for the Cisco plugin we are running test_db_plugin tests but we needed to override the setup and teardown to perform some plugin-specific configuration (and also a couple of other testss). So we have a test suite which derives almost completely from the test_db_plugin, but that derived test class needs to be used. Does that help to satisfy this requirement?

Revision history for this message
dan wendlandt (danwent) wrote :

btw, there's at least one related bug already filed for OVS (https://bugs.launchpad.net/quantum/+bug/1029142), and I think one for RYU. I've also talked to the Cisco team about doing this.

Your timing is good though, as I had just earlier today been thinking if we needed to bite the bullet and do this in Folsom (I had previously been thinking that this is something we would tackle in Grizzly). I think at the least the OVS + LB plugin tests should be moved over in Folsom, as those are the most widely used and supported open source plugins. I

Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

Dan,
is your comment complete... looks like you dropped in the middle of a sentence!

BTW I checked bug 1029142 before submitting this, and it looked like that bug is for bringing plugin specific tests into the main test dir so that tox can run them.

For the reasons mentioned by Sumit, the cisco plugin is not running at all the "core" unit tests, but instead has its own test suite.

I have tried an approach based on tweaking run_tests.sh for executing multiple runs of core unit tests against different plugin. It worked with all plugins bar cisco & metaplugin. This approach works (can push the branch for your perusal) but:
1) runs plugin-independent test for every plugin, and we don't want to waste time
2) cannot be leverage by the gating mechanism which uses tox.

I am now trying a simpler solution - just inheriting from test_db_plugin and adding plugin-specific code in setup and tearDown where applicable. This should work with tox which just invokes nosetest.

Revision history for this message
dan wendlandt (danwent) wrote :

Ignore the "I". I had an extra sentence there, but deleted it, missing the I.

Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

np.

This is the code that allows to run unit tests for multiple plugins tweaking run_tests. For the aforementioned reasons, I don't think it can be submitted for review, as it will be pretty much useless for tox.
https://github.com/salv-orlando/quantum/tree/multi_tests

Anyway, check it out and let me know if there's something you'd like from it.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to quantum (master)

Fix proposed to branch: master
Review: https://review.openstack.org/11829

Changed in quantum:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to quantum (master)

Reviewed: https://review.openstack.org/11829
Committed: http://github.com/openstack/quantum/commit/9548defc4f56ed40f2bea3a04b1a74cc8b7e89b2
Submitter: Jenkins
Branch: master

commit 9548defc4f56ed40f2bea3a04b1a74cc8b7e89b2
Author: Salvatore Orlando <email address hidden>
Date: Wed Aug 22 16:29:24 2012 -0700

    Run core unit tests for each plugin

    Fixes bug 1040277

    Execute the test cases in test_db_plugin.py for each plugin.
    Each plugin now get its own test_<plugin_name>_plugin test module.
    This commit excludes cisco and metaplugin plugins, which do not run
    the "core" unit tests at all.

    Change-Id: I137411f47d8cfcc753aa3a88359f5b1590ca8237

Changed in quantum:
status: In Progress → Fix Committed
Revision history for this message
dan wendlandt (danwent) wrote :

I think this is a good start, but for the most part, it seems like for the most part this results in re-running the same tests multiple times over code that is for the most part shared across all plugins via inheritance based on the db_base_plugin_v2.

I think its also important to get the plugin-specific unit test code currently under each plugin's directory to also be run every time the main unit tests are run. For example, the tests in quantum/plugins/openvswitch/tests .

Do we want to file a separate bug to do this work? Or just re-use this one?

Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

This bug fix add coverage for the plugin classes; we need to add coverage for other plugin modules such as the agents.
I would file per plugin bugs (I think we already have bugs for OVS and metaplugin).

Revision history for this message
Sumit Naiksatam (snaiksat) wrote :

I believe one of the reasons we were not running plugin specific tests earlier was because we were not including plugin-specific package dependencies in test-requires. How are we planning to get around this? Should plugin-specific packages be included in the quantum tools/test-requires?

Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

Thanks for raising the point about test-requires.
Since we now want to cover all plugins with unit tests, it makes sense to have all packages in test-requires, even plugin specific ones.

This won't apply to pip-requires, as one wouldn't want to install packages required by a plugin he/she does not use, but this is probably another story.

Revision history for this message
dan wendlandt (danwent) wrote :

In general, we can get a lot of mileage out of using mock/mox to remove the dependence on outside libraries. At the same time, the "cost" of requiring a package for test-requires is lower that pip-requires, assuming we do not expect unit tests to work with binaries shipped by distros.

I guess this is something we'll have to get more experience with. For now I think its ok to put items into test-requires that are plugin specific, as long as they are easily installed via pip. We should probably identify the source of the requirement using a comment though, in case we decide to reverse this direction in the future.

Thierry Carrez (ttx)
Changed in quantum:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in quantum:
milestone: folsom-rc1 → 2012.2
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.