On 02/14/2014 01:57 PM, Chris Evich wrote:
On 02/07/2014 05:59 PM, Lucas Meneghel Rodrigues wrote:
On 02/05/2014 07:19 PM, Lucas Meneghel Rodrigues wrote:
On 02/05/2014 03:07 PM, Lucas Meneghel Rodrigues wrote:
So, after a while thinking, I made the simplest change that touched only
in the biggest offender, virttest.virsh, while not breaking any current
code and still letting people to create virsh tests without 2 PRs
(hopefully). You can see it here:

https://github.com/autotest/tp-libvirt/pull/12

(not meaning to sound defensive) Every module is the 'offender' with
circular dependencies, that's why they're circular!

Consider the term 'biggest offender' to mean only 'the module people will touch with 99% certainty when writing a new virsh test, that will trigger the need to write 2 PRs instead of one.

In my experience, these issues are nearly always some low-level code
trying to use a higher level interface (i.e. a down-cast)**.  Fixing
these permanently involves identifying those usages and correcting them
to either be independent or get promoted to a higher-level.

And to add new virsh functions, you just add them to

provider/virsh.py

This will cause an even worse problem!  If _anything_ other than 'from
virttest.virsh import *' (eek!) exists in that module,

It may not be pretty, but it was the least intrusive yet functional fix I could find. Believe me, I spent hours moving APIs, only to find out I left out something and broke virt-test.

  it will 'fork'
the Virsh() class namespace.  Before anything can be added to
provider/virsh.py...

As long as tests use only provider.virsh, that is fine.

In tp-libvirt. Of couse, at some point we need to actually move the 150
functions that are not necessary by core virt-test functionality, but we

...the Virsh() class (and anything depending on it) _must_ be promoted
out of virttest.  That implies any virttest modules using the Virsh()
class interface, must be 'adjusted' to either always use the module
functions, or also get promoted to the test provider namespace.

If not, Virsh() will have undefined/ambiguous behavior!
i.e. [low-leve]l Virsh() != Virsh() [high-level]  (and the instance user
won't know the difference!)

provider.virsh works one way, and it is supposed to be used by tests. virttest.virsh should only be used by other virttest internal modules. Again, it is not pretty, but having to work out a change with reduced time did not work for me.

Now, I'm sure we can make something a lot better now. Let's do this.

_______________________________________________
Virt-test-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/virt-test-devel

Reply via email to