diff --git a/HACKING.rst b/HACKING.rst index f0d1373ee..b69ad91e0 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -18,4 +18,5 @@ Vitrage Specific Commandments [V325] Use six.iterkeys() or dict.keys() instead of dict.iterkeys() [V326] Use six.itervalues() or dict.values instead of dict.itervalues() [V327] Method's default argument shouldn't be mutable -[V328] Disallow LOG.warn \ No newline at end of file +[V328] Disallow LOG.warn +[V329] Don't use assertEqual(True/False, observed). \ No newline at end of file diff --git a/vitrage/hacking/checks.py b/vitrage/hacking/checks.py index ce0e7ae2b..778afef43 100644 --- a/vitrage/hacking/checks.py +++ b/vitrage/hacking/checks.py @@ -144,8 +144,29 @@ def no_log_warn(logical_line): yield(0, 'V328: Use LOG.warning() rather than LOG.warn()') +def check_assert_true_false(logical_line): + """V329 - Don't use assertEqual(True/False, observed).""" + if re.search(r"assertEqual\(\s*True,[^,]*(,[^,]*)?", logical_line): + msg = ("V329: Use assertTrue(observed) instead of " + "assertEqual(True, observed)") + yield (0, msg) + if re.search(r"assertEqual\([^,]*,\s*True(,[^,]*)?", logical_line): + msg = ("V329: Use assertTrue(observed) instead of " + "assertEqual(True, observed)") + yield (0, msg) + if re.search(r"assertEqual\(\s*False,[^,]*(,[^,]*)?", logical_line): + msg = ("V329: Use assertFalse(observed) instead of " + "assertEqual(False, observed)") + yield (0, msg) + if re.search(r"assertEqual\([^,]*,\s*False(,[^,]*)?", logical_line): + msg = ("V329: Use assertFalse(observed) instead of " + "assertEqual(False, observed)") + yield (0, msg) + + def factory(register): register(assert_true_instance) + register(check_assert_true_false) register(assert_equal_type) register(assert_equal_none) register(no_translate_logs) diff --git a/vitrage/tests/functional/api_handler/test_apis.py b/vitrage/tests/functional/api_handler/test_apis.py index bd4f3c430..52bbe0e25 100755 --- a/vitrage/tests/functional/api_handler/test_apis.py +++ b/vitrage/tests/functional/api_handler/test_apis.py @@ -413,7 +413,7 @@ class TestApis(TestEntityGraphUnitBase): condition = condition and \ (not tmp_project_id or (tmp_project_id and tmp_project_id == project_id)) - self.assertEqual(True, condition) + self.assertTrue(condition) def _check_resource_properties(self, resource, vitrage_id, resource_type, project_id=None): diff --git a/vitrage/tests/unit/graph/test_graph.py b/vitrage/tests/unit/graph/test_graph.py index 00fccc621..5a9a61fcd 100644 --- a/vitrage/tests/unit/graph/test_graph.py +++ b/vitrage/tests/unit/graph/test_graph.py @@ -548,8 +548,8 @@ class TestFilter(base.BaseTest): "HOST.NAME}", "host": "some_host_kukoo" } - self.assertEqual(True, check_filter(data=event_properties, - attr_filter=attr_filter)) + self.assertTrue(check_filter(data=event_properties, + attr_filter=attr_filter)) def test_basic_regex_with_no_match(self): event_properties = { @@ -566,5 +566,5 @@ class TestFilter(base.BaseTest): "HOST.NAME}", "host": "some_host_kukoo" } - self.assertEqual(False, check_filter(data=event_properties, - attr_filter=attr_filter)) + self.assertFalse(check_filter(data=event_properties, + attr_filter=attr_filter)) diff --git a/vitrage/tests/unit/hacking/test_hacking.py b/vitrage/tests/unit/hacking/test_hacking.py index 23b85c415..0608b50d7 100644 --- a/vitrage/tests/unit/hacking/test_hacking.py +++ b/vitrage/tests/unit/hacking/test_hacking.py @@ -157,13 +157,50 @@ class HackingTestCase(base.BaseTest): self.assertEqual(0, len(list(checks.no_log_warn('LOG.warning("bl")')))) self.assertEqual(1, len(list(checks.no_log_warn('LOG.warn("foo")')))) + def test_asserttruefalse(self): + true_fail_code1 = """ + test_bool = True + self.assertEqual(True, test_bool) + """ + true_fail_code2 = """ + test_bool = True + self.assertEqual(test_bool, True) + """ + true_pass_code = """ + test_bool = True + self.assertTrue(test_bool) + """ + false_fail_code1 = """ + test_bool = False + self.assertEqual(False, test_bool) + """ + false_fail_code2 = """ + test_bool = False + self.assertEqual(test_bool, False) + """ + false_pass_code = """ + test_bool = False + self.assertFalse(test_bool) + """ + self.assertEqual(1, len( + list(checks.check_assert_true_false(true_fail_code1)))) + self.assertEqual(1, len( + list(checks.check_assert_true_false(true_fail_code2)))) + self.assertEqual(0, len( + list(checks.check_assert_true_false(true_pass_code)))) + self.assertEqual(1, len( + list(checks.check_assert_true_false(false_fail_code1)))) + self.assertEqual(1, len( + list(checks.check_assert_true_false(false_fail_code2)))) + self.assertFalse(list(checks.check_assert_true_false(false_pass_code))) + def test_factory(self): class Register(object): def __init__(self): self.funcs = [] - def __call__(self, func): - self.funcs.append(func) + def __call__(self, _func): + self.funcs.append(_func) register = Register() checks.factory(register) diff --git a/vitrage/tests/unit/machine_learning/jaccard_correlation/test_jaccard_correlation.py b/vitrage/tests/unit/machine_learning/jaccard_correlation/test_jaccard_correlation.py index 1722bc601..3379949ff 100644 --- a/vitrage/tests/unit/machine_learning/jaccard_correlation/test_jaccard_correlation.py +++ b/vitrage/tests/unit/machine_learning/jaccard_correlation/test_jaccard_correlation.py @@ -382,7 +382,7 @@ class JaccardCorrelationTest(base.BaseTest): str(now) + "_correlations_test.out" is_file = os.path.isfile(file_path) - self.assertEqual(is_file, True) + self.assertTrue(is_file) if os.path.isfile(file_path): os.remove(file_path) diff --git a/vitrage_tempest_tests/tests/e2e/test_actions_base.py b/vitrage_tempest_tests/tests/e2e/test_actions_base.py index 415966302..9e675fb3f 100644 --- a/vitrage_tempest_tests/tests/e2e/test_actions_base.py +++ b/vitrage_tempest_tests/tests/e2e/test_actions_base.py @@ -69,8 +69,6 @@ class TestActionsBase(BaseVitrageTempest): g_utils.first_match(rca['nodes'], **expected_alarm), 'expected_alarm is not in the rca %s' % str(expected_alarm)) rca_inspected = rca['nodes'][rca['inspected_index']] - self.assertEqual( - True, - g_utils.is_subset(inspected, rca_inspected), - 'Invalid inspected item \n%s\n%s' % - (str(rca_inspected), str(inspected))) + self.assertTrue(g_utils.is_subset(inspected, rca_inspected), + 'Invalid inspected item \n%s\n%s' % + (str(rca_inspected), str(inspected))) diff --git a/vitrage_tempest_tests/tests/e2e/test_basic_actions.py b/vitrage_tempest_tests/tests/e2e/test_basic_actions.py index db6f4f55d..574e7ae17 100644 --- a/vitrage_tempest_tests/tests/e2e/test_basic_actions.py +++ b/vitrage_tempest_tests/tests/e2e/test_basic_actions.py @@ -176,7 +176,7 @@ class TestBasicActions(TestActionsBase): alarms = TempestClients.vitrage().alarm.list( vitrage_id=self.orig_host.get(VProps.VITRAGE_ID), all_tenants=True) - self.assertEqual(True, len(alarms) >= 2, 'alarms %s' % str(alarms)) + self.assertTrue(len(alarms) >= 2, 'alarms %s' % str(alarms)) deduced = g_utils.first_match(alarms, **DEDUCED_PROPS) trigger = g_utils.first_match(alarms, **TRIGGER_ALARM_2_PROPS)