gpcheckcat refactoring

- more accurate function names in GPObject
- remove duplication of query strings
- make file import for test subject independent of current working
  directory
- update test names to match casing convention

gpMgmt/bin/gpcheckcat

@@ -3444,7 +3444,7 @@ def checkUniqueIndexViolation():
         issue = CatUniqueIndexViolationIssue(violation['table_name'],
                                              violation['index_name'],
                                              violation['segment_id'])
-        error_object.setDuplicateIssue(issue)
+        error_object.addDuplicateIssue(issue)
 
 
 ############################################################################
@@ -3833,9 +3833,9 @@ def dropLeakedSchemas(dbname):
             # the query will return unquoted schema names
             # but we only search for schemas with name like pg_temp_[0-9](see the method getLeakedSchemas),
             # so we don't really need to quote the sql statement
-            logger.debug('DROP SCHEMA IF EXISTS "%s" CASCADE;\n' % schema)
             qry = 'DROP SCHEMA IF EXISTS "%s" CASCADE;\n' % schema
-            curs = db.query(qry)
+            logger.info(qry)
+            db.query(qry)
     except Exception, e:
         setError(ERROR_NOREPAIR)
         myprint('  Execution error: ' + str(e))
@@ -4124,11 +4124,11 @@ def processMissingDuplicateEntryResult(catname, colname, allValues, type):
 
         if type == "missing":
             issue = CatMissingIssue(catname, pkeys, segids, row[-2])
-            gpObj.setMissingIssue(issue)
+            gpObj.addMissingIssue(issue)
         else:
             assert (type == "duplicate")
             issue = CatDuplicateIssue(catname, pkeys, segids, row[-2])
-            gpObj.setDuplicateIssue(issue)
+            gpObj.addDuplicateIssue(issue)
 
 
 # -------------------------------------------------------------------------------
@@ -4182,7 +4182,7 @@ def processInconsistentEntryResult(catname, pknames, colname, allValues):
             rowObjName = 'pg_type'
 
         gpObj = getGPObject(oid, rowObjName)
-        gpObj.setInconsistentIssue(issue)
+        gpObj.addInconsistentIssue(issue)
 
 
 # -------------------------------------------------------------------------------
@@ -4223,7 +4223,7 @@ def processForeignKeyResult(catname, pkcatname, colname, allValues):
         elif pkcatname == 'pg_resourcetype' and gpColName == 'restypid':
             oid = getResourceTypeOid(oid)
         gpObj = getGPObject(oid, rowObjName)
-        gpObj.setForeignKeyIssue(issue)
+        gpObj.addForeignKeyIssue(issue)
 
 
 # -------------------------------------------------------------------------------
@@ -4263,7 +4263,7 @@ def processACLResult(catname, colname, allValues):
 
         issue = CatACLIssue(catname, pkeys, segid, macl_only, sacl_only)
         gpObj = getGPObject(oid, gpObjName)
-        gpObj.setACLIssue(issue)
+        gpObj.addACLIssue(issue)
 
 
 # -------------------------------------------------------------------------------
@@ -4422,31 +4422,31 @@ class GPObject:
         self.aclIssues = {}  # key=issue.catname, value=list of catACLIssue
         self.foreignkeyIssues = {}  # key=issue.catname, value=list of catForeignKeyIssue
 
-    def setMissingIssue(self, issue):
+    def addMissingIssue(self, issue):
         if issue.catname in self.missingIssues:
             self.missingIssues[issue.catname].append(issue)
         else:
             self.missingIssues[issue.catname] = [issue]
 
-    def setInconsistentIssue(self, issue):
+    def addInconsistentIssue(self, issue):
         if issue.catname in self.inconsistentIssues:
             self.inconsistentIssues[issue.catname].append(issue)
         else:
             self.inconsistentIssues[issue.catname] = [issue]
 
-    def setDuplicateIssue(self, issue):
+    def addDuplicateIssue(self, issue):
         if issue.catname in self.duplicateIssues:
             self.duplicateIssues[issue.catname].append(issue)
         else:
             self.duplicateIssues[issue.catname] = [issue]
 
-    def setACLIssue(self, issue):
+    def addACLIssue(self, issue):
         if issue.catname in self.aclIssues:
             self.aclIssues[issue.catname].append(issue)
         else:
             self.aclIssues[issue.catname] = [issue]
 
-    def setForeignKeyIssue(self, issue):
+    def addForeignKeyIssue(self, issue):
         if issue.catname in self.foreignkeyIssues:
             self.foreignkeyIssues[issue.catname].append(issue)
         else:

gpMgmt/bin/gppylib/test/unit/test_unit_gpcheckcat.py

@@ -10,10 +10,10 @@ class GpCheckCatTestCase(GpTestCase):
     def setUp(self):
         # because gpcheckcat does not have a .py extension, we have to use imp to import it
         # if we had a gpcheckcat.py, this is equivalent to:
