Merge pull request #347 from webtech-network/test-harnessing

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

Author: ArthurCRodrigues

.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: