Skip to content

Issue 11 checks failing non fatal #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 21, 2025
6 changes: 5 additions & 1 deletion grader/checks/abstract_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -65,3 +65,7 @@ def is_running_within_venv() -> bool:
:rtype: bool
"""
return VirtualEnvironment.is_initialized


class CheckError(Exception):
"""Custom exception for check errors."""
41 changes: 26 additions & 15 deletions grader/checks/coverage_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Expand All @@ -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)]

Expand All @@ -75,28 +76,38 @@ 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):
"""
Generate a report from the coverage tool.
"""
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)
23 changes: 17 additions & 6 deletions grader/checks/pylint_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
Expand All @@ -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)

Expand All @@ -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)]

Expand Down Expand Up @@ -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):
Expand Down
21 changes: 16 additions & 5 deletions grader/checks/type_hints_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)]

Expand Down
15 changes: 13 additions & 2 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions tests/test_abstract_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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()
11 changes: 6 additions & 5 deletions tests/test_coverage_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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")
Expand All @@ -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")
Expand Down
5 changes: 3 additions & 2 deletions tests/test_type_hints_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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")
Expand Down