-        #   from lib import gpcheckcat
+        #   import gpcheckcat
         #   self.subject = gpcheckcat
-        gpcheckcat_path = os.path.abspath('gpcheckcat')
-        self.subject = imp.load_source('gpcheckcat', gpcheckcat_path)
+        gpcheckcat_file = os.path.abspath(os.path.dirname(__file__) + "/../../../gpcheckcat")
+        self.subject = imp.load_source('gpcheckcat', gpcheckcat_file)
 
         self.subject.logger = Mock(spec=['log', 'info', 'debug'])
         self.db_connection = Mock(spec=[])
@@ -32,29 +32,30 @@ class GpCheckCatTestCase(GpTestCase):
             patch("gpcheckcat.UniqueIndexViolationCheck", return_value=self.unique_index_violation_check),
         ])
 
-    @skip("order of checks")
-    def test_runAllChecks_runsAllChecksInCorrectOrder(self, check):
-        self.subject.runAllChecks()
-        self.unique_index_violation_check.runCheck.assert_any_call(self.db_connection)
-        # add other checks here
-        # figure out how to enforce the order of calls;
-        # at a minimum, check the order number of the static list gpcheckcat.all_checks
-
-    def test_runningUnknownCheck_raisesException(self):
+    def test_running_unknown_check__raises_exception(self):
         with self.assertRaises(LookupError):
             self.subject.runOneCheck('some_unknown_check')
 
-    def test_runningUniqueIndexViolationCheck_makesTheCheck(self):
+    # @skip("order of checks")
+    # def test_run_all_checks__runs_all_checks_in_correct_order(self):
+    #     self.subject.runAllChecks()
+    #
+    #     self.unique_index_violation_check.runCheck.assert_any_call(self.db_connection)
+    #     # add other checks here
+    #     # figure out how to enforce the order of calls;
+    #     # at a minimum, check the order number of the static list gpcheckcat.all_checks
+
+    def test_running_unique_index_violation_check__makes_the_check(self):
         self.subject.runOneCheck('unique_index_violation')
 
         self.unique_index_violation_check.runCheck.assert_called_with(self.db_connection)
 
-    def test_runningCheck_whenNoViolationsAreFound_passesTheCheck(self):
+    def test_running_unique_index_violation_check__when_no_violations_are_found__passes_the_check(self):
         self.subject.runOneCheck('unique_index_violation')
 
         self.assertEqual(self.subject.GV.checkStatus, True)
 
-    def test_runningCheck_whenViolationsAreFound_failsTheCheck(self):
+    def test_running_unique_index_violation_check__when_violations_are_found__fails_the_check(self):
         self.unique_index_violation_check.runCheck.return_value = [
             dict(table_oid=123, table_name='stephen_table', index_name='finger', segment_id=8),
             dict(table_oid=456, table_name='larry_table', index_name='stock', segment_id=-1),
@@ -64,7 +65,7 @@ class GpCheckCatTestCase(GpTestCase):
 
         self.assertEqual(self.subject.GV.checkStatus, False)
 
-    def test_checkcatReport_afterRunningUniqueIndexViolationCheck_reportsViolations(self):
+    def test_checkcat_report__after_running_unique_index_violations_check__reports_violations(self):
         self.unique_index_violation_check.runCheck.return_value = [
             dict(table_oid=123, table_name='stephen_table', index_name='finger', segment_id=8),
             dict(table_oid=456, table_name='larry_table', index_name='stock', segment_id=-1),
@@ -79,7 +80,7 @@ class GpCheckCatTestCase(GpTestCase):
         self.assertIn(expected_message1, log_messages)
         self.assertIn(expected_message2, log_messages)
 
-    def test_dropLeakedSchemas_dropsOrphanedAndLeakedSchemas(self):
+    def test_drop_leaked_schemas__drops_orphaned_and_leaked_schemas(self):
         self.db_connection.mock_add_spec(['close', 'query'])
         self.subject.getLeakedSchemas = Mock(return_value=["fake_leak_1", "fake_leak_2"])
 

gpMgmt/bin/gppylib/test/unit/test_unit_gpcheckcat_unique_index_violation_check.py

@@ -25,14 +25,14 @@ class GpCheckCatUniqueIndexViolationCheckTestCase(GpTestCase):
         else:
             return self.violated_index_query_result
 
-    def test_runIndexViolationCheck_whenThereAreNoIssues(self):
+    def test_run_check__when_there_are_no_issues(self):
         self.violated_index_query_result.getresult.return_value = []
 
         violations = self.subject.runCheck(self.db_connection)
 
         self.assertEqual(len(violations), 0)
 
-    def test_runIndexViolationCheck_whenIndexIsViolated(self):
+    def test_run_check__when_index_is_violated(self):
         self.violated_index_query_result.getresult.return_value = [
             (-1, 'value1', 'value2', 2)
         ]