So far, the (fake) SSH file manager had a couple of check functions (which return "True" or "False") and a couple of functions which throw an exception instead of returning "False" (as in those cases more information for debugging is neeede). However, it was not obvious from the function name which behavior to expect. This patch renames all functions which throw exceptions to "Assert*" and removes superfluous 'self.assertTrue(*)" calls around them.
Signed-off-by: Helga Velroyen <[email protected]> --- test/py/ganeti.backend_unittest.py | 63 ++++++++++++++++---------------------- test/py/testutils_ssh.py | 11 +++---- 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/test/py/ganeti.backend_unittest.py b/test/py/ganeti.backend_unittest.py index 804cd63..73db0a9 100755 --- a/test/py/ganeti.backend_unittest.py +++ b/test/py/ganeti.backend_unittest.py @@ -1119,10 +1119,9 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self._ssh_file_manager.PotentialMasterCandidatesOnlyHavePublicKey( + self._ssh_file_manager.AssertPotentialMasterCandidatesOnlyHavePublicKey( new_node_name) - self.assertTrue(self._ssh_file_manager.AllNodesHaveAuthorizedKey( - new_node_key)) + self._ssh_file_manager.AssertAllNodesHaveAuthorizedKey(new_node_key) def testAddPotentialMasterCandidate(self): new_node_name = "new_node_name" @@ -1148,10 +1147,9 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self._ssh_file_manager.PotentialMasterCandidatesOnlyHavePublicKey( + self._ssh_file_manager.AssertPotentialMasterCandidatesOnlyHavePublicKey( new_node_name) - self.assertTrue(self._ssh_file_manager.NoNodeHasAuthorizedKey( - new_node_key)) + self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(new_node_key) def testAddNormalNode(self): new_node_name = "new_node_name" @@ -1177,10 +1175,8 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self.assertTrue(self._ssh_file_manager.NoNodeHasPublicKey( - new_node_uuid, new_node_key)) - self.assertTrue(self._ssh_file_manager.NoNodeHasAuthorizedKey( - new_node_key)) + self._ssh_file_manager.AssertNoNodeHasPublicKey(new_node_uuid, new_node_key) + self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(new_node_key) def testPromoteToMasterCandidate(self): # Get one of the potential master candidates @@ -1201,10 +1197,9 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self._ssh_file_manager.PotentialMasterCandidatesOnlyHavePublicKey( + self._ssh_file_manager.AssertPotentialMasterCandidatesOnlyHavePublicKey( node_name) - self.assertTrue(self._ssh_file_manager.AllNodesHaveAuthorizedKey( - node_key)) + self._ssh_file_manager.AssertAllNodesHaveAuthorizedKey(node_key) def testRemoveMasterCandidate(self): (node_name, node_uuid, node_key, is_potential_master_candidate, @@ -1224,9 +1219,8 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self.assertTrue(self._ssh_file_manager.NoNodeHasPublicKey( - node_uuid, node_key)) - self.assertTrue(self._ssh_file_manager.NoNodeHasAuthorizedKey(node_key)) + self._ssh_file_manager.AssertNoNodeHasPublicKey(node_uuid, node_key) + self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_key) self.assertEqual(0, len(self._ssh_file_manager.GetPublicKeysOfNode(node_name))) self.assertEqual(0, @@ -1250,9 +1244,8 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self.assertTrue(self._ssh_file_manager.NoNodeHasPublicKey( - node_uuid, node_key)) - self.assertTrue(self._ssh_file_manager.NoNodeHasAuthorizedKey(node_key)) + self._ssh_file_manager.AssertNoNodeHasPublicKey(node_uuid, node_key) + self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_key) self.assertEqual(0, len(self._ssh_file_manager.GetPublicKeysOfNode(node_name))) self.assertEqual(0, @@ -1276,9 +1269,8 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self.assertTrue(self._ssh_file_manager.NoNodeHasPublicKey( - node_uuid, node_key)) - self.assertTrue(self._ssh_file_manager.NoNodeHasAuthorizedKey(node_key)) + self._ssh_file_manager.AssertNoNodeHasPublicKey(node_uuid, node_key) + self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_key) self.assertEqual(0, len(self._ssh_file_manager.GetPublicKeysOfNode(node_name))) self.assertEqual(0, @@ -1305,8 +1297,9 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self._ssh_file_manager.PotentialMasterCandidatesOnlyHavePublicKey(node_name) - self.assertTrue(self._ssh_file_manager.NoNodeHasAuthorizedKey(node_key)) + self._ssh_file_manager.AssertPotentialMasterCandidatesOnlyHavePublicKey( + node_name) + self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_key) def testDemotePotentialMasterCandidateToNormalNode(self): (node_name, node_uuid, node_key, is_potential_master_candidate, @@ -1329,9 +1322,8 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self.assertTrue(self._ssh_file_manager.NoNodeHasPublicKey( - node_uuid, node_key)) - self.assertTrue(self._ssh_file_manager.NoNodeHasAuthorizedKey(node_key)) + self._ssh_file_manager.AssertNoNodeHasPublicKey(node_uuid, node_key) + self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_key) def _GetReducedOnlineNodeList(self): """'Randomly' mark some nodes as offline.""" @@ -1424,10 +1416,10 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self._ssh_file_manager.PotentialMasterCandidatesOnlyHavePublicKey( + self._ssh_file_manager.AssertPotentialMasterCandidatesOnlyHavePublicKey( new_node_name) - self.assertTrue(self._ssh_file_manager.AllNodesHaveAuthorizedKey( - new_node_key)) + self._ssh_file_manager.AssertAllNodesHaveAuthorizedKey( + new_node_key) def testAddKeyFailedOnNewNodeWithRetries(self): """Tests clean up if updating a new node's SSH setup fails. @@ -1465,8 +1457,7 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): self.assertFalse(self._ssh_file_manager.NodeHasAuthorizedKey( node, new_node_key)) - self.assertTrue(self._ssh_file_manager.NoNodeHasPublicKey( - new_node_uuid, new_node_key)) + self._ssh_file_manager.AssertNoNodeHasPublicKey(new_node_uuid, new_node_key) def testAddKeySuccessfullyOnOldNodeWithRetries(self): """Tests adding a new key even if updating nodes takes retries. @@ -1499,8 +1490,7 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self.assertTrue(self._ssh_file_manager.AllNodesHaveAuthorizedKey( - new_node_key)) + self._ssh_file_manager.AssertAllNodesHaveAuthorizedKey(new_node_key) def testAddKeyFailedOnOldNodeWithRetries(self): """Tests adding keys when updating one node's SSH setup fails. @@ -1572,9 +1562,8 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock) - self.assertTrue(self._ssh_file_manager.NoNodeHasPublicKey( - node_uuid, node_key)) - self.assertTrue(self._ssh_file_manager.NoNodeHasAuthorizedKey(node_key)) + self._ssh_file_manager.AssertNoNodeHasPublicKey(node_uuid, node_key) + self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_key) def testRemoveKeyFailedWithRetriesOnOtherNode(self): """Test removing keys even if one of the old nodes fails even with retries. diff --git a/test/py/testutils_ssh.py b/test/py/testutils_ssh.py index a180144..43ba709 100644 --- a/test/py/testutils_ssh.py +++ b/test/py/testutils_ssh.py @@ -257,7 +257,7 @@ class FakeSshFileManager(object): """ return key in self._authorized_keys[file_node_name] - def AllNodesHaveAuthorizedKey(self, key): + def AssertAllNodesHaveAuthorizedKey(self, key): """Check if all nodes have a particular key in their auth. keys file. @type key: string @@ -270,9 +270,8 @@ class FakeSshFileManager(object): if key not in self._authorized_keys[name]: raise Exception("Node '%s' does not have the key '%s' in its" " 'authorized_keys' file." % (name, key)) - return True - def NoNodeHasAuthorizedKey(self, key): + def AssertNoNodeHasAuthorizedKey(self, key): """Check if none of the nodes has a particular key in their auth. keys file. @type key: string @@ -286,9 +285,8 @@ class FakeSshFileManager(object): if key in node_auth_keys: raise Exception("Node '%s' does have the key '%s' in its" " 'authorized_keys' file." % (name, key)) - return True - def NoNodeHasPublicKey(self, uuid, key): + def AssertNoNodeHasPublicKey(self, uuid, key): """Check if none of the nodes have the given public key in their file. @type uuid: string @@ -302,9 +300,8 @@ class FakeSshFileManager(object): if (uuid, key) in node_pub_keys.items(): raise Exception("Node '%s' does have public key '%s' of node '%s'" % (name, key, uuid)) - return True - def PotentialMasterCandidatesOnlyHavePublicKey(self, query_node_name): + def AssertPotentialMasterCandidatesOnlyHavePublicKey(self, query_node_name): """Checks if the node's key is on all potential master candidates only. This ensures that the node's key is in all public key files of all -- 2.2.0.rc0.207.ga3a616c
