Change in vdsm[ovirt-3.5]: tests: clear libvirtconnection cache

Hello Nir Soffer, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/39948

to review the following change.

Change subject: tests: clear libvirtconnection cache
......................................................................

tests: clear libvirtconnection cache

the libvirtconnection has a cache to reuse connections,
and this caching doesn't play well with the monkeypatching
which we do in tests.

In libvirtconnectionTests, we create fake libvirt objects,
most notably fake virConnect-s. Unfortunately, the test fails
to clean up properly the libvirtconnection cache.

So, the test runs just fine in isolation, but if run in batch
like in 'make check', later tests which expect the real object
become broken.

The failure presents itself like the following:

AttributeError: 'virConnect' object has no attribute 'getMemoryStats'

This patch fixes this error adding a facility to clear
the libvirtconnection cache, and clearing it after each
test.

Change-Id: If4d64920e5f26c276accf26b7a532461d04f02df
Signed-off-by: Francesco Romani 
Reviewed-on: http://gerrit.ovirt.org/37747
Reviewed-by: Nir Soffer 
Reviewed-by: Dan Kenigsberg 
---
M lib/vdsm/libvirtconnection.py
M tests/libvirtconnectionTests.py
2 files changed, 12 insertions(+), 0 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/48/39948/1

diff --git a/lib/vdsm/libvirtconnection.py b/lib/vdsm/libvirtconnection.py
index b1332db..33fdf11 100644
--- a/lib/vdsm/libvirtconnection.py
+++ b/lib/vdsm/libvirtconnection.py
@@ -95,6 +95,14 @@
     return utils.retry(libvirtOpen, timeout=10, sleep=0.2)

+def _clear():
+    """
+    For clearing connections during the tests.
+    """
+    with __connectionLock:
+        __connections.clear()
+
+
 def get(target=None, killOnFailure=True):
     """Return current connection to libvirt or open a new one.
     Use target to get/create the connection object linked to that object.
diff --git a/tests/libvirtconnectionTests.py b/tests/libvirtconnectionTests.py
index 1ac60a0..18e1984 100644
--- a/tests/libvirtconnectionTests.py
+++ b/tests/libvirtconnectionTests.py
@@ -103,6 +103,10 @@

class testLibvirtconnection(TestCaseBase):
+
+    def tearDown(self):
+        libvirtconnection._clear()
+
     @MonkeyPatch(libvirtconnection, 'libvirt', LibvirtMock())
     def testCallSucceeded(self):
         """Positive test - libvirtMock does not raise any errors"""

[e-mail] has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 1: Verified-1

* Update tracker::IGNORE, no Bug-Url found

* Check Bug-Url::ERROR, At least one bug-url is required for the stable branch
* Check merged to previous::OK, change not open on any previous branch

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 1: Code-Review+1

[e-mail] has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found

* Check Bug-Url::ERROR, At least one bug-url is required for the stable branch
* Check merged to previous::OK, change not open on any previous branch

[e-mail] has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 3:

* Update tracker::#1267851::OK
* Check Bug-Url::OK
* Check Public Bug::#1267851::OK, public bug
* Check Product::#1267851::OK, Correct classification oVirt
* Check TR::#1267851::ERROR, wrong target release for stable branch, --- should match ^3.[54321].*
* Check merged to previous::OK, change not open on any previous branch

Francesco Romani has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 3: Verified+1

tests passes, and this code will be used only in tests: V+1

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 3: Code-Review+1

Francesco Romani has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 3: Continuous-Integration+1

run CI checks manually

[e-mail] has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 4:

* Update tracker::#1267851::OK
* Check Bug-Url::OK
* Check Public Bug::#1267851::OK, public bug
* Check Product::#1267851::OK, Correct classification oVirt
* Check TR::#1267851::ERROR, wrong target release for stable branch, --- should match ^3.[54321].*
* Check merged to previous::OK, change not open on any previous branch

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 4: Code-Review+2

Yaniv Bronhaim has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 4: Continuous-Integration+1

Yaniv Bronhaim has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 5: Continuous-Integration+1

Yaniv Bronhaim has submitted this change and it was merged.

Change subject: tests: clear libvirtconnection cache
......................................................................

tests: clear libvirtconnection cache

the libvirtconnection has a cache to reuse connections,
and this caching doesn't play well with the monkeypatching
which we do in tests.

In libvirtconnectionTests, we create fake libvirt objects,
most notably fake virConnect-s. Unfortunately, the test fails
to clean up properly the libvirtconnection cache.

So, the test runs just fine in isolation, but if run in batch
like in 'make check', later tests which expect the real object
become broken.

The failure presents itself like the following:

AttributeError: 'virConnect' object has no attribute 'getMemoryStats'

This patch fixes this error adding a facility to clear
the libvirtconnection cache, and clearing it after each
test.

Change-Id: If4d64920e5f26c276accf26b7a532461d04f02df
Signed-off-by: Francesco Romani 
Bug-Url: https://bugzilla.redhat.com/1267851
Reviewed-on: http://gerrit.ovirt.org/37747
Reviewed-by: Nir Soffer 
Reviewed-by: Dan Kenigsberg 
Reviewed-on: https://gerrit.ovirt.org/39948
Continuous-Integration: Yaniv Bronhaim 
---
M lib/vdsm/libvirtconnection.py
M tests/libvirtconnectionTests.py
2 files changed, 12 insertions(+), 0 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, but someone else must approve
  Yaniv Bronhaim: Passed CI tests
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Verified

[e-mail] has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 5:

* Update tracker::#1267851::OK
* Check Bug-Url::OK
* Check Public Bug::#1267851::OK, public bug
* Check Product::#1267851::OK, Correct classification oVirt
* Check TM::#1267851::OK, correct target milestone ovirt-3.5.7
* Check merged to previous::OK, change not open on any previous branch

[e-mail] has posted comments on this change.

Change subject: tests: clear libvirtconnection cache
......................................................................

Patch Set 6:

* Update tracker::#1267851::OK
* Set MODIFIED::bug 1267851::::#1267851::::IGNORE, not oVirt prod but vdsm