Merge ~jslarraz/review-tools:remove-redundant-tests into review-tools:master

[Jorge Sancho Larraz]

So make check were running unittest test once under unittest target and again under check-names just to collect check names. This change merges unittest and check-names targets so we run check-names essentially for free.

[Alex Murray (alexmurray)]

LGTM - nice cleanup!

review: Approve

diff --git a/Makefile b/Makefile
index c76f2bf..523b0ac 100644
--- a/Makefile
+++ b/Makefile
@@ -63,21 +63,15 @@ flake8: clean
 
 check-syntax: pylint flake8
 
-.PHONY: check-names.list
-check-names.list:
-	(python3 collect-check-names-from-tests 2>&1 \
-	| grep 'CHECK|' | cut -d '|' -f 2- | egrep -v '(skeleton|some-check)' | LC_ALL=C sort \
-	| awk -F '|' '(l && $$1 != l1) {print l} {l1=$$1; l=$$0} END {print l}') > check-names.list
-
-check-names: check-names.list
-	# make sure check-names.list is up to date
-	cp -f check-names.list check-names.list.orig
-	$(MAKE) check-names.list
-	diff -Naur check-names.list.orig check-names.list || exit 1
-	rm -f check-names.list.orig
-
+TMP_DIR := $(shell mktemp -d)
 unittest:
-	python3 -m unittest discover -v -s ./reviewtools/tests -p 'test_*'
+	CHECK_NAMES_LIST=$(TMP_DIR)/check-names.raw python3 -m unittest discover -v -s ./reviewtools/tests -p 'test_*'
+
+	(cat $(TMP_DIR)/check-names.raw 2>&1 | egrep -v '(skeleton|some-check)' | LC_ALL=C sort \
+	| awk -F '|' '(l && $$1 != l1) {print l} {l1=$$1; l=$$0} END {print l}') > $(TMP_DIR)/check-names.list
+
+	diff -Naur check-names.list $(TMP_DIR)/check-names.list || exit 1
+	rm -rf $(TMP_DIR)
 
 coverage:
 	python3 -m coverage run -m unittest discover -s ./reviewtools/tests -p 'test_*'
@@ -100,7 +94,7 @@ functest-updates:
 functest-dump-tool:
 	./tests/test-dump-tool.sh
 
-check: check-deps check-style check-syntax check-names unittest functest-updates functest-dump-tool functest
+check: check-deps check-style check-syntax unittest functest-updates functest-dump-tool functest
 
 clean:
 	rm -rf ./reviewtools/__pycache__ ./reviewtools/tests/__pycache__
diff --git a/collect-check-names-from-tests b/collect-check-names-from-tests
 deleted file mode 100755
index f8da1fa..0000000
--- a/collect-check-names-from-tests
+++ /dev/null
@@ -1,28 +0,0 @@
-#!/usr/bin/python3
-'''collect-check-names-from-tests: print list of check names discovered from tests'''
-#
-# Copyright (C) 2015 Canonical Ltd.
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; version 3 of the License.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-# NOTE: changes to this file may also need to be made to 'run-tests'
-
-import logging
-import unittest
-
-test_directory = 'reviewtools/tests/'
-
-logging.basicConfig(level=logging.DEBUG)
-
-suite = unittest.TestLoader().discover(test_directory)
-unittest.TextTestRunner(verbosity=0).run(suite)
diff --git a/reviewtools/common.py b/reviewtools/common.py
index a42f919..eaaadee 100644
--- a/reviewtools/common.py
+++ b/reviewtools/common.py
@@ -20,7 +20,6 @@ import copy
 from enum import Enum
 import inspect
 import json
-import logging
 import os
 from pkg_resources import resource_filename
 import re
@@ -196,20 +195,19 @@ class ReviewBase(object):
         manual_review=False,
     ):
         """Add result to report"""
+        if review_name not in self.review_report.results[self.section][result_type]:
+            # save info about checks that has been run to be later
+            # compared against check-names.list
+            if os.getenv("CHECK_NAMES_LIST") is not None:
+                name = ":".join(review_name.split(":")[:2])
+                link_text = link if link is not None else ""
+                with open(os.getenv("CHECK_NAMES_LIST"), "a") as fd:
+                    fd.write("{}|{}\n".format(name, link_text))
+
         self.review_report.add_result(
             self.section, result_type, review_name, result, link, manual_review
         )
 
-        if review_name not in self.review_report.results[self.section][result_type]:
-            # log info about check so it can be collected into the
-            # check-names.list file
-            # format should be
-            # CHECK|<review_type:check_name>|<link>
-            msg = "CHECK|{}|{}"
-            name = ":".join(review_name.split(":")[:2])
-            link_text = link if link is not None else ""
-            logging.debug(msg.format(name, link_text))
-
     def do_checks(self):
         """Run all methods that start with check_"""
         methodList = [