diff --git a/grader/checks/abstract_check.py b/grader/checks/abstract_check.py index 1e3a5a0..56d87ee 100644 --- a/grader/checks/abstract_check.py +++ b/grader/checks/abstract_check.py @@ -32,7 +32,7 @@ def run(self) -> float: :rtype: float """ if self._is_venv_required and not self.is_running_within_venv(): - raise RuntimeError("Virtual environment is required for this check") + raise CheckError("Virtual environment is required for this check") logger.log(VERBOSE, "Running %s", self.name) @@ -65,3 +65,7 @@ def is_running_within_venv() -> bool: :rtype: bool """ return VirtualEnvironment.is_initialized + + +class CheckError(Exception): + """Custom exception for check errors.""" diff --git a/grader/checks/coverage_check.py b/grader/checks/coverage_check.py index 10822c7..042acc1 100644 --- a/grader/checks/coverage_check.py +++ b/grader/checks/coverage_check.py @@ -5,7 +5,7 @@ import logging import os -from grader.checks.abstract_check import AbstractCheck +from grader.checks.abstract_check import AbstractCheck, CheckError from grader.utils.constants import ( COVERAGE_PATH, COVERAGE_RUN_ARGS, @@ -38,15 +38,12 @@ def run(self) -> float: """ super().run() - is_coverage_run_okay = self.__coverage_run() - - if not is_coverage_run_okay: - return 0.0 + self.__coverage_run() coverage_report_result = self.__coverage_report() if coverage_report_result is None: - return 0.0 + raise CheckError("Coverage report generation failed") return self.__translate_score(coverage_report_result) @@ -58,6 +55,10 @@ def __translate_score(self, coverage_score: float) -> float: :param coverage_score: The score from pylint to be translated :return: The translated score """ + + if self._max_points == -1: + raise CheckError("Max points for coverage check is set to -1") + step = 100 / (self._max_points + 1) steps = [i * step for i in range(self._max_points + 2)] @@ -75,13 +76,15 @@ def __coverage_run(self): """ command = [self.__coverage_full_path] + COVERAGE_RUN_ARGS + COVERAGE_RUN_PYTEST_ARGS + [self._project_root] - output = run(command, current_directory=self._project_root) + try: + output = run(command, current_directory=self._project_root) + except (OSError, ValueError) as e: + logger.error("Coverage run failed: %s", e) + raise CheckError("Coverage run failed") from e if output.returncode != 0: logger.error("Coverage run failed") - return False - - return True + raise CheckError("Coverage run failed") def __coverage_report(self): """ @@ -89,14 +92,22 @@ def __coverage_report(self): """ source_files = find_all_source_files(self._project_root) - command = [self.__coverage_full_path] + COVERAGE_REPORT_ARGS_NO_FORMAT + source_files - output = run(command, current_directory=self._project_root) + try: + command = [self.__coverage_full_path] + COVERAGE_REPORT_ARGS_NO_FORMAT + source_files + output = run(command, current_directory=self._project_root) + except (OSError, ValueError) as e: + logger.error("Coverage report (no format) failed: %s", e) + raise CheckError("Coverage report (no format) failed") from e - command = [self.__coverage_full_path] + COVERAGE_REPORT_ARGS + source_files - output = run(command, current_directory=self._project_root) + try: + command = [self.__coverage_full_path] + COVERAGE_REPORT_ARGS + source_files + output = run(command, current_directory=self._project_root) + except (OSError, ValueError) as e: + logger.error("Coverage report (with format) failed: %s", e) + raise CheckError("Coverage report (with format) failed") from e if output.returncode != 0: logger.error("Coverage report failed") - return None + raise CheckError("Coverage report failed") return int(output.stdout) diff --git a/grader/checks/pylint_check.py b/grader/checks/pylint_check.py index aa9f93c..2282f0c 100644 --- a/grader/checks/pylint_check.py +++ b/grader/checks/pylint_check.py @@ -12,7 +12,7 @@ import grader.utils.constants as const from grader.utils import process -from grader.checks.abstract_check import AbstractCheck +from grader.checks.abstract_check import AbstractCheck, CheckError from grader.utils.files import find_all_python_files logger = logging.getLogger("grader") @@ -38,7 +38,11 @@ def run(self) -> float: """ super().run() - pylint_args = find_all_python_files(self._project_root) + try: + pylint_args = find_all_python_files(self._project_root) + except OSError as error: + logger.error("Error while finding python files: %s", error) + raise CheckError("Error while finding python files") from error logger.debug("Running pylint check on files: %s", pylint_args) pylintrc_path = const.PYLINTRC @@ -47,10 +51,14 @@ def run(self) -> float: pylint_args.extend(["--rcfile", pylintrc_path]) command = [os.path.join(self._project_root, const.PYLINT_PATH)] + pylint_args - results = process.run(command, current_directory=self._project_root) + try: + results = process.run(command, current_directory=self._project_root) + except (OSError, ValueError) as error: + logger.error("Error while running pylint: %s", error) + raise CheckError("Error while running pylint") from error if results.returncode != 0: - return 0 + raise CheckError("Pylint check failed") pylint_score = self.__get_pylint_score(results.stdout) @@ -65,6 +73,9 @@ def __translate_score(self, pylint_score: float) -> float: :param pylint_score: The score from pylint to be translated :return: The translated score """ + if self._max_points == -1: + raise CheckError("Max points for pylint check is set to -1") + step = self.__pylint_max_score / (self._max_points + 1) steps = [i * step for i in range(self._max_points + 2)] @@ -94,10 +105,10 @@ def __get_pylint_score(self, pylint_output: str) -> float: return float(score) case None: logger.error("Pylint score not found") - return 0.0 + raise CheckError("Pylint score not found") logger.error("Pylint score not found") - return 0.0 + raise CheckError("Pylint score not found") class PylintCustomReporter(TextReporter): diff --git a/grader/checks/type_hints_check.py b/grader/checks/type_hints_check.py index e94b88f..891365e 100644 --- a/grader/checks/type_hints_check.py +++ b/grader/checks/type_hints_check.py @@ -5,7 +5,7 @@ import logging -from grader.checks.abstract_check import AbstractCheck +from grader.checks.abstract_check import AbstractCheck, CheckError from grader.utils.constants import MYPY_TYPE_HINT_CONFIG, REPORTS_TEMP_DIR, MYPY_LINE_COUNT_REPORT from grader.utils import files from grader.utils import process @@ -41,19 +41,27 @@ def run(self) -> float: super().run() # Gather all files - all_source_files = files.find_all_source_files(self._project_root) + try: + all_source_files = files.find_all_source_files(self._project_root) + except OSError as error: + logger.error("Error while finding python files: %s", error) + raise CheckError("Error while finding python files") from error # Run mypy on all files command = [self.__mypy_binary] + self.__mypy_arguments + all_source_files - _ = process.run(command) + try: + _ = process.run(command) + except (OSError, ValueError) as error: + logger.error("Error while running mypy: %s", error) + raise CheckError("Error while running mypy") from error # Read mypy linecount report try: with open(MYPY_LINE_COUNT_REPORT, "r", encoding="utf-8") as report_file: report = report_file.readline().strip().split() - except FileNotFoundError: + except FileNotFoundError as error: logger.error("Mypy linecount report not found") - return 0.0 + raise CheckError("Mypy linecount report not found") from error # Fancy way to get the needed values - I need the 3rd and 4th values, out of 5 total *_, lines_with_type_annotations, lines_total, _ = report @@ -73,6 +81,9 @@ def __translate_score(self, mypy_score: float) -> float: :param pylint_score: The score from pylint to be translated :return: The translated score """ + if self._max_points == -1: + raise CheckError("Max points for type hints check is set to -1") + step = self.__mypy_max_score / (self._max_points + 1) steps = [i * step for i in range(self._max_points + 2)] diff --git a/main.py b/main.py index d0a766e..6c72f4a 100644 --- a/main.py +++ b/main.py @@ -6,6 +6,7 @@ import sys import grader.utils.constants as const +from grader.checks.abstract_check import CheckError from grader.checks.checks_factory import create_checks from grader.utils.cli import get_args from grader.utils.config import load_config @@ -40,12 +41,22 @@ non_venv_checks, venv_checks = create_checks(config, project_root) for check in non_venv_checks: - check_score = check.run() + try: + check_score = check.run() + except CheckError as error: + logger.error("Check failed: %s", error) + check_score = 0.0 + scores.append((check.name, check_score, check.max_points)) with VirtualEnvironment(project_root) as venv: for check in venv_checks: - check_score = check.run() + try: + check_score = check.run() + except CheckError as error: + logger.error("Check failed: %s", error) + check_score = 0.0 + scores.append((check.name, check_score, check.max_points)) for name, score, max_score in scores: diff --git a/tests/test_abstract_check.py b/tests/test_abstract_check.py index 0dfa450..502d4ab 100644 --- a/tests/test_abstract_check.py +++ b/tests/test_abstract_check.py @@ -4,7 +4,7 @@ import unittest -from grader.checks.abstract_check import AbstractCheck +from grader.checks.abstract_check import AbstractCheck, CheckError class TestAbstractCheck(unittest.TestCase): @@ -29,5 +29,5 @@ def run(self) -> float: check = DummyCheck("dummy", 1, "dummy", is_venv_requred=True) # Act & Assert - with self.assertRaises(RuntimeError): + with self.assertRaises(CheckError): check.run() diff --git a/tests/test_coverage_check.py b/tests/test_coverage_check.py index 015da82..aba1f06 100644 --- a/tests/test_coverage_check.py +++ b/tests/test_coverage_check.py @@ -6,6 +6,7 @@ from subprocess import CompletedProcess from unittest.mock import patch, MagicMock +from grader.checks.abstract_check import CheckError from grader.checks.coverage_check import CoverageCheck @@ -25,18 +26,18 @@ def setUp(self): @patch("subprocess.run") def test_01_coverage_run_fail(self, mocked_run: MagicMock): """ - Test that a failed coverage run logs an error and returns a score of 0.0. + Test that a failed coverage run logs an error and raises an exception. """ # Arrange mocked_run.return_value = CompletedProcess(args=["coverage", "run"], returncode=1) # Act with self.assertLogs("grader", level="ERROR") as log: - result = self.coverage_check.run() + with self.assertRaises(CheckError): + self.coverage_check.run() is_message_logged = "ERROR:grader:Coverage run failed" in log.output # Assert - self.assertEqual(0.0, result) self.assertTrue(is_message_logged) @patch("subprocess.run") @@ -55,11 +56,11 @@ def mocked_run_side_effect(*args, **kwargs): mocked_run.side_effect = mocked_run_side_effect # Act with self.assertLogs("grader", level="ERROR") as log: - result = self.coverage_check.run() + with self.assertRaises(CheckError): + self.coverage_check.run() is_message_logged = "ERROR:grader:Coverage report failed" in log.output # Assert - self.assertEqual(0.0, result) self.assertTrue(is_message_logged) @patch("grader.checks.coverage_check.CoverageCheck._CoverageCheck__coverage_run") diff --git a/tests/test_type_hints_check.py b/tests/test_type_hints_check.py index 1b89351..520fae7 100644 --- a/tests/test_type_hints_check.py +++ b/tests/test_type_hints_check.py @@ -9,6 +9,7 @@ from unittest.mock import patch, MagicMock import grader.utils.constants as const +from grader.checks.abstract_check import CheckError from grader.checks.type_hints_check import TypeHintsCheck @@ -71,11 +72,11 @@ def test_03_mypy_report_missing(self, mocked_run: MagicMock): # Act with self.assertLogs(level="ERROR") as cm: - result = self.type_hints_check.run() + with self.assertRaises(CheckError): + self.type_hints_check.run() is_error_logged = any("Mypy linecount report not found" in log for log in cm.output) # Assert - self.assertEqual(result, 0.0) self.assertTrue(is_error_logged) @patch("grader.utils.process.run")