diff --git a/kodi_addon_checker/__main__.py b/kodi_addon_checker/__main__.py index f57607de..d59783a4 100644 --- a/kodi_addon_checker/__main__.py +++ b/kodi_addon_checker/__main__.py @@ -13,7 +13,7 @@ from kodi_addon_checker import __version__, check_addon, ValidKodiVersions from kodi_addon_checker.check_repo import check_repo -from kodi_addon_checker.common import load_plugins +from kodi_addon_checker.common import load_plugins, get_debug_log_path from kodi_addon_checker.config import Config, ConfigManager from kodi_addon_checker.logger import Logger from kodi_addon_checker.record import INFORMATION, PROBLEM, WARNING, Record @@ -75,12 +75,13 @@ def main(): parser.add_argument("--PR", help="Tell if tool is to run on a pull requests or not", action='store_true') parser.add_argument("--allow-folder-id-mismatch", help="Allow the addon's folder name and id to mismatch", action="store_true") - parser.add_argument("--enable-debug-log", help="Enable debug logging to kodi-addon-checker.log", + parser.add_argument("--enable-debug-log", + help="Enable debug logging to %s" % os.path.basename(get_debug_log_path()), action="store_true", default=False) ConfigManager.fill_cmd_args(parser) args = parser.parse_args() - log_file_name = os.path.join(os.getcwd(), "kodi-addon-checker.log") + log_file_name = get_debug_log_path() Logger.create_logger(log_file_name, __package__, args.enable_debug_log) all_repo_addons = check_addon.get_all_repo_addons() diff --git a/kodi_addon_checker/check_addon.py b/kodi_addon_checker/check_addon.py index 28c8b0dd..3f47b7e1 100644 --- a/kodi_addon_checker/check_addon.py +++ b/kodi_addon_checker/check_addon.py @@ -42,6 +42,16 @@ def start(addon_path, args, all_repo_addons, config=None): addon_xml_path = os.path.join(addon_path, "addon.xml") parsed_xml = ET.parse(addon_xml_path).getroot() + # Check if the debug log and/or the reporter log are enabled: it will be + # used to add them to the list of files to ignored + reporter_log_enabled = False + if args.reporter is not None: + if "log" in args.reporter: + reporter_log_enabled = True + debug_log_enabled = False + if args.enable_debug_log is not None: + debug_log_enabled = args.enable_debug_log + # Extract common path from addon paths # All paths will be printed relative to this path common.REL_PATH = os.path.split(addon_path[:-1])[0] @@ -103,7 +113,8 @@ def start(addon_path, args, all_repo_addons, config=None): # General blacklist check_string.find_blacklisted_strings(addon_report, addon_path, [], [], []) - check_files.check_file_whitelist(addon_report, file_index, addon_path) + check_files.check_file_whitelist(addon_report, file_index, addon_path, + debug_log_enabled, reporter_log_enabled) else: addon_report.add( Record(INFORMATION, "Addon marked as broken - skipping")) diff --git a/kodi_addon_checker/check_files.py b/kodi_addon_checker/check_files.py index 6aa0000d..0269bacc 100644 --- a/kodi_addon_checker/check_files.py +++ b/kodi_addon_checker/check_files.py @@ -12,7 +12,7 @@ import xml.etree.ElementTree as ET from . import handle_files -from .common import relative_path +from .common import relative_path, get_debug_log_path, get_reporter_log_path from .common.decorators import posix_only from .record import INFORMATION, PROBLEM, WARNING, Record from .report import Report @@ -111,9 +111,14 @@ def check_for_new_language_directory_structure(report: Report, addon_path: str, "an upper branch/kodi version." % os.path.join(language_path, directory))) -def check_file_whitelist(report: Report, file_index: list, addon_path: str): - """check whether the files present in addon are in whitelist or not - It ignores README.md and .gitignore file +def check_file_whitelist(report: Report, + file_index: list, + addon_path: str, + debug_log_enabled: bool, + reporter_log_enabled: bool): + """Check whether the files present in addon are in whitelist or not + It ignores the .gitignore file and the log files generated by the tool + (if the associated arguments are used). :file_index: list having names and path of all the files present in addon :addon_path: path to the addon folder """ @@ -127,14 +132,22 @@ def check_file_whitelist(report: Report, file_index: list, addon_path: str): r"help|list|mpeg|pls|info|ttf|xsp|theme|yaml|dict|crt|ico)?$" ) + # Depending on the arguments used, ignore the log files + files_to_ignore = [] + if debug_log_enabled: + files_to_ignore.append(get_debug_log_path()) + if reporter_log_enabled: + files_to_ignore.append(get_reporter_log_path()) + for file in file_index: - file_parts = file["name"].rsplit(".") - if len(file_parts) > 1: - file_ending = "." + file_parts[len(file_parts) - 1] - if re.match(whitelist, file_ending, re.IGNORECASE) is None: - report.add(Record(WARNING, - "Found non whitelisted file ending in filename %s" % - relative_path(os.path.join(file["path"], file["name"])))) + if os.path.join(file["path"], file["name"]) not in files_to_ignore: + file_parts = file["name"].rsplit(".") + if len(file_parts) > 1: + file_ending = "." + file_parts[len(file_parts) - 1] + if re.match(whitelist, file_ending, re.IGNORECASE) is None: + report.add(Record(WARNING, + "Found non whitelisted file ending in filename %s" % + relative_path(os.path.join(file["path"], file["name"])))) @posix_only diff --git a/kodi_addon_checker/common/__init__.py b/kodi_addon_checker/common/__init__.py index 00c5a86f..ea169d37 100644 --- a/kodi_addon_checker/common/__init__.py +++ b/kodi_addon_checker/common/__init__.py @@ -49,3 +49,21 @@ def relative_path(file_path): """ path_to_print = file_path[len(REL_PATH):] return ".{}".format(path_to_print) + + +def get_debug_log_path(): + """Return the path of the debug log file (including its name). + + :return: the full path of the debug log file + """ + + return os.path.join(os.getcwd(), "kodi-addon-checker.log") + + +def get_reporter_log_path(): + """Return the path of the reporter log file (including its name). + + :return: the full path of the reporter log file + """ + + return os.path.join(os.getcwd(), "kodi-addon-checker-report.log") diff --git a/kodi_addon_checker/plugins/log_reporter.py b/kodi_addon_checker/plugins/log_reporter.py index 37155b00..67ff6535 100644 --- a/kodi_addon_checker/plugins/log_reporter.py +++ b/kodi_addon_checker/plugins/log_reporter.py @@ -7,8 +7,8 @@ """ import logging import logging.handlers -import os +from kodi_addon_checker.common import get_reporter_log_path from kodi_addon_checker.report import Record from kodi_addon_checker.reporter import Reporter, reporter @@ -18,14 +18,14 @@ class LogReporter(Reporter): def __init__(self): self.logger = None + self.log_file_path = get_reporter_log_path() def create_logger(self): - log_file_name = os.path.join(os.getcwd(), "kodi-addon-checker-report.log") logger = logging.getLogger("log_reporter") logger.setLevel(logging.DEBUG) logger.propagate = False formatter = logging.Formatter(fmt="%(asctime)s: %(message)s", datefmt="%Y-%m-%d %H:%M:%S") - log_handler = logging.handlers.RotatingFileHandler(log_file_name, encoding="utf-8", mode="w") + log_handler = logging.handlers.RotatingFileHandler(self.log_file_path, encoding="utf-8", mode="w") log_handler.setFormatter(formatter) log_handler.setLevel(logging.DEBUG) logger.addHandler(log_handler) diff --git a/tests/test_check_addon.py b/tests/test_check_addon.py index 1e78eb36..9008b3ee 100644 --- a/tests/test_check_addon.py +++ b/tests/test_check_addon.py @@ -11,6 +11,8 @@ class Args(): PR = False allow_folder_id_mismatch = False branch = "krypton" + reporter = None + enable_debug_log = None class TestCheckAddon(unittest.TestCase): diff --git a/tests/test_check_files.py b/tests/test_check_files.py index 38d96a24..14758e23 100644 --- a/tests/test_check_files.py +++ b/tests/test_check_files.py @@ -2,11 +2,10 @@ import unittest from os.path import abspath, dirname, join -from kodi_addon_checker.check_files import check_file_permission +from kodi_addon_checker.check_files import check_file_permission, check_file_whitelist from kodi_addon_checker.handle_files import create_file_index -from kodi_addon_checker.common import load_plugins -from kodi_addon_checker.common import relative_path +from kodi_addon_checker.common import load_plugins, relative_path, get_debug_log_path, get_reporter_log_path from kodi_addon_checker.record import Record from kodi_addon_checker.reporter import ReportManager from kodi_addon_checker.report import Report @@ -39,3 +38,134 @@ def test_check_file_permission_is_None(self): path = join(HERE, 'test_data', 'Non-Executable_file') file_index = create_file_index(path) self.assertIsNone(check_file_permission(self.report, file_index)) + + +class TestCheckFileWhitelist(unittest.TestCase): + """Use cases: + - Ignore the log files if they are generated in the folder that is + being checked + - Report the files which have the same name as the log files but that + are not generated in this run of the checker + - Report the log files in the current folder if one/both reporter(s) + is/are disabled + """ + + def setUp(self): + # General initializaiton + load_plugins() + ReportManager.enable(["array"]) + self.report = Report("") + + # Create a dummy path not containing ".module." so that the check is + # not skipped + self.addon_path = "script.test" + + # Get the name of the log files + self.debug_log_filename = os.path.basename(get_debug_log_path()) + self.reporter_log_filename = os.path.basename(get_reporter_log_path()) + + # Create the file index from the "script.test" folder + script_test_path = join(HERE, "..", 'script.test') + self.file_index = create_file_index(script_test_path) + # The log files are supposed to be generated in the folder being + # checked so add them to the file index but stub the path to be in the + # current folder (since the log files are supposed to be generated there) + self.file_index.append({"path": os.path.dirname(get_debug_log_path()), + "name": self.debug_log_filename}) + self.file_index.append({"path": os.path.dirname(get_reporter_log_path()), + "name": self.reporter_log_filename}) + + def check_if_log_files_were_reported(self): + """Helper function to check if the log files were reported.""" + + # Initialize the returned values + debug_log_reported = False + reporter_log_reported = False + + # For each records generated check if the name of the log files was mentioned + for r in ReportManager.getEnabledReporters()[0].reports: + # Convert the record to a string + record = Record.__str__(r) + # Check if the log files name is in the record + if self.debug_log_filename in record: + debug_log_reported = True + elif self.reporter_log_filename in record: + reporter_log_reported = True + # Note: "elif" is used to improve performance because we assume only 1 + # log file may exist in a give record + + # Resert the report so that the next test case starts fresh + ReportManager.getEnabledReporters()[0].reports = [] + + return debug_log_reported, reporter_log_reported + + def test_check_file_whitelist_both_log_files_enabled(self): + """Test "check_file_whitelist" when both log files are enabled. + + The log files must be ignored because they are being generated in the + folder that is being analyzed. + """ + + # Call the function with both logs enabled + check_file_whitelist(self.report, self.file_index, self.addon_path, True, True) + + # Check if the log files were reported + debug_log_reported, reporter_log_reported = self.check_if_log_files_were_reported() + + # Only the reporter log file must have been reported + self.assertFalse(debug_log_reported) + self.assertFalse(reporter_log_reported) + + def test_check_file_whitelist_debug_log_file_enabled(self): + """Test "check_file_whitelist" when only the debug log is enabled. + + The debug log file must be ignored but if a file having the same as the + reporter log exists it must be reported. + """ + + # Call the function with only the debug log enabled + check_file_whitelist(self.report, self.file_index, self.addon_path, True, False) + + # Check if the log files were reported + debug_log_reported, reporter_log_reported = self.check_if_log_files_were_reported() + + # Only the reporter log file must have been reported + self.assertFalse(debug_log_reported) + self.assertTrue(reporter_log_reported) + + def test_check_file_whitelist_reporter_log_file_enabled(self): + """Test "check_file_whitelist" when only the reporter log is enabled. + + The reporter log file must be ignored but if a file having the same as + the reporter log exists it must be reported. + """ + + # Call the function with only the reporter log enabled + check_file_whitelist(self.report, self.file_index, self.addon_path, False, True) + + # Check if the log files were reported + debug_log_reported, reporter_log_reported = self.check_if_log_files_were_reported() + + # Only the reporter log file must have been reported + self.assertTrue(debug_log_reported) + self.assertFalse(reporter_log_reported) + + def test_check_file_whitelist_log_files_disabled(self): + """Test "check_file_whitelist" when both log files are disabled. + + The log files must be reported because they are being generated in the + folder that is being analyzed. + This use case validates also when the log files are generated out of + the analyzed folder but there are files with the same names in the + analyzed folder. + """ + + # Call the function with only the reporter log enabled + check_file_whitelist(self.report, self.file_index, self.addon_path, False, False) + + # Check if the log files were reported + debug_log_reported, reporter_log_reported = self.check_if_log_files_were_reported() + + # Only the reporter log file must have been reported + self.assertTrue(debug_log_reported) + self.assertTrue(reporter_log_reported)