Hi Martin,

Fixed.

On 07/22/2015 09:26 AM, Martin Basti wrote:
On 22/07/15 09:23, Oleg Fayans wrote:
Hi Martin,

Patch updated. Thank you for the review!

On 07/21/2015 05:45 PM, Martin Basti wrote:
On 20/07/15 14:07, Oleg Fayans wrote:
Hi Martin,

Updated.


On 07/20/2015 12:46 PM, Martin Basti wrote:
On 20/07/15 11:57, Oleg Fayans wrote:
+        pwfile = api.env.dot_ipa + os.sep + ".dmpw"
+        if ipautil.file_exists(pwfile):
+            fp = open(pwfile, "r")
+            dm_password = fp.read().rstrip()
+            fp.close()
+        else:
Hello,

1) Can you use os.path.join() instead of "+ os.sep +" please

2) Can you use with statement with file?

with open(pwfile, "r") as f:
    dm_password = f.read().rstrip()

3) Please keep PEP8 in new code

./ipatests/test_ipaserver/test_topology_plugin.py:30:80: E501 line too long (102 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:33:80: E501 line too long (92 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:39:80: E501 line too long (124 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:44:80: E501 line too long (92 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:45:48: E128 continuation line under-indented for visual indent ./ipatests/test_ipaserver/test_topology_plugin.py:45:80: E501 line too long (89 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:46:48: E128 continuation line under-indented for visual indent ./ipatests/test_ipaserver/test_topology_plugin.py:46:80: E501 line too long (89 > 79 characters) ./ipatests/test_ipaserver/test_topology_plugin.py:58:80: E501 line too long (87 > 79 characters)

4) Missing nose import
raise nose.SkipTest("No directory manager password in %s" % pwfile)

5) Can you use sets here instead of sorted lists?
assert(sorted(entry.keys()) == sorted(pluginattrs.keys()))


Martin^2


1)
Sorry, I didn't notice before, but there is missing header in that file.

2)
You don't need to specify ldap_uri, you just need to call ldap2(api), by default api.env.ldap_uri is used, which is the same as you specified

3)
Can you indent values of dict which are on newline? It is readable better.
            u'nsslapd-topo-plugin-shared-config-base':
                [u'cn=ipa,cn=etc,dc=example,dc=com'],
            u'nsslapd-pluginDescription': [u'ipa-topology-plugin'],

4)
Please use lower F as variable, in python we use capital letters for class definitions
            with open(pwfile, "r") as F:
                dm_password = F.read().rstrip()

Otherwise it works as expected.

Martin^2





Sorry.
You added there old license format, we now use in new files new format

#
# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
#


--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

From d9c0040ccdf07aad9c3ab2ca8dffee1865796409 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Wed, 22 Jul 2015 09:20:09 +0200
Subject: [PATCH] Added test - topology plugin is listed among DS plugins

---
 ipatests/test_ipaserver/test_topology_plugin.py | 85 +++++++++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 ipatests/test_ipaserver/test_topology_plugin.py

diff --git a/ipatests/test_ipaserver/test_topology_plugin.py b/ipatests/test_ipaserver/test_topology_plugin.py
new file mode 100644
index 0000000000000000000000000000000000000000..8cc9d351ac6ceefc75533de1ae17b318d01fe1ad
--- /dev/null
+++ b/ipatests/test_ipaserver/test_topology_plugin.py
@@ -0,0 +1,85 @@
+#
+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
+#
+
+import os
+from ipaserver.plugins.ldap2 import ldap2
+from ipalib import api
+from ipapython import ipautil
+from ipapython.dn import DN
+import nose
+
+
+class TestTopologyPlugin(object):
+    """
+    Test Topology plugin from the DS point of view
+    """
+
+    def setup(self):
+        """
+        setup for test
+        """
+        self.conn = None
+
+    def teardown(self):
+        if self.conn and self.conn.isconnected():
+            self.conn.disconnect()
+
+    def test_topologyplugin(self):
+        pluginattrs = {
+            u'nsslapd-pluginPath': [u'libtopology'],
+            u'nsslapd-pluginVendor': [u'freeipa'],
+            u'cn': [u'IPA Topology Configuration'],
+            u'nsslapd-plugin-depends-on-named':
+                [u'Multimaster Replication Plugin', u'ldbm database'],
+            u'nsslapd-topo-plugin-shared-replica-root': [u'dc=example,dc=com'],
+            u'nsslapd-pluginVersion': [u'1.0'],
+            u'nsslapd-topo-plugin-shared-config-base':
+                [u'cn=ipa,cn=etc,dc=example,dc=com'],
+            u'nsslapd-pluginDescription': [u'ipa-topology-plugin'],
+            u'nsslapd-pluginEnabled': [u'on'],
+            u'nsslapd-pluginId': [u'ipa-topology-plugin'],
+            u'objectClass': [u'top', u'nsSlapdPlugin', u'extensibleObject'],
+            u'nsslapd-topo-plugin-startup-delay': [u'20'],
+            u'nsslapd-topo-plugin-shared-binddngroup':
+                [u'cn=replication managers,cn=sysaccounts,cn=etc,dc=example,dc=com'],
+            u'nsslapd-pluginType': [u'object'],
+            u'nsslapd-pluginInitfunc': [u'ipa_topo_init']
+        }
+        variable_attrs = {u'nsslapd-topo-plugin-shared-replica-root',
+                          u'nsslapd-topo-plugin-shared-config-base',
+                          u'nsslapd-topo-plugin-shared-binddngroup'}
+
+        # Now eliminate keys that have domain-dependent values.
+        checkvalues = set(pluginattrs.keys()) - variable_attrs
+        topoplugindn = DN(('cn', 'IPA Topology Configuration'),
+                          ('cn', 'plugins'),
+                          ('cn', 'config'))
+        pwfile = os.path.join(api.env.dot_ipa, ".dmpw")
+        if ipautil.file_exists(pwfile):
+            with open(pwfile, "r") as f:
+                dm_password = f.read().rstrip()
+        else:
+            raise nose.SkipTest("No directory manager password in %s" % pwfile)
+        self.conn = ldap2(api)
+        self.conn.connect(bind_dn=DN(('cn', 'directory manager')),
+                          bind_pw=dm_password)
+        entry = self.conn.get_entry(topoplugindn)
+        assert(set(entry.keys()) == set(pluginattrs.keys()))
+        for i in checkvalues:
+            assert(pluginattrs[i] == entry[i])
-- 
2.4.3

-- 
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