On 06/23/2016 10:30 AM, Martin Basti wrote:



On 23.06.2016 06:55, Lenka Doudova wrote:



On 06/22/2016 05:11 PM, Lenka Doudova wrote:



On 06/22/2016 04:37 PM, Lenka Doudova wrote:



On 06/22/2016 08:33 AM, Martin Basti wrote:



On 22.06.2016 07:37, Lenka Doudova wrote:



On 06/21/2016 06:57 PM, Martin Basti wrote:



On 21.06.2016 15:39, Lenka Doudova wrote:
Hi,

attaching patch for failing location tests (ipatests/test_xmlrpc/test_location_plugin.py).

Lenka




Hello,

1)
+ expected_updates={u'ipalocation_location': [location.idnsname_obj],
+ u'enabled_role_servrole': (
+ u'CA server', u'DNS server', u'NTP server')},

This depends on services installed on server, so server without DNS will cause test failures. We probably should skip test id DNS isn't installed.
Without DNS installed you get much more different warnings


2)
+ def update(self, updates, expected_updates=None, messages=None):
....
+        self.messages = messages

Why is this needed? I'm puzzled by this

It is defined outside __init__ what is wrong and it is never used.

Hi, thanks for review.
ad 1: will fix
ad 2: the 'messages' key is indeed used, because this key is returned every time a server is attached to/removed from a location. If the 'messages' is not supplied to the result comparison, the test fails (see https://paste.fedoraproject.org/382936/65732411/ for result of test without applied patch).

Lenka

Please point me to the line where ServerTracker.messages is used, I still don't see it. In your fpaste, removed self.messages in that context is TestLocationsServer.messages not ServerTracker.messages

Martin^2

Ah, right you are, my good sir. I fixed both issues, hopefully will be fine now. Fixed patch attached.
Lenka


Self NACK, will provide some more changes tomorrow.
Lenka


And here it goes...

Lenka


This is really weird

fuzzy_servrole = Fuzzy(
    type=list,
     test=lambda other: False not in set(
        item in (u'CA server', u'DNS server', u'NTP server')
        for item in other)
    and u'DNS server' in other)

1) Why is there special case 'DNS server' in other, IMO general fuzzy object should not enforce DNS server role

2) Several servroles related to AD trust are missing there, so if you are doing general fuzzy object for server roles it should contains all roles

3)
I suggest something like
lambda other: all(item in {u'CA server', u'DNS server', u'NTP server', ...} for item in other)

4) during yesterday's interactive review, I suggested to use just lambda other: True, as serveroles are tested in different test suite, but If you fix issues above, it can stay as it is

Martin^2
Fixed according to suggestion from interactiove review. Attached.
Lenka
From e5e795c1fd1c589c505c3312788ef5663c67af51 Mon Sep 17 00:00:00 2001
From: Lenka Doudova <ldoud...@redhat.com>
Date: Tue, 21 Jun 2016 15:29:46 +0200
Subject: [PATCH] Tests: Fix for failing location tests

---
 ipatests/test_xmlrpc/test_location_plugin.py    | 54 ++++++++++++++++---------
 ipatests/test_xmlrpc/tracker/location_plugin.py | 10 ++++-
 ipatests/test_xmlrpc/tracker/server_plugin.py   | 49 +++++++++++++++++++---
 3 files changed, 87 insertions(+), 26 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_location_plugin.py b/ipatests/test_xmlrpc/test_location_plugin.py
index 3f0edfbcf791feea1a9e1b584386215d84b44606..a7d8098c9bcccd13c99e549591563c86b5c59848 100644
--- a/ipatests/test_xmlrpc/test_location_plugin.py
+++ b/ipatests/test_xmlrpc/test_location_plugin.py
@@ -10,7 +10,7 @@ from ipatests.test_xmlrpc.tracker.location_plugin import LocationTracker
 from ipatests.test_xmlrpc.tracker.server_plugin import ServerTracker
 from ipatests.test_xmlrpc.xmlrpc_test import (
     XMLRPC_test,
-    raises_exact,
+    raises_exact
 )
 from ipapython.dnsutil import DNSName
 
