refactor: overhaul unit tests and achieve 100% coverage on core modules

.github/workflows/pylint.yml

@@ -32,4 +32,4 @@ jobs:
       - name: Analysing the code with pylint
         if: steps.changed-files.outputs.files != ''
         run: |
-          pylint ${{ steps.changed-files.outputs.files }}
+          PYTHONPATH=. pylint ${{ steps.changed-files.outputs.files }}

autograder/autograder.py

@@ -74,7 +74,7 @@ def run(self, submission: Submission):
                     )
                     break
                 logger.info("Step %s completed successfully (external_user_id=%s)", step_name, submission.user_id)
-            except Exception as e:
+            except Exception as e:  # pylint: disable=broad-exception-caught
                 pipeline_execution.status = PipelineStatus.INTERRUPTED
                 logger.error(
                     "Unhandled exception in step %s (external_user_id=%s): %s",
@@ -84,7 +84,6 @@ def run(self, submission: Submission):
                     exc_info=True,
                 )
                 break
-
         pipeline_execution.finish_execution() # Generates GradingResult object in pipeline execution
 
         # Cleanup: Destroy sandbox if it was created
@@ -113,7 +112,7 @@ def _cleanup_sandbox(self, pipeline_execution: PipelineExecution) -> None:
                     pipeline_execution.submission.user_id,
                     language.value if language else "none",
                 )
-        except Exception as e:
+        except Exception as e:  # pylint: disable=broad-exception-caught
             # Log error but don't fail the pipeline
             logger.warning(
                 "Failed to cleanup sandbox (external_user_id=%s): %s",
@@ -122,7 +121,7 @@ def _cleanup_sandbox(self, pipeline_execution: PipelineExecution) -> None:
             )
 
 
