On 08/11/2014 02:35 PM, Crístian Viana wrote:

Hi!

After creating a few patches for Kimchi, it occurred to me that the mockmodel design has more cons than pros. So I started wondering why we still use it.

I understand - and totally agree - that we should have an extensive test infrastructure to make sure the code runs as designed and to make it very easy to find regression bugs on every new feature we implement. And as Kimchi works very integrated to its host system, it may not be easy to create expected/failed situations for things we don't always have control.

But for every new feature we implement, we currently have to:

1) Implement the feature itself 2) Create tests for (1) 3) Implement the feature in a mock environment 4) Create tests for (3)

And by looking at some existing code, many of the mock tests don't actually test anything. They're just there so we can say we have a mock implementation and a test for it. Why create a mock function and a test for the mock function if what we need is to test the real code? And a lot of times, the mock implementation (and the tests) are just copy/paste of the original feature. For me, that seems very tedious and useless.

Why don't we get rid of the mockmodel and keep only the "real" model? I understand that it's not so simple to remove it right away, that's why I'm starting this discussion so we could see how to do it better - if we're actually going to do it. What would we miss if we didn't have the mockmodel anymore? I believe we'll need to be more careful when writing tests which run on a real environment (i.e. we can't "leak" VMs, networks, templates, or anything else) but at least we'll always test real code and we won't be writing code that won't be used.

Please share your opinion with us.

Best regards, Crístian.

Hi Cristian, Sorry about the time but let me clarify some things. The mockmodel exists to make the tests easier but also to allow user run Kimchi without affecting it system (with --test option). It permits user uses Kimchi without the fear of breaking something on system. But I completely agree with you that what we have today is not the best solution for it. Because that I proposed a mockmodel refactor for the next release. ---- The mockmodel was designed to provide a way to user tests Kimchi without affecting the system (by "kimchid --test") and also to make the tests easier to do. But in fact, we have a bunch of code that completely differs from the real model, so the developer needs to do 2 kinds of implementations while developing a new feature. My proposal is simplify that by using Model() with "test:///default" URI. This configuration does not affect the system in real and also can provide us a way to test the new feature in a closer scenario to the real one: by connecting to libvirt and doing the operations. As you may know "test:///default" driver has some limitations that prevents some operations, for example, screenshots. To solve it we could only override what is not supported by this driver and create a simpler mockmodel. Example: class MockModel(Model): super(Model, self).__init__("test:///default") def vmscreenshot_lookup(): # do some mock code I think in this way we will have a significant reduction in our mockmodel code. ---- I think with this refactoring we can make mockmodel more devel friendly and avoid a lot of duplicated code.