@@ -30,13 +30,13 @@ def location_invalid(request):
 @pytest.fixture(scope='class')
 def location_absolute(request):
     tracker = LocationTracker(u'invalid.absolute.')
-    return tracker.make_fixture(request)
+    return tracker
 
 
 @pytest.fixture(scope='class')
 def server(request):
     tracker = ServerTracker(api.env.host)
-    return tracker
+    return tracker.make_fixture_clean_location(request)
 
 
 @pytest.mark.tier1
@@ -122,7 +122,18 @@ class TestCRUD(XMLRPC_test):
 
 
 @pytest.mark.tier1
+@pytest.mark.skipif(
+    not api.Command.dns_is_enabled()['result'], reason='DNS not configured')
 class TestLocationsServer(XMLRPC_test):
+    messages = [{
+        u'data': {u'service': u'named-pkcs11.service',
+                  u'server': u'%s' % api.env.host},
+        u'message': (u'Service named-pkcs11.service requires restart '
+                     u'on IPA server %s to apply configuration '
+                     u'changes.' % api.env.host),
+        u'code': 13025,
+        u'type': u'warning',
+        u'name': u'ServiceRestartRequired'}]
 
     def test_add_nonexistent_location_to_server(self, server):
         nonexistent_loc = DNSName(u'nonexistent-location')
@@ -140,13 +151,13 @@ class TestLocationsServer(XMLRPC_test):
     def test_add_location_to_server(self, location, server):
         location.ensure_exists()
         server.update(
-            dict(ipalocation_location=location.idnsname_obj),
-            expected_updates=dict(
-                ipalocation_location=[location.idnsname_obj],
-            )
-        )
+            updates={u'ipalocation_location': location.idnsname_obj},
+            expected_updates={u'ipalocation_location': [location.idnsname_obj],
+                              u'enabled_role_servrole': lambda other: True},
+            messages=self.messages)
         location.add_server_to_location(server.server_name)
         location.retrieve()
+        location.remove_server_from_location(server.server_name)
 
     def test_retrieve(self, server):
         server.retrieve()
@@ -174,16 +185,15 @@ class TestLocationsServer(XMLRPC_test):
 
     def test_add_location_to_server_custom_weight(self, location, server):
         location.ensure_exists()
+
         server.update(
-            dict(
-                ipalocation_location=location.idnsname_obj,
-                ipaserviceweight=200,
-            ),
-            expected_updates=dict(
-                ipalocation_location=[location.idnsname_obj],
-                ipaserviceweight=[u'200'],
-            )
-        )
+            updates={u'ipalocation_location': location.idnsname_obj,
+                     u'ipaserviceweight': 200},
+            expected_updates={u'ipalocation_location': [location.idnsname_obj],
+                              u'enabled_role_servrole': lambda other: True,
+                              u'ipaserviceweight': [u'200']},
+            messages=self.messages)
+
         # remove invalid data from the previous test
         location.remove_server_from_location(server.server_name)
 
@@ -191,10 +201,16 @@ class TestLocationsServer(XMLRPC_test):
         location.retrieve()
 
     def test_remove_location_from_server(self, location, server):
-        server.update(dict(ipalocation_location=None))
+        server.update(
+            updates={u'ipalocation_location': None},
+            expected_updates={u'enabled_role_servrole': lambda other: True},
+            messages=self.messages)
         location.remove_server_from_location(server.server_name)
         location.retrieve()
 
     def test_remove_service_weight_from_server(self, location, server):
-        server.update(dict(ipaserviceweight=None))
+        server.update(
+            updates={u'ipaserviceweight': None},
+            expected_updates={u'enabled_role_servrole': lambda other: True},
+            messages=self.messages)
         location.retrieve()
diff --git a/ipatests/test_xmlrpc/tracker/location_plugin.py b/ipatests/test_xmlrpc/tracker/location_plugin.py
index 3bce6669aed59141c6cddf492afd478bcabff06d..92ccf874b497056e9c162086fca4defd7f1b7e90 100644
--- a/ipatests/test_xmlrpc/tracker/location_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/location_plugin.py
@@ -17,7 +17,8 @@ if six.PY3:
 
 class LocationTracker(Tracker):
     """Tracker for IPA Location tests"""