-def build_pipeline(
+def build_pipeline(  # pylint: disable=too-many-arguments,too-many-locals
     template_name,
     include_feedback,
     grading_criteria,
@@ -160,7 +159,7 @@ def build_pipeline(
     elif template_name:
         # Normalize template names (can be string, comma-separated string, or list)
         from autograder.steps.load_template_step import TemplateLoaderStep
-        names = TemplateLoaderStep._normalize_template_names(template_name)
+        names = TemplateLoaderStep.normalize_template_names(template_name)
         for name in names:
             templates.append(template_service.load_builtin_template(name))
 

autograder/models/abstract/ai_test_function.py

@@ -4,6 +4,7 @@
 from autograder.models.abstract.test_function import TestFunction
 from autograder.models.dataclass.submission import SubmissionFile
 from autograder.models.dataclass.test_result import TestResult
+from autograder.utils.executors.ai_executor import AiExecutor, TestInput
 from sandbox_manager.sandbox_container import SandboxContainer
 
 
@@ -35,6 +36,7 @@ def execute(
         self,
         files: Optional[List[SubmissionFile]],
         sandbox: Optional[SandboxContainer],
+        *args,
         **kwargs,
     ) -> TestResult:
         """
@@ -45,15 +47,18 @@ def execute(
         test name in that dict and returns the result directly. If the dict is
         absent (standalone usage), it falls back to ``_run_single()``.
         """
+        # args is not used in AI tests as parameters are in kwargs
+        _ = args
         pre_computed: Optional[Dict[str, TestResult]] = kwargs.get("pre_computed_results")
         if pre_computed is not None and self.name in pre_computed:
             return pre_computed[self.name]
 
-        return self._run_single(files, **kwargs)
+        return self._run_single(files, *args, **kwargs)
 
     def _run_single(
         self,
         files: Optional[List[SubmissionFile]],
+        *args,
         **kwargs,
     ) -> TestResult:
         """
@@ -62,9 +67,7 @@ def _run_single(
         Used when ``pre_computed_results`` is not available (e.g. unit tests,
         standalone runners, or pipelines that do not include ``AiBatchStep``).
         """
-        # Local import to avoid circular dependencies.
-        from autograder.utils.executors.ai_executor import AiExecutor, TestInput
-
+        _ = args
         locale: str = kwargs.get("locale", "en")
         prompt = self.build_prompt(files, **kwargs)
         submission_files = {f.filename: f.content for f in files} if files else {}

autograder/models/abstract/criteria_tree_processer.py

@@ -6,14 +6,17 @@
 
 
 class CriteriaTreeProcesser(ABC):
+    """
+    Abstract base class for processing the criteria tree structure.
+    """
     @abstractmethod
     def process_subject(self, subject: "SubjectNode") -> Any:
-        pass
+        """Process a subject node in the criteria tree."""
 
     @abstractmethod
     def process_test(self, test: "TestNode") -> Any:
-        pass
+        """Process a test node in the criteria tree."""
 
     @abstractmethod
     def process_category(self, category: "CategoryNode") -> Any:
-        pass
+        """Process a category node in the criteria tree."""

autograder/models/abstract/step.py

@@ -9,11 +9,13 @@
 
 
 class Step(ABC):
+    """
+    Abstract base class for all pipeline steps.
+    """
     @property
     @abstractmethod
     def step_name(self) -> StepName:
         """Return the name of the step (e.g. StepName.GRADE)."""
-        pass
 
     def execute(self, pipeline_exec: PipelineExecution) -> PipelineExecution:
         """
@@ -24,7 +26,7 @@ def execute(self, pipeline_exec: PipelineExecution) -> PipelineExecution:
         locale = pipeline_exec.locale
         try:
             return self._execute(pipeline_exec)
-        except Exception as e:
+        except Exception as e:  # pylint: disable=broad-exception-caught
             logger.exception("Step %s was interrupted: %s", self.step_name.value, str(e))
 
             error_msg = t("system.error.step_interrupted", locale=locale, error=str(e))
@@ -42,5 +44,4 @@ def execute(self, pipeline_exec: PipelineExecution) -> PipelineExecution:
     @abstractmethod
     def _execute(self, pipeline_exec: PipelineExecution) -> PipelineExecution:
         """Internal execution logic to be implemented by subclasses."""
-        pass
 

autograder/models/config/category.py

@@ -6,6 +6,7 @@
 
 
 class CategoryConfig(BaseModel):
+    """Configuration for a grading category (base, bonus, or penalty)."""
     weight: float = Field(
         ..., ge=0, le=100, description="Weight of this category (0-100)"
     )

autograder/models/config/criteria.py

@@ -9,7 +9,7 @@ class CriteriaConfig(BaseModel):
 
     test_library: Optional[str] = Field(
         None, description="Name of the test library/template to use"
-    ) # TODO -> Remove this attribute (it already sits in grading config)
+    )
     base: CategoryConfig = Field(..., description="Base grading criteria (required)")
     bonus: Optional[CategoryConfig] = Field(None, description="Bonus points criteria")
     penalty: Optional[CategoryConfig] = Field(None, description="Penalty criteria")

autograder/models/config/setup.py

@@ -11,6 +11,7 @@ class AssetConfig(BaseModel):
     @field_validator('source')
     @classmethod
     def validate_source(cls, v: str) -> str:
+        """Validate the source path of the asset."""
         if not v:
             raise ValueError("source must not be empty")
         if v.startswith('/'):
@@ -22,6 +23,7 @@ def validate_source(cls, v: str) -> str:
     @field_validator('target')
     @classmethod
     def validate_target(cls, v: str) -> str:
+        """Validate the target path of the asset."""
         if not v:
             raise ValueError("target must not be empty")
         if not v.startswith('/'):

autograder/models/config/subject.py

@@ -4,6 +4,7 @@
 
 
 class SubjectConfig(BaseModel):
+    """Configuration for a subject within a grading category."""
     subject_name: str = Field(..., description="Name of the subject")
     weight: float = Field(
         ..., ge=0, le=100, description="Weight of this subject (0-100)"

autograder/models/config/test.py

@@ -39,7 +39,7 @@ def get_args_list(self) -> List[Any]:
     def get_kwargs_dict(self) -> Dict[str, Any]:
         """Convert named parameters to keyword arguments dictionary."""
         kwargs = {}
-        if self.parameters:
+        if self.parameters is not None:
             kwargs.update({param.name: param.value for param in self.parameters})
 
         if self.model_extra:

autograder/models/criteria_tree.py

@@ -58,6 +58,7 @@ def __repr__(self):
         )
 
     def get_all_tests(self) -> List[TestNode]:
+        """Recursively collect all test nodes under this subject."""
         tests = [*self.tests]
         for subject in self.subjects:
             tests.extend(subject.get_all_tests())
@@ -88,6 +89,7 @@ def __repr__(self):
         )
 
     def add_subjects(self, subjects: List[SubjectNode]) -> None:
+        """Add a list of subjects to this category."""
         self.subjects.extend(subjects)
 
     def get_all_tests(self) -> List[TestNode]:

autograder/models/dataclass/focus.py

@@ -6,6 +6,7 @@
 
 @dataclass
 class FocusedTest:
+    """Represents a test result that has been prioritized for feedback."""
     test_result: TestResultNode
     diff_score: float
 
@@ -19,6 +20,7 @@ def to_dict(self) -> Dict[str, Any]:
 
 @dataclass
 class Focus:
+    """Organizes prioritized tests by category (base, penalty, bonus)."""
     base: List[FocusedTest]
     penalty: Optional[List[FocusedTest]]
     bonus: Optional[List[FocusedTest]]
@@ -30,4 +32,3 @@ def to_dict(self) -> Dict[str, Any]:
             "penalty": [test.to_dict() for test in self.penalty] if self.penalty else None,
             "bonus": [test.to_dict() for test in self.bonus] if self.bonus else None
         }
-

autograder/models/dataclass/grading_result.py

@@ -6,6 +6,7 @@
 
 @dataclass
 class GradingResult:
+    """Holds the final results of a grading execution."""
     final_score: float
     #status: str I'll evaluate if we keep this attribute or not
     feedback: Optional[str] = None

autograder/models/dataclass/submission.py

@@ -4,14 +4,16 @@
 
 @dataclass
 class SubmissionFile:
+    """Represents a single file in a submission."""
     filename: str
     content: str
 
 @dataclass
 class Submission:
+    """Represents a student's submission for an assignment."""
     username: str
     user_id: int
     assignment_id: int
     submission_files: Dict[str,SubmissionFile]
     language: Optional[Language] = None
-    locale: str = "en"
+    locale: str = "en"

autograder/models/dataclass/test_result.py

@@ -17,6 +17,7 @@ def get_result(self):
         return [self]
 
     def to_dict(self) -> dict:
+        """Convert TestResult to dictionary for JSON serialization."""
         return {
             "test_name": self.test_name,
             "score": self.score,

autograder/models/pipeline_execution.py

@@ -6,13 +6,13 @@
 from autograder.models.dataclass.grading_result import GradingResult
 from autograder.models.dataclass.step_result import StepResult, StepName, StepStatus
 from autograder.models.dataclass.submission import Submission
+from autograder.models.dataclass.structural_analysis_result import StructuralAnalysisResult
 
 if TYPE_CHECKING:
     from autograder.models.abstract.template import Template
     from autograder.models.criteria_tree import CriteriaTree
     from autograder.models.dataclass.focus import Focus
     from autograder.models.dataclass.grade_step_result import GradeStepResult
-    from autograder.models.dataclass.structural_analysis_result import StructuralAnalysisResult
     from autograder.models.result_tree import ResultTree
     from sandbox_manager.sandbox_container import SandboxContainer
 
@@ -133,7 +133,6 @@ def get_structural_analysis_result(self) -> Optional["StructuralAnalysisResult"]
         """
         if not self.has_step_result(StepName.STRUCTURAL_ANALYSIS):
             return None
-        from autograder.models.dataclass.structural_analysis_result import StructuralAnalysisResult
         return cast(
             StructuralAnalysisResult,
             self._require_step_data(StepName.STRUCTURAL_ANALYSIS, "structural analysis result"),

autograder/services/assets/cache_manager.py

@@ -7,6 +7,9 @@
 logger = logging.getLogger("AssetCacheManager")
 
 class AssetCacheManager:
+    """
+    Manages caching of assets in memory and on disk.
+    """
     def __init__(self, in_memory_limit: int = 0, disk_cache_dir: str = "/tmp/autograder_assets_cache"):
         """
         Initialize the asset cache manager.
@@ -48,7 +51,7 @@ def get(self, asset_key: str) -> Optional[bytes]:
                     self._add_to_memory_cache(asset_key, content)
 
                 return content
-            except Exception as e:
+            except Exception as e:  # pylint: disable=broad-exception-caught
                 logger.error("Failed to read asset from disk cache: %s", str(e))
 
         return None
@@ -63,7 +66,7 @@ def put(self, asset_key: str, content: bytes) -> None:
             with open(disk_path, "wb") as f:
                 f.write(content)
             logger.debug("Stored asset %s in disk cache", asset_key)
-        except Exception as e:
+        except Exception as e:  # pylint: disable=broad-exception-caught
             logger.error("Failed to write asset to disk cache: %s", str(e))
 
         # Save to memory if enabled

autograder/services/assets/provider.py

@@ -2,6 +2,9 @@
 from typing import Optional
 
 class AssetProvider(ABC):
+    """
+    Abstract base class for asset providers.
+    """
     @abstractmethod
     def get_asset(self, source: str, target: str, read_only: bool = True) -> Optional[bytes]:
         """
@@ -15,4 +18,3 @@ def get_asset(self, source: str, target: str, read_only: bool = True) -> Optiona
         Returns:
             Raw content as bytes, or None if not found or on error.
         """
-        ...

autograder/services/assets/resolver.py

@@ -9,6 +9,9 @@
 logger = logging.getLogger("AssetSourceResolver")
 
 class AssetSourceResolver:
+    """
+    Resolves asset configurations into actual asset content using providers and caches.
+    """
     def __init__(self):
         in_memory_limit = int(os.getenv("EXTERNAL_ASSETS_IN_MEMORY_CACHE_LIMIT", "100"))
         self.cache_manager = AssetCacheManager(in_memory_limit=in_memory_limit)

autograder/services/assets/s3_provider.py

@@ -9,6 +9,9 @@
 logger = logging.getLogger("S3AssetProvider")
 
 class S3AssetProvider(AssetProvider):
+    """
+    Asset provider that fetches files from Amazon S3.
+    """
     def __init__(self, cache_manager: AssetCacheManager):
         self.cache_manager = cache_manager
 
@@ -53,6 +56,6 @@ def get_asset(self, source: str, target: str, read_only: bool = True) -> Optiona
             self.cache_manager.put(cache_key, raw_content)
 
             return raw_content
-        except Exception as e:
+        except Exception as e:  # pylint: disable=broad-exception-caught
             logger.error("Failed to fetch asset from S3 (%s): %s", source, str(e))
             return None

autograder/services/focus_service.py

@@ -9,6 +9,9 @@
 
 
 class FocusService:
+    """
+    Service responsible for identifying the most impactful tests for student feedback.
+    """
     def __calculate_impact(
         self, test: TestResultNode, cumulative_multiplier: float
     ) -> float:
@@ -25,7 +28,7 @@ def __calculate_impact(
     def __process_subject(
         self, subject: SubjectResultNode, parent_multiplier: float
     ) -> List[FocusedTest]:
-        focused_tests = list()
+        focused_tests = []
 
         # Determine the multiplier for children of this subject
         # If this subject has sub-subjects and tests, we might need to split the weight
@@ -98,6 +101,9 @@ def __process_category(self, category: CategoryResultNode) -> List[FocusedTest]:
         return focused_tests
 
     def find(self, result_tree: ResultTree) -> Focus:
+        """
+        Find and prioritize impactful tests from the result tree.
+        """
         return Focus(
             base=self.__process_category(result_tree.root.base),
             penalty=self.__process_category(result_tree.root.penalty)