-    retrieve_keys = {'idnsname', 'description', 'dn', 'servers_server'}
+    retrieve_keys = {
+        'idnsname', 'description', 'dn', 'servers_server', 'dns_server'}
     retrieve_all_keys = retrieve_keys | {'objectclass'}
     create_keys = {'idnsname', 'description', 'dn', 'objectclass'}
     find_keys = {'idnsname', 'description', 'dn',}
@@ -130,21 +131,26 @@ class LocationTracker(Tracker):
     def add_server_to_location(
             self, server_name, weight=100, relative_weight=u"100.0%"):
         self.attrs.setdefault('servers_server', []).append(server_name)
+        self.attrs.setdefault('dns_server', []).append(server_name)
         self.servers[server_name] = {
             'cn': [server_name],
             'ipaserviceweight': [unicode(weight)],
-            'service_relative_weight': [relative_weight]
+            'service_relative_weight': [relative_weight],
+            'enabled_role_servrole': lambda other: True
         }
 
     def remove_server_from_location(self, server_name):
         if 'servers_server' in self.attrs:
             try:
                 self.attrs['servers_server'].remove(server_name)
+                self.attrs['dns_server'].remove(server_name)
             except ValueError:
                 pass
             else:
                 if not self.attrs['servers_server']:
                     del self.attrs['servers_server']
+                if not self.attrs['dns_server']:
+                    del self.attrs['dns_server']
         try:
             del self.servers[server_name]
         except KeyError:
diff --git a/ipatests/test_xmlrpc/tracker/server_plugin.py b/ipatests/test_xmlrpc/tracker/server_plugin.py
index 7540f45bf5f222e603fb446f7f2935376cca6be6..0a78cc9ace4d008b2c6d3306c3c2819439c4ae5d 100644
--- a/ipatests/test_xmlrpc/tracker/server_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/server_plugin.py
@@ -3,6 +3,7 @@
 #
 from __future__ import absolute_import
 
+from ipalib import errors
 from ipapython.dn import DN
 from ipatests.util import assert_deepequal
 from ipatests.test_xmlrpc.tracker.base import Tracker
@@ -13,7 +14,7 @@ class ServerTracker(Tracker):
     retrieve_keys = {
         'cn', 'dn', 'ipamaxdomainlevel', 'ipamindomainlevel',
         'iparepltopomanagedsuffix_topologysuffix', 'ipalocation_location',
-        'ipaserviceweight',
+        'ipaserviceweight', 'enabled_role_servrole'
     }
     retrieve_all_keys = retrieve_keys | {'objectclass'}
     create_keys = retrieve_keys | {'objectclass'}
@@ -101,10 +102,48 @@ class ServerTracker(Tracker):
             result=[],
         ), result)
 
-    def check_update(self, result, extra_keys=()):
+    def check_update(self, result, extra_keys=(), messages=None):
         """Check `server-update` command result"""
-        assert_deepequal(dict(
+        expected = dict(
             value=self.server_name,
-            summary=u'Modified IPA server "{server}"'.format(server=self.name),
+            summary=u'Modified IPA server "{server}"'.format(
+                server=self.name),
             result=self.filter_attrs(self.update_keys | set(extra_keys))
-        ), result)
+            )
+        if messages:
+            expected['messages'] = messages
+
+        assert_deepequal(expected, result)
+
+    def update(self, updates, expected_updates=None, messages=None):
+        if expected_updates is None:
+            expected_updates = {}
+
+        self.ensure_exists()
+        command = self.make_update_command(updates)
+        result = command()
+        self.attrs.update(updates)
+        self.attrs.update(expected_updates)
+        for key, value in self.attrs.items():
+            if value is None:
+                del self.attrs[key]
+
+        self.check_update(
+            result,
+            extra_keys=set(updates.keys()) | set(expected_updates.keys()),
+            messages=messages)
+
+    def make_fixture_clean_location(self, request):
+        command = self.make_update_command({u'ipalocation_location': None})
+        try:
+            command()
+        except errors.EmptyModlist:
+            pass
+
+        def cleanup():
+            try:
+                command()
+            except errors.EmptyModlist:
+                pass
+        request.addfinalizer(cleanup)
+        return self
-- 
2.7.4

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to