diff --git a/.github/prompts/prompt.prompt.md b/.github/prompts/prompt.prompt.md new file mode 100644 index 0000000..538ab46 --- /dev/null +++ b/.github/prompts/prompt.prompt.md @@ -0,0 +1,6 @@ +--- +agent: agent +--- +Define the task to achieve, including specific requirements, constraints, and success criteria. + +Before proceeding with any implementation, align the plan with the user. Ask any clarification questions that will help with the successful implementation of the plan. \ No newline at end of file diff --git a/.github/workflows/cicd.yml b/.github/workflows/cicd.yml index 0fc03e3..72c87e9 100644 --- a/.github/workflows/cicd.yml +++ b/.github/workflows/cicd.yml @@ -32,17 +32,11 @@ jobs: - name: Install the project run: uv sync --locked --all-extras --dev - - name: pylint - run: uv run pylint grader desktop tests pygrader.py --fail-under 9 - - - name: mypy - run: uv run mypy grader desktop tests pygrader.py --ignore-missing-imports - - - name: flake8 - run: uv run flake8 grader desktop tests pygrader.py - - - name: complexipy - run: uv run complexipy . + - name: Install just + run: apt install just + + - name: Lint code + run: just lint test: runs-on: ubuntu-latest needs: lint @@ -60,15 +54,15 @@ jobs: - name: Install the project run: uv sync --locked --all-extras --dev + - name: Install just + run: apt install just + - name: Run the unit tests - run: | - find tests -type f -name "test_*.py" -not -name "test_functional.py" -not -path "*sample_project*" | xargs uv run -m unittest -v - shell: bash + run: just unit_tests - name: Run the functional tests - run: | - uv run -m unittest discover -s tests -p "test_functional.py" - shell: bash + run: just functional_tests + docker: needs: test runs-on: ubuntu-latest diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 4ee489d..1add914 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -24,18 +24,13 @@ jobs: - name: Install the project run: uv sync --locked --all-extras --dev + + - name: Install just + run: apt install just - - name: pylint - run: uv run pylint grader desktop tests pygrader.py --fail-under 9 + - name: Lint code + run: just lint - - name: mypy - run: uv run mypy grader desktop tests pygrader.py --ignore-missing-imports - - - name: flake8 - run: uv run flake8 grader desktop tests pygrader.py - - - name: complexipy - run: uv run complexipy . test: runs-on: ubuntu-latest needs: lint @@ -52,13 +47,42 @@ jobs: - name: Install the project run: uv sync --locked --all-extras --dev + + - name: Install just + run: apt install just - name: Run the unit tests + run: just unit_tests + + - name: Run the functional tests + run: just functional_tests + + changelog-and-version: + runs-on: ubuntu-latest + needs: test + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Check if CHANGELOG.md is updated run: | - find tests -type f -name "test_*.py" -not -name "test_functional.py" -not -path "*sample_project*" | xargs uv run -m unittest -v + git fetch origin main:main + if git diff --name-only origin/main...HEAD | grep -q "CHANGELOG.md"; then + echo "✓ CHANGELOG.md has been updated" + else + echo "✗ CHANGELOG.md has not been updated" + exit 1 + fi shell: bash - - name: Run the functional tests + - name: Check if version is changed run: | - uv run -m unittest discover -s tests -p "test_functional.py" + git fetch origin main:main + if git diff origin/main...HEAD pyproject.toml | grep -q "^[+-]version = "; then + echo "✓ Version in pyproject.toml has been changed" + else + echo "✗ Version in pyproject.toml has not been changed" + exit 1 + fi shell: bash \ No newline at end of file diff --git a/.gitignore b/.gitignore index 9c0a106..44ba5f6 100644 --- a/.gitignore +++ b/.gitignore @@ -183,4 +183,6 @@ docs/diagrams/out .private *.zip -**/.DS_Store \ No newline at end of file +**/.DS_Store + +.complexipy_cache \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 77bbd10..f15e72a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # pygrader +## 1.8.0 + +- Check results now show useful info and error messages +- The venv created by pygrader is now with a non-standard name +- Deleting existing venv is now optional +- If a zip contains only the project as a subdirectory, use the subdir instead +- Requirements check can now check for project installation + ## 1.7.1 - Subpackages are now properly exported diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..09a3c69 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,388 @@ +# Claude AI Assistant Guide for pygrader + +This document provides context and guidelines for Claude Sonnet 4.5 when working on the pygrader project. + +## Project Overview + +**pygrader** is an automated grading tool for Python projects used in the "Programming with Python" course at Sofia University. It evaluates student submissions based on multiple quality checks and generates grading reports. + +### Key Information +- **Language**: Python 3.10+ +- **Version**: 1.7.1 +- **License**: GPL-3.0 +- **Architecture**: Modular check-based system with CLI and web interfaces +- **Current Branch**: `show-check-results` (default: `main`) + +## Project Structure + +``` +pygrader/ +├── grader/ # Core grading engine +│ ├── grader.py # Main Grader orchestrator class +│ ├── checks/ # Check implementations +│ │ ├── abstract_check.py # Base classes for all checks +│ │ ├── checks_factory.py # Factory pattern for check creation +│ │ ├── coverage_check.py # Code coverage validation +│ │ ├── pylint_check.py # Linting checks +│ │ ├── requirements_check.py # Dependency validation +│ │ ├── run_tests_check.py # Test execution +│ │ ├── structure_check.py # Project structure validation +│ │ └── type_hints_check.py # Type hint checking (mypy) +│ └── utils/ # Shared utilities +│ ├── config.py # Configuration loading +│ ├── constants.py # Project constants +│ ├── environment.py # Environment management +│ ├── external_resources.py # External resource handling +│ ├── files.py # File operations +│ ├── json_with_templates.py # JSON template processing +│ ├── logger.py # Logging utilities +│ ├── process.py # Subprocess management +│ ├── structure_validator.py # Structure validation logic +│ └── virtual_environment.py # venv management +├── desktop/ # CLI interface +│ ├── cli.py # Command-line argument parsing +│ ├── main.py # Desktop entry point +│ └── results_reporter.py # Results formatting/output +├── web/ # Web interface (Flask/Django) +├── tests/ # Test suite +│ ├── unit/ # Unit tests (test_*.py files) +│ └── functional/ # Integration tests +├── config/ # Configuration templates +│ ├── *.json # Various grading configurations +│ └── *.pylintrc # Pylint configuration files +└── docs/ # Sphinx documentation + └── source/ # RST documentation files +``` + +## Core Concepts + +### 1. Checks System +The grading system is built around **checks** - independent validation modules: + +- **AbstractCheck**: Base class for all checks (`grader/checks/abstract_check.py`) + - `ScoredCheck`: Checks that contribute to the final score + - `NonScoredCheck`: Checks that only report pass/fail +- **ChecksFactory**: Creates check instances from configuration (`checks_factory.py`) +- Each check returns a `CheckResult` with the following fields: + - `ScoredCheckResult`: Contains `name`, `result` (score), `info`, `error`, and `max_score` + - `info`: Informational output about the check (e.g., coverage percentage, pylint summary) + - `error`: Error messages if the check failed + - `NonScoredCheckResult`: Contains `name`, `result` (pass/fail), `info`, and `error` + - `CheckError`: Exception raised when a check cannot execute + +### 2. Configuration System +- JSON-based configuration files in `config/` directory +- Supports templates via `json_with_templates.py` +- Defines which checks to run and their scoring weights +- Example configs: `2024.json`, `projects_2025.json`, `full.json` +- **Virtual Environment Configuration**: Optional `venv` key for customizing venv behavior: + ```json + { + "venv": { + "is_keeping_existing_venv": true, // Keep existing .venv/.venv-pygrader + "name": ".venv-custom" // Custom venv directory name + } + } + ``` +- **Configuration Validation**: Currently NO schema validation + - Unknown keys in config are silently ignored + - Typos in venv config keys will cause defaults to be used + - Consider adding validation for production use + +### 3. Virtual Environment Management +- Each project is graded in an isolated virtual environment +- Managed by `VirtualEnvironment` class (`utils/virtual_environment.py`) +- Handles dependency installation from `requirements.txt` or `pyproject.toml` +- **Configuration Options**: + - `is_keeping_venv_after_run`: Keep venv after grading (default: False) + - Set via CLI: `--keep-venv` flag + - Useful for debugging and inspecting installed packages + - `is_keeping_existing_venv`: Don't delete existing .venv directories (default: False) + - Set via config file `venv.is_keeping_existing_venv` + - Speeds up repeated grading runs + - `name`: Custom venv directory name (default: `.venv-pygrader`) + - Set via config file `venv.name` + - Prevents conflicts with student's own .venv + +### 4. Grader Orchestration +The `Grader` class (`grader/grader.py`) orchestrates: +1. Configuration loading +2. Virtual environment setup +3. Check execution (in order) +4. Results aggregation +5. Cleanup + +## Development Guidelines + +### Code Style +- **Linting**: pylint (config: `.pylintrc`, `config/*.pylintrc`) +- **Type Hints**: mypy enforced (config: `mypy.ini`) +- **Formatting**: Follow existing patterns +- **Complexity**: Max cyclomatic complexity 15 (complexipy) +- **Line Length**: Follow project conventions + +### Testing +- **Framework**: unittest with coverage (not pytest) +- **Test Runner**: `uv run -m unittest` or `just test` +- **Unit Tests**: `tests/unit/test_*.py` - test individual components +- **Functional Tests**: `tests/functional/test_functional.py` - end-to-end tests +- **Coverage Target**: 85% minimum (enforced by `just coverage`) +- **Coverage Tool**: coverage.py with lcov output + +#### Testing Best Practices +- **Mock External Dependencies**: Use `@patch` decorators to mock external calls + - Mock at the point of use, not at the import location + - Example: Mock `pathlib.Path.exists` if code uses `Path().exists()`, not `os.path.exists` +- **Test CheckResult Fields**: When testing checks, verify all fields in `CheckResult`: + - `name`: Check name + - `result`: Score or pass/fail + - `info`: Expected informational output (not empty string if check provides info) + - `error`: Expected error messages (usually empty string for successful checks) + - `max_score`: Maximum possible score +- **Access Private Methods in Tests**: Use Python name mangling for private methods: + - `self.check_instance._ClassName__private_method()` to test internal logic +- **Run Specific Tests**: + ```bash + # Run specific test class + uv run python -m unittest tests.unit.test_pylint_check.TestPylintCheck -v + + # Run specific test method + uv run python -m unittest tests.unit.test_pylint_check.TestPylintCheck.test_01_pylint_called -v + ``` + +### Adding New Checks +1. Create new check class in `grader/checks/` inheriting from `AbstractCheck` +2. Implement required methods: `run()`, `__str__()`, `__repr__()` +3. Add check type to `ChecksFactory` in `checks_factory.py` +4. Add corresponding tests in `tests/unit/test_.py` +5. Update documentation in `docs/source/` +6. Add configuration examples in `config/` + +### Common Tasks + +**Note**: This project uses [just](https://github.com/casey/just) as a command runner (see `justfile`) and [uv](https://github.com/astral-sh/uv) for Python package management. + +#### Project Setup +```bash +just init # Create venv and install dependencies +``` + +#### Running the Grader Locally +```bash +# Run grader on a project +python pygrader.py --project-root --config + +# Run with verbose mode to see info/error fields from checks +python pygrader.py --project-root --config -v + +# Or with uv +uv run python pygrader.py --project-root --config +``` + +**Verbose Mode** (`-v` or `--verbosity` flag): +- Displays additional `info` and `error` fields from check results +- Shows detailed feedback like coverage percentages, pylint messages, test results +- Works with all output formats (text, JSON, CSV) +- Example output difference: + - Normal: `Check: pylint, Score: 2/2` + - Verbose: `Check: pylint, Score: 2/2. Info: main.py: Missing module docstring. Info: utils.py: Line too long` + +#### Using Docker +```bash +# Build and run via convenience script +./run + +# Or build manually +just build_docker # Syncs uv, locks dependencies, builds Docker image +docker run -v :/project pygrader +``` + +#### Linting and Quality Checks +```bash +just lint # Run all linters (pylint, mypy, flake8, complexipy) + +# Individual linters +uv run pylint grader desktop tests pygrader.py --fail-under 9 +uv run mypy grader desktop tests pygrader.py --ignore-missing-imports +uv run flake8 grader desktop tests pygrader.py +uv run complexipy . +``` + +#### Running Tests +```bash +just test # Run all tests (unit + functional) +just unit_tests # Unit tests only +just functional_tests # Functional tests only + +# Manual test execution +uv run -m unittest discover -s tests/unit -p "test_*.py" -v +uv run -m unittest discover -s tests/functional -p "test_*.py" +``` + +#### Coverage Analysis +```bash +just coverage # Run tests with coverage (85% minimum) + +# Manual coverage +uv run coverage run --source=grader,desktop -m unittest discover -s tests/unit -p "test_*.py" +uv run coverage report -m --fail-under 85 --sort=cover +uv run coverage lcov -o lcov.info # Generate lcov report +``` + +#### Documentation +```bash +just docs # Generate Sphinx documentation + +# Manual documentation build +uv run sphinx-apidoc -o docs/source grader +uv run sphinx-build -b html docs/source docs/build + +# Build PlantUML diagrams +just build_diagrams # Requires plantuml.jar +``` + +#### Sample Project Management +```bash +just setup_sample_project # Clone pygrader-sample-project for testing +just clean_sample_project # Remove sample project +``` + +#### Cleanup +```bash +just clean # Remove build artifacts and logs +just clean_logs # Remove log files only +just clean_venv # Remove virtual environment +``` + +### File Patterns +- **Test files**: `test_*.py` in `tests/unit/` or `tests/functional/` +- **Check implementations**: `*_check.py` in `grader/checks/` +- **Utility modules**: `*.py` in `grader/utils/` +- **Config files**: `*.json` or `*.pylintrc` in `config/` + +## Important Considerations + +### When Making Changes + +1. **Type Safety**: All code must pass mypy type checking +2. **Tests Required**: Add/update tests for any code changes + - When modifying check output (info/error fields), update corresponding tests + - Verify all CheckResult fields match the actual implementation +3. **Documentation**: Update RST docs for new features +4. **Backward Compatibility**: Consider existing config files +5. **Error Handling**: Use proper exception handling and logging +6. **Virtual Environments**: Be aware of isolation requirements + +### Common Pitfalls + +- **Path Handling**: Use absolute paths; project runs in containers +- **Virtual Environment State**: Clean up venvs unless `--keep-venv` specified +- **Config Validation**: Validate config structure early +- **Check Independence**: Checks should not depend on each other's state +- **External Dependencies**: Document any new dependencies in `pyproject.toml` + +### Dependencies +**Build/Package Management**: +- `uv` - Fast Python package installer and resolver +- `just` - Command runner (alternative to make) + +Core dependencies (from `pyproject.toml`): +- `pylint>=4.0.2` - Code linting +- `python-dotenv>=1.2.1` - Environment variable management +- `requests>=2.32.5` - HTTP requests for external resources + +Dev dependencies: +- `coverage>=7.11.3` - Test coverage analysis +- `mypy>=1.18.2` - Type checking +- `flake8>=7.3.0` - Additional linting +- `sphinx>=8.1.3` - Documentation generation +- `complexipy>=4.2.0` - Cyclomatic complexity analysis + +## Debugging Tips + +1. **Logging**: Use `logger` passed to classes; check log output +2. **Virtual Environments**: Use `--keep-venv` flag to inspect created venv +3. **Configuration**: Validate JSON configs with `json_with_templates.py` +4. **Check Results**: Examine `CheckResult` objects for detailed information + - `info` field contains user-facing output (summaries, percentages, etc.) + - `error` field contains error messages if check failed +5. **Test Isolation**: Run single test files to isolate issues +6. **Mock Verification**: When tests fail, verify mocks patch the correct module/method + - Check if code uses `pathlib.Path` vs `os.path` for file operations + - Ensure subprocess mocks return the expected structure (`CompletedProcess` with stdout/stderr) + +## Architecture Patterns + +- **Factory Pattern**: `ChecksFactory` for check creation +- **Template Method**: `AbstractCheck` defines check execution flow +- **Strategy Pattern**: Different checks implement different validation strategies +- **Dependency Injection**: Logger and config injected into classes + +## Quick Reference + +### Key Classes +- `Grader` - Main orchestrator +- `AbstractCheck`, `ScoredCheck`, `NonScoredCheck` - Check base classes +- `ChecksFactory` - Check instantiation +- `VirtualEnvironment` - venv management +- `Config` - Configuration data structure + +### Key Files +- `grader/grader.py` - Entry point for grading logic +- `grader/checks/abstract_check.py` - Check interface definitions +- `grader/utils/config.py` - Configuration loading +- `pygrader.py` - CLI entry point +- `pyproject.toml` - Project metadata and dependencies + +### Testing Philosophy +- Unit tests mock external dependencies (subprocess calls, file system operations, etc.) +- Functional tests use real project structures +- Test both success and failure paths +- Validate error messages and logging +- **CheckResult Verification**: Always verify the complete `CheckResult` structure: + - Tests should match the actual `info` and `error` fields returned by checks + - Most checks populate the `info` field with useful feedback (e.g., "0.5% of the functions have type hints") + - Empty `info` fields are only expected when checks don't provide additional information + +### Common Testing Patterns + +#### Testing Checks with Info Output +```python +# Example: Testing a check that provides info +@patch("grader.utils.process.run") +def test_check_with_info(self, mocked_run): + # Arrange + output = self._create_sample_output(score=5.0) + mocked_run.return_value = CompletedProcess("tool", 0, output) + + # Calculate expected info using the same logic as the check + expected_info = self.check._CheckClass__private_method(output) + expected_result = ScoredCheckResult("check_name", 1, expected_info, "", 2) + + # Act + actual_result = self.check.run() + + # Assert + self.assertEqual(expected_result, actual_result) +``` + +#### Mocking File System Operations +```python +# When code uses pathlib.Path +@patch("pathlib.Path.exists") +def test_file_exists(self, mocked_exists): + mocked_exists.return_value = True + # ... test logic + +# When code uses os.path +@patch("os.path.exists") +def test_file_exists_os(self, mocked_exists): + mocked_exists.return_value = True + # ... test logic +``` + +--- + +**Last Updated**: January 2026 +**For**: Claude Sonnet 4.5 +**Maintained by**: pygrader contributors diff --git a/Dockerfile b/Dockerfile index 9b17241..0ec4171 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,18 +1,17 @@ -FROM ghcr.io/astral-sh/uv:python3.13-slim +FROM ghcr.io/astral-sh/uv:python3.13-trixie-slim RUN mkdir /app RUN mkdir /assets WORKDIR /app -# COPY uv.lock . -# COPY pyproject.toml . -# RUN uv sync --locked --no-install-project --no-dev - COPY . . RUN uv sync --locked --no-dev -VOLUME ["/project"] +COPY entrypoint.sh /app/entrypoint.sh +RUN chmod +x /app/entrypoint.sh + +VOLUME ["/project", "/tmp/project_bind"] -ENTRYPOINT ["uv", "run", "--no-dev", "pygrader.py", "--config", "https://api.github.com/repos/fmipython/PythonCourse2025/contents/homeworks/homework3/pygrader_config_public_web.json", "/project"] +ENTRYPOINT ["/app/entrypoint.sh"] diff --git a/config/projects_2025.json b/config/projects_2025.json new file mode 100644 index 0000000..3fe99a5 --- /dev/null +++ b/config/projects_2025.json @@ -0,0 +1,28 @@ +{ + "checks": [ + { + "name": "requirements", + "max_points": 2, + "is_venv_required": false, + "is_checking_install": true + }, + { + "name": "pylint", + "max_points": 2, + "is_venv_required": true + }, + { + "name": "type-hints", + "max_points": 2, + "is_venv_required": true + }, + { + "name": "coverage", + "max_points": 5, + "is_venv_required": true + } + ], + "venv": { + "is_keeping_existing_venv": true + } +} \ No newline at end of file diff --git a/desktop/main.py b/desktop/main.py index 7e163b1..840a580 100644 --- a/desktop/main.py +++ b/desktop/main.py @@ -4,6 +4,7 @@ """ import os +from pathlib import Path import shutil import grader.utils.constants as const @@ -48,6 +49,17 @@ def run_grader() -> None: if is_path_zip(args["project_root"]): project_root = unzip_archive(args["project_root"]) + + # If the unzipped folder contains only one subfolder (except MACOS subdirectories), use that as the project root + project_root_dir = Path(project_root) + subdirs = [ + directory + for directory in project_root_dir.iterdir() + if directory.is_dir() and directory.name not in const.IGNORE_DIRS + ] + + if len(subdirs) == 1: + project_root = str(subdirs[0]) else: project_root = str(args["project_root"]) # type safety @@ -58,9 +70,10 @@ def run_grader() -> None: checks_results = grader.grade() reporter = build_reporter(args["report_format"]) + verbose = args["verbosity"] >= 1 # TODO - Add output to a file - reporter.display(checks_results) + reporter.display(checks_results, verbose=verbose) if os.path.exists(const.WORK_DIR): shutil.rmtree(const.WORK_DIR) diff --git a/desktop/results_reporter.py b/desktop/results_reporter.py index a4bc866..2b01ec6 100644 --- a/desktop/results_reporter.py +++ b/desktop/results_reporter.py @@ -18,10 +18,11 @@ class ResultsReporter(ABC): """ @abstractmethod - def display(self, results: list[CheckResult], file_descriptor: TextIO = sys.stdout) -> None: + def display(self, results: list[CheckResult], verbose: bool, file_descriptor: TextIO = sys.stdout) -> None: """ Display the results in a specific format. :param results: A list of CheckResult objects to display. + :param verbose: Whether to include info and error fields in the output. :param file_descriptor: The file descriptor to write the output to, defaults to sys.stdout. """ @@ -42,15 +43,15 @@ class JSONResultsReporter(ResultsReporter): This class implements the `display` method to format and print the results in JSON format. """ - def display(self, results: list[CheckResult], file_descriptor: TextIO = sys.stdout) -> None: + def display(self, results: list[CheckResult], verbose: bool, file_descriptor: TextIO = sys.stdout) -> None: scored_results = [result for result in results if isinstance(result, ScoredCheckResult)] total_score = sum(scored_result.result for scored_result in scored_results) total_max_score = sum(result.max_score for result in scored_results) content = { - "scored_checks": [result_to_json(result) for result in scored_results], + "scored_checks": [result_to_json(result, verbose) for result in scored_results], "non_scored_checks": [ - result_to_json(result) for result in results if isinstance(result, NonScoredCheckResult) + result_to_json(result, verbose) for result in results if isinstance(result, NonScoredCheckResult) ], "total_score": total_score, "total_max_score": total_max_score, @@ -61,64 +62,113 @@ def display(self, results: list[CheckResult], file_descriptor: TextIO = sys.stdo self._to_file_descriptor(output, file_descriptor) -def result_to_json(check_result: CheckResult) -> dict: +def result_to_json(check_result: CheckResult, verbose: bool) -> dict: """ Convert a CheckResult to a JSON-compatible dictionary. :param result: The CheckResult to convert. :type result: CheckResult + :param verbose: Whether to include info and error fields. + :type verbose: bool :raises ValueError: If the result is not of type ScoredCheckResult or NonScoredCheckResult. :return: A dictionary representation of the CheckResult. :rtype: dict """ match check_result: - case ScoredCheckResult(name, score, max_score): - return { - "name": name, - "score": score, - "max_score": max_score, - } - case NonScoredCheckResult(name, result): - return { - "name": name, - "result": result, - } + case ScoredCheckResult(): + return scored_result_to_dict(check_result, verbose) + case NonScoredCheckResult(): + return non_scored_result_to_dict(check_result, verbose) case _: raise ValueError("Unknown CheckResult type") +def non_scored_result_to_dict(non_scored_result: NonScoredCheckResult, verbose: bool) -> dict: + """ + Convert a NonScoredCheckResult to a dictionary. + + :param non_scored_result: The NonScoredCheckResult to convert. + :type non_scored_result: NonScoredCheckResult + :param verbose: Whether to include info and error fields. + :type verbose: bool + :return: A dictionary representation of the NonScoredCheckResult. + :rtype: dict + """ + + result_dict = {"name": non_scored_result.name, "result": non_scored_result.result} + if verbose: + if non_scored_result.info: + result_dict["info"] = non_scored_result.info + if non_scored_result.error: + result_dict["error"] = non_scored_result.error + return result_dict + + +def scored_result_to_dict(scored_result: ScoredCheckResult, verbose: bool) -> dict: + """ + Convert a ScoredCheckResult to a dictionary. + + :param scored_result: The ScoredCheckResult to convert. + :type scored_result: ScoredCheckResult + :param verbose: Whether to include info and error fields. + :type verbose: bool + :return: A dictionary representation of the ScoredCheckResult. + :rtype: dict + """ + result_dict = { + "name": scored_result.name, + "score": scored_result.result, + "max_score": scored_result.max_score, + } + if verbose: + if scored_result.info: + result_dict["info"] = scored_result.info + if scored_result.error: + result_dict["error"] = scored_result.error + return result_dict + + class CSVResultsReporter(ResultsReporter): """Concrete class for CSV output. This class implements the `display` method to format and print the results in CSV format. """ - def display(self, results: list[CheckResult], file_descriptor: TextIO = sys.stdout) -> None: + def display(self, results: list[CheckResult], verbose: bool, file_descriptor: TextIO = sys.stdout) -> None: scored_results = [result for result in results if isinstance(result, ScoredCheckResult)] total_score = sum(scored_result.result for scored_result in scored_results) total_max_score = sum(result.max_score for result in scored_results) - output = ["Check,Score,Max Score"] - output += [result_to_csv(check_result) for check_result in results] + if verbose: + output = ["Check,Score,Max Score,Info,Error"] + else: + output = ["Check,Score,Max Score"] + output += [result_to_csv(check_result, verbose) for check_result in results] output.append(f"Total,{total_score},{total_max_score}") self._to_file_descriptor("\n".join(output) + "\n", file_descriptor) -def result_to_csv(check_result: CheckResult) -> str: +def result_to_csv(check_result: CheckResult, verbose: bool) -> str: """ Convert a CheckResult to a CSV-compatible string. :param result: The CheckResult to convert. :type result: CheckResult + :param verbose: Whether to include info and error fields. + :type verbose: bool :raises ValueError: If the result is not of type ScoredCheckResult or NonScoredCheckResult. :return: A CSV-compatible string representation of the CheckResult. :rtype: str """ match check_result: - case ScoredCheckResult(name, score, max_score): + case ScoredCheckResult(name, score, info, error, max_score): + if verbose: + return f"{name},{score},{max_score},{info},{error}" return f"{name},{score},{max_score}" - case NonScoredCheckResult(name, result): + case NonScoredCheckResult(name, result, info, error): + if verbose: + return f"{name},{result},NaN,{info},{error}" return f"{name},{result},NaN" case _: raise ValueError("Unknown CheckResult type") @@ -130,30 +180,70 @@ class PlainTextResultsReporter(ResultsReporter): This class implements the `display` method to format and print the results in plain text format. """ - def display(self, results: list[CheckResult], file_descriptor: TextIO = sys.stdout) -> None: + def display(self, results: list[CheckResult], verbose: bool, file_descriptor: TextIO = sys.stdout) -> None: scored_results = [result for result in results if isinstance(result, ScoredCheckResult)] total_score = sum(scored_result.result for scored_result in scored_results) total_max_score = sum(result.max_score for result in scored_results) - output = [result_to_plain_text(check_result) for check_result in results] + output = [result_to_plain_text(check_result, verbose) for check_result in results] output.append(f"Total Score: {total_score}/{total_max_score}") self._to_file_descriptor("\n".join(output) + "\n", file_descriptor) -def result_to_plain_text(check_result: CheckResult) -> str: +def result_to_plain_text(check_result: CheckResult, verbose: bool) -> str: """ Convert a CheckResult to a plain text string. :param result: The CheckResult to convert. :type result: CheckResult + :param verbose: Whether to include info and error fields. + :type verbose: bool :raises ValueError: If the result is not of type ScoredCheckResult or NonScoredCheckResult. :return: A plain text string representation of the CheckResult. :rtype: str """ match check_result: - case ScoredCheckResult(name, score, max_score): - return f"Check: {name}, Score: {score}/{max_score}" - case NonScoredCheckResult(name, result): - return f"Check: {name}, Result: {result}" + case ScoredCheckResult(): + return scored_result_to_text(check_result, verbose) + case NonScoredCheckResult(): + return non_scored_result_to_text(check_result, verbose) case _: raise ValueError(f"Unknown CheckResult type ({type(check_result)}) for check {check_result.name}") + + +def scored_result_to_text(scored_result: ScoredCheckResult, verbose: bool) -> str: + """ + Convert a ScoredCheckResult to a plain text string. + :param scored_result: The ScoredCheckResult to convert. + :type scored_result: ScoredCheckResult + :param verbose: Whether to include info and error fields. + :type verbose: bool + :return: A plain text string representation of the ScoredCheckResult. + :rtype: str + """ + parts = [f"Check: {scored_result.name}, Score: {scored_result.result}/{scored_result.max_score}"] + if verbose: + if scored_result.info: + parts.append(f"Info: {scored_result.info}") + if scored_result.error: + parts.append(f"Error: {scored_result.error}") + return ". ".join(parts) + + +def non_scored_result_to_text(non_scored_result: NonScoredCheckResult, verbose: bool) -> str: + """ + Convert a NonScoredCheckResult to a plain text string. + :param non_scored_result: The NonScoredCheckResult to convert. + :type non_scored_result: NonScoredCheckResult + :param verbose: Whether to include info and error fields. + :type verbose: bool + :return: A plain text string representation of the NonScoredCheckResult. + :rtype: str + """ + parts = [f"Check: {non_scored_result.name}, Result: {non_scored_result.result}"] + if verbose: + if non_scored_result.info: + parts.append(f"Info: {non_scored_result.info}") + if non_scored_result.error: + parts.append(f"Error: {non_scored_result.error}") + return ". ".join(parts) diff --git a/entrypoint.sh b/entrypoint.sh new file mode 100644 index 0000000..e8bc844 --- /dev/null +++ b/entrypoint.sh @@ -0,0 +1,8 @@ +#!/bin/sh +set -e + +# Copy project files from bind mount to working directory +cp -r /tmp/project_bind/* /project/ + +# Run the grader +exec uv run --no-dev pygrader.py --config ./config/projects_2025.json /project diff --git a/grader/checks/abstract_check.py b/grader/checks/abstract_check.py index ca945f4..6e5d284 100644 --- a/grader/checks/abstract_check.py +++ b/grader/checks/abstract_check.py @@ -25,6 +25,8 @@ class CheckResult(Generic[T]): name: str result: T + info: str + error: str class AbstractCheck(ABC, Generic[T]): diff --git a/grader/checks/coverage_check.py b/grader/checks/coverage_check.py index 35f591f..73a4f1e 100644 --- a/grader/checks/coverage_check.py +++ b/grader/checks/coverage_check.py @@ -53,7 +53,9 @@ def run(self) -> ScoredCheckResult: raise CheckError("Coverage report generation failed") score = self.__translate_score(coverage_report_result) - return ScoredCheckResult(self.name, score, self.max_points) + return ScoredCheckResult( + self.name, score, f"Tests cover {coverage_report_result:.2f}% of the code", "", self.max_points + ) def __translate_score(self, coverage_score: float) -> float: """ @@ -91,8 +93,8 @@ def __coverage_run(self) -> None: raise CheckError("Coverage run failed") from e if output.returncode != 0: - logger.error("Coverage run failed") - raise CheckError("Coverage run failed") + logger.error("Coverage run failed. stdout: %s. stderr: %s", output.stdout, output.stderr) + raise CheckError(f"Coverage run failed: {output.stdout}") def __coverage_report(self) -> int: """ diff --git a/grader/checks/pylint_check.py b/grader/checks/pylint_check.py index e06f875..5734f8b 100644 --- a/grader/checks/pylint_check.py +++ b/grader/checks/pylint_check.py @@ -76,7 +76,9 @@ def run(self) -> ScoredCheckResult: logger.debug("Pylint score: %s", pylint_score) score = self.__translate_score(pylint_score) - return ScoredCheckResult(self.name, score, self.max_points) + short_output = self.__summarize_output(results.stdout) + + return ScoredCheckResult(self.name, score, short_output, "", self.max_points) def __translate_score(self, pylint_score: float) -> float: """ @@ -123,6 +125,26 @@ def __get_pylint_score(self, pylint_output: str) -> float: logger.error("Pylint score not found") raise CheckError("Pylint score not found") + def __summarize_output(self, pylint_output: str) -> str: + """ + Summarize the pylint output to only include the messages. + + :param pylint_output: The output from the pylint check + :return: The summarized output + """ + # TODO: Improve parsing to handle Windows paths (C:\path) and complex pylint output format + # Current implementation may break with multiple colons in paths or line:column:code:message format + summarized_output = [] + for line in pylint_output.strip().split("\n"): + parts = line.split(":") + + if len(parts) < 2: + continue + + file, message = parts[0], parts[-1] + summarized_output.append(f"{file.strip()}: {message.strip()}") + return "\n".join(summarized_output) + class PylintCustomReporter(TextReporter): """ diff --git a/grader/checks/requirements_check.py b/grader/checks/requirements_check.py index 4c55b52..f4d183b 100644 --- a/grader/checks/requirements_check.py +++ b/grader/checks/requirements_check.py @@ -6,8 +6,11 @@ import logging import os +from pathlib import Path + from grader.checks.abstract_check import ScoredCheck, ScoredCheckResult from grader.utils.constants import REQUIREMENTS_FILENAME, PYPROJECT_FILENAME +from grader.utils.virtual_environment import VirtualEnvironment, VirtualEnvironmentError from typing import Optional logger = logging.getLogger("grader") @@ -24,12 +27,14 @@ def __init__( project_root: str, max_points: int, is_venv_required: bool, + is_checking_install: bool = False, env_vars: Optional[dict[str, str]] = None, ): super().__init__(name, max_points, project_root, is_venv_required, env_vars) self.__requirements_path = os.path.join(self._project_root, REQUIREMENTS_FILENAME) self.__pyproject_path = os.path.join(self._project_root, PYPROJECT_FILENAME) + self.__is_checking_install = is_checking_install def run(self) -> ScoredCheckResult: """ @@ -40,10 +45,25 @@ def run(self) -> ScoredCheckResult: """ self._pre_run() - files_to_search = [self.__requirements_path, self.__pyproject_path] + requirements = Path(self.__requirements_path) + pyproject = Path(self.__pyproject_path) + + files_to_search = [requirements, pyproject] - is_one_of_files_present = any(os.path.exists(file_path) for file_path in files_to_search) + is_one_of_files_present = any(file_path.exists() for file_path in files_to_search) score = int(is_one_of_files_present) * self.max_points - return ScoredCheckResult(self.name, score, self.max_points) + info = "" + if not is_one_of_files_present: + info = "requirements.txt or pyproject.toml not found" + + if self.__is_checking_install and is_one_of_files_present: + try: + with VirtualEnvironment(self._project_root, is_keeping_existing_venv=True): + # Context manager automatically handles setup() and teardown() + pass + except VirtualEnvironmentError as e: + return ScoredCheckResult(self.name, 0, "", str(e), self.max_points) + + return ScoredCheckResult(self.name, score, info, "", self.max_points) diff --git a/grader/checks/run_tests_check.py b/grader/checks/run_tests_check.py index 2fb841a..992bde6 100644 --- a/grader/checks/run_tests_check.py +++ b/grader/checks/run_tests_check.py @@ -3,6 +3,7 @@ """ from collections import defaultdict +from dataclasses import dataclass from typing import Optional import logging import os @@ -16,6 +17,18 @@ logger = logging.getLogger("grader") +@dataclass +class TestId: + class_name: str + test_name: str + + def __str__(self) -> str: + return f"{self.class_name}::{self.test_name}" + + def pretty(self, is_passing: bool) -> str: + return f"Test {str(self)} {'passed' if is_passing else 'failed'}." + + class RunTestsCheck(ScoredCheck): """ The tests check class. @@ -83,7 +96,11 @@ def run(self) -> ScoredCheckResult: logger.log(VERBOSE, "Passed tests: %d/%d", len(passed), total_amount) logger.log(VERBOSE, "Failed tests: %d/%d", len(failed), total_amount) - return ScoredCheckResult(self.name, passed_tests_score, self.max_points) + tests_info = [] + tests_info += [test.pretty(True) for test in passed] + tests_info += [test.pretty(False) for test in failed] + + return ScoredCheckResult(self.name, passed_tests_score, "\n".join(tests_info), "", self.max_points) def __pytest_run(self) -> str: """ @@ -122,7 +139,7 @@ def __pytest_run(self) -> str: return output.stdout - def __parse_pytest_output(self, output: str) -> tuple[list[tuple[str, str]], list[tuple[str, str]]]: + def __parse_pytest_output(self, output: str) -> tuple[list[TestId], list[TestId]]: """ Parse the output from pytest to determine passed and failed tests. @@ -139,12 +156,13 @@ def __parse_pytest_output(self, output: str) -> tuple[list[tuple[str, str]], lis if line.startswith("PASSED"): class_name = items[-2] test_name = items[-1] - passed_tests.append((class_name, test_name)) + passed_tests.append(TestId(class_name, test_name)) elif line.startswith("FAILED"): class_name = items[-2] test_name = items[-1].split(" ")[0] # test_06_str_method - AssertionError: ... - logger.log(VERBOSE, f"Test {class_name}::{test_name} failed") - failed_tests.append((class_name, test_name)) + test_id = TestId(class_name, test_name) + logger.log(VERBOSE, "Test %s failed", test_id) + failed_tests.append(test_id) return passed_tests, failed_tests @@ -155,9 +173,7 @@ def _pre_run(self) -> None: path if not is_resource_remote(path) else download_file_from_url(path) for path in self.__tests_path ] - def __calculate_score( - self, passed_tests: list[tuple[str, str]], failed_tests: list[tuple[str, str]] - ) -> tuple[float, float, float]: + def __calculate_score(self, passed_tests: list[TestId], failed_tests: list[TestId]) -> tuple[float, float, float]: """ Calculate the total score based on passed and failed tests. @@ -167,21 +183,15 @@ def __calculate_score( :rtype: float """ - passed_tests_score = sum( - self.__score_test(passed_test_class, passed_test_name) - for passed_test_class, passed_test_name in passed_tests - ) + passed_tests_score = sum(self.__score_test(passed_test) for passed_test in passed_tests) - failed_tests_score = sum( - self.__score_test(failed_test_class, failed_test_name) - for failed_test_class, failed_test_name in failed_tests - ) + failed_tests_score = sum(self.__score_test(failed_test) for failed_test in failed_tests) total_score = passed_tests_score + failed_tests_score return passed_tests_score, failed_tests_score, total_score - def __score_test(self, test_class: str, test_name: str) -> float: + def __score_test(self, test: TestId) -> float: """ Score an individual test based on its class and name. @@ -194,6 +204,8 @@ def __score_test(self, test_class: str, test_name: str) -> float: :param test_name: The name of the test. :return: The score for the test. """ + test_class = test.class_name + test_name = test.test_name if test_class in self.__test_score_mapping and test_name in self.__test_score_mapping: score = self.__test_score_mapping[test_name] diff --git a/grader/checks/structure_check.py b/grader/checks/structure_check.py index 0d5e9f3..5b9d90a 100644 --- a/grader/checks/structure_check.py +++ b/grader/checks/structure_check.py @@ -57,9 +57,9 @@ def run(self) -> NonScoredCheckResult: logger.log(VERBOSE, "Is %s structure valid ? %s", element.name, is_element_valid) if element.required and not is_element_valid: - return NonScoredCheckResult(self.name, False) + return NonScoredCheckResult(self.name, False, f"Structure for '{element.name}' is invalid.", "") - return NonScoredCheckResult(self.name, True) + return NonScoredCheckResult(self.name, True, "Structure is valid", "") @staticmethod def __load_structure_file(filepath: str) -> list[StructureValidator]: diff --git a/grader/checks/type_hints_check.py b/grader/checks/type_hints_check.py index 3f7282d..508387c 100644 --- a/grader/checks/type_hints_check.py +++ b/grader/checks/type_hints_check.py @@ -80,12 +80,19 @@ def run(self) -> ScoredCheckResult: if int(lines_total) == 0: logger.error("Mypy linecount report is empty") - return ScoredCheckResult(self.name, 0, self.max_points) + return ScoredCheckResult(self.name, 0, "", "", self.max_points) # Calculate score - score = self.__translate_score(int(lines_with_type_annotations) / int(lines_total)) - - return ScoredCheckResult(self.name, score, self.max_points) + covered_lines_percentage = int(lines_with_type_annotations) / int(lines_total) + score = self.__translate_score(covered_lines_percentage) + + return ScoredCheckResult( + self.name, + score, + f"{covered_lines_percentage * 100:.2f}% of the functions have type hints", + "", + self.max_points, + ) def __translate_score(self, mypy_score: float) -> float: """ diff --git a/grader/grader.py b/grader/grader.py index e2835f0..4765182 100644 --- a/grader/grader.py +++ b/grader/grader.py @@ -43,6 +43,7 @@ def __init__( self.__is_skipping_venv_creation = is_skipping_venv_creation try: self.__config = load_config(config_path) + self.__logger.debug(f"Config contents: {self.__config}") except InvalidConfigError as exc: self.__logger.error("Error with the configuration file") self.__logger.exception(exc) @@ -75,7 +76,9 @@ def grade(self) -> list[CheckResult]: if self.__is_skipping_venv_creation or len(venv_checks) == 0: return scores - with VirtualEnvironment(self.__project_root, self.__is_keeping_venv): + venv_config = self.__config.get("venv", {}) + + with VirtualEnvironment(self.__project_root, is_keeping_venv_after_run=self.__is_keeping_venv, **venv_config): scores += [self.__run_check(check) for check in venv_checks] self.__cleanup() @@ -95,11 +98,12 @@ def __run_check(self, check: AbstractCheck) -> CheckResult: except CheckError as error: self.__logger.error("Check failed: %s", error) + # TODO - Pass the information from the exception match check: case ScoredCheck(): - check_result = ScoredCheckResult(check.name, 0, check.max_points) + check_result = ScoredCheckResult(check.name, 0, "", str(error), check.max_points) case NonScoredCheck(): - check_result = NonScoredCheckResult(check.name, False) + check_result = NonScoredCheckResult(check.name, False, "", str(error)) case _: raise TypeError(f"Unknown check type: {type(check)}") from error diff --git a/grader/utils/constants.py b/grader/utils/constants.py index ae844f3..3f27f89 100644 --- a/grader/utils/constants.py +++ b/grader/utils/constants.py @@ -26,8 +26,8 @@ # Virtual environment constants REQUIREMENTS_FILENAME = "requirements.txt" PYPROJECT_FILENAME = "pyproject.toml" -VENV_NAME = ".venv" -POSSIBLE_VENV_DIRS = ["venv", ".venv"] +VENV_NAME = ".venv-pygrader" +POSSIBLE_VENV_DIRS = ["venv", ".venv", ".venv-pygrader"] PIP_PATH_WINDOWS = os.path.join("Scripts", "pip.exe") PIP_PATH_UNIX = os.path.join("bin", "pip") @@ -52,7 +52,7 @@ PYLINT_BIN_UNIX = os.path.join("bin", "pylint") PYLINT_BIN = PYLINT_BIN_WINDOWS if os.name == "nt" else PYLINT_BIN_UNIX PYLINT_PATH = os.path.join(VENV_NAME, PYLINT_BIN) -PYLINTRC = os.path.join(CONFIG_DIR, "2025-hw3.pylintrc") +PYLINTRC = os.path.join(CONFIG_DIR, "2024.pylintrc") # Pytest constants PYTEST_BIN_WINDOWS = "pytest.exe" @@ -92,6 +92,7 @@ "__pycache__", ".pytest_cache", "_MACOSX", + "__MACOSX", os.path.join("build", "lib"), *POSSIBLE_VENV_DIRS, ] diff --git a/grader/utils/virtual_environment.py b/grader/utils/virtual_environment.py index c120f03..16a90c3 100644 --- a/grader/utils/virtual_environment.py +++ b/grader/utils/virtual_environment.py @@ -22,10 +22,18 @@ class VirtualEnvironment: is_initialized = False - def __init__(self, project_path: str, keep_venv: bool = False): + def __init__( + self, + project_path: str, + is_keeping_venv_after_run: bool = False, + is_keeping_existing_venv: bool = False, + name: str = const.VENV_NAME, + ): self._project_path = project_path - self._venv_path = os.path.join(project_path, const.VENV_NAME) - self.__keep_venv = keep_venv + # TODO - To fully allow for a custom venv name, we need to rethink how we handle paths in the constants + self._venv_path = os.path.join(project_path, name) + self.__is_keeping_venv_after_run = is_keeping_venv_after_run + self.__is_keeping_existing_venv = is_keeping_existing_venv def __enter__(self) -> VirtualEnvironment: self.setup() @@ -46,12 +54,8 @@ def setup(self) -> None: Install the grader dependencies as well. """ # Check for existing venv - possible_venv_paths = [os.path.join(self._project_path, venv_path) for venv_path in const.POSSIBLE_VENV_DIRS] - - for path in possible_venv_paths: - if os.path.exists(path): - logger.log(VERBOSE, "Found existing venv at %s", path) - shutil.rmtree(path) + if not self.__is_keeping_existing_venv: + self.__remove_existing_venv() # Check for requirements.txt @@ -86,12 +90,23 @@ def setup(self) -> None: grader_requirements_path = const.GRADER_REQUIREMENTS VirtualEnvironment.__install_requirements(self._venv_path, grader_requirements_path) + def __remove_existing_venv(self) -> None: + """ + Remove any existing virtual environment in the project directory. + """ + possible_venv_paths = [os.path.join(self._project_path, venv_path) for venv_path in const.POSSIBLE_VENV_DIRS] + + for path in possible_venv_paths: + if os.path.exists(path): + logger.log(VERBOSE, "Found existing venv at %s", path) + shutil.rmtree(path) + def teardown(self) -> None: """ Delete the virtual environment. """ - logger.debug(self.__keep_venv) - if not self.__keep_venv: + logger.debug(self.__is_keeping_venv_after_run) + if not self.__is_keeping_venv_after_run: shutil.rmtree(self._venv_path) @staticmethod diff --git a/justfile b/justfile index 1b3d018..467bbcb 100644 --- a/justfile +++ b/justfile @@ -17,13 +17,13 @@ lint: test: unit_tests functional_tests unit_tests: - find tests -type f -name "test_*.py" -not -name "test_functional.py" -not -path "*sample_project*" | xargs uv run -m unittest -v + uv run -m unittest discover -s tests/unit -p "test_*.py" -v functional_tests: - uv run -m unittest discover -s tests -p "test_functional.py" + uv run -m unittest discover -s tests/functional -p "test_*.py" coverage: - find tests -type f -name "test_*.py" -not -name "test_functional.py" | xargs uv run coverage run --source={{packages}} -m unittest + uv run coverage run --source={{packages}} -m unittest discover -s tests/unit -p "test_*.py" uv run coverage lcov -o lcov.info uv run coverage report -m --fail-under 85 --sort=cover @@ -39,6 +39,9 @@ clean: clean_logs rm -rf docs/build rm -f lcov.info rm -rf "pygrader-sample-project" + rm -rf __pycache__ + rm -rf .complexipy_cache + rm -rf pygrader.egg-info clean_logs: rm -rf *.log.* diff --git a/pyproject.toml b/pyproject.toml index 8304584..55db970 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "pygrader" -version = "1.7.1" +version = "1.8.0" description = "Add your description here" readme = "README.md" requires-python = ">=3.10" diff --git a/tests/functional/__init__.py b/tests/functional/__init__.py new file mode 100644 index 0000000..10ba5ea --- /dev/null +++ b/tests/functional/__init__.py @@ -0,0 +1,3 @@ +""" +Functional tests for the grader. +""" diff --git a/tests/test_functional.py b/tests/functional/test_functional.py similarity index 99% rename from tests/test_functional.py rename to tests/functional/test_functional.py index 9a97387..f4be7d0 100644 --- a/tests/test_functional.py +++ b/tests/functional/test_functional.py @@ -41,7 +41,7 @@ def setUp(self) -> None: raise RuntimeError(f"Failed to checkout branch {current_branch}: {checkout_result.stderr}") # Remove the functional tests from the repo, as they cause issues and time loss. - functional_tests_path = os.path.join(self.clone_path, "tests", "test_functional.py") + functional_tests_path = os.path.join(self.clone_path, "tests", "functional", "test_functional.py") if os.path.exists(functional_tests_path): os.remove(functional_tests_path) diff --git a/tests/test_requirements_check.py b/tests/test_requirements_check.py deleted file mode 100644 index bcdf3e5..0000000 --- a/tests/test_requirements_check.py +++ /dev/null @@ -1,52 +0,0 @@ -""" -Unit tests for the RequirementsCheck class in the requirements_check module. -""" - -import unittest -from unittest.mock import patch, MagicMock - -from grader.checks.requirements_check import RequirementsCheck -from grader.checks.abstract_check import ScoredCheckResult - - -class TestRequirementsCheck(unittest.TestCase): - """ - Unit tests for the RequirementsCheck class. - """ - - def setUp(self) -> None: - """ - Set up the test case environment. - """ - self.requirements_check = RequirementsCheck("requirements", "sample_dir", 1, is_venv_required=False) - return super().setUp() - - @patch("os.path.exists") - def test_01_requirements_exist(self, mocked_exists: MagicMock) -> None: - """ - Test that the requirements file exists. - """ - # Arrange - mocked_exists.return_value = True - expected_score = ScoredCheckResult(self.requirements_check.name, 1, self.requirements_check.max_points) - - # Act - actual_score = self.requirements_check.run() - - # Assert - self.assertEqual(expected_score, actual_score) - - @patch("os.path.exists") - def test_02_requirements_does_not_exist(self, mocked_exists: MagicMock) -> None: - """ - Test that the requirements file does not exist. - """ - # Arrange - mocked_exists.return_value = False - expected_score = ScoredCheckResult(self.requirements_check.name, 0, self.requirements_check.max_points) - - # Act - actual_score = self.requirements_check.run() - - # Assert - self.assertEqual(expected_score, actual_score) diff --git a/tests/unit/__init__.py b/tests/unit/__init__.py new file mode 100644 index 0000000..f7468cb --- /dev/null +++ b/tests/unit/__init__.py @@ -0,0 +1,3 @@ +""" +Unit tests for the grader. +""" diff --git a/tests/test_abstract_check.py b/tests/unit/test_abstract_check.py similarity index 94% rename from tests/test_abstract_check.py rename to tests/unit/test_abstract_check.py index bd478d3..8830b19 100644 --- a/tests/test_abstract_check.py +++ b/tests/unit/test_abstract_check.py @@ -28,7 +28,7 @@ def run(self) -> CheckResult[int]: Run the dummy check. """ super()._pre_run() - return CheckResult("dummy", 0) + return CheckResult("dummy", 0, "", "") check = DummyCheck("dummy", "dummy", is_venv_required=True) diff --git a/tests/test_checks_factory.py b/tests/unit/test_checks_factory.py similarity index 100% rename from tests/test_checks_factory.py rename to tests/unit/test_checks_factory.py diff --git a/tests/test_cli.py b/tests/unit/test_cli.py similarity index 100% rename from tests/test_cli.py rename to tests/unit/test_cli.py diff --git a/tests/test_config.py b/tests/unit/test_config.py similarity index 100% rename from tests/test_config.py rename to tests/unit/test_config.py diff --git a/tests/test_coverage_check.py b/tests/unit/test_coverage_check.py similarity index 89% rename from tests/test_coverage_check.py rename to tests/unit/test_coverage_check.py index 8a4e1ba..234a372 100644 --- a/tests/test_coverage_check.py +++ b/tests/unit/test_coverage_check.py @@ -36,7 +36,7 @@ def test_01_coverage_run_fail(self, mocked_run: MagicMock) -> None: with self.assertLogs("grader", level="ERROR") as log: with self.assertRaises(CheckError): self.coverage_check.run() - is_message_logged = "ERROR:grader:Coverage run failed" in log.output + is_message_logged = "ERROR:grader:Coverage run failed. stdout:" in log.output[0] # Assert self.assertTrue(is_message_logged) @@ -74,7 +74,7 @@ def test_03_translate_score_zero(self, mocked_report: MagicMock, mocked_run: Mag # Arrange mocked_run.return_value = True mocked_report.return_value = 0 - expected_score = ScoredCheckResult("Coverage", 0, 2) + expected_score = ScoredCheckResult("Coverage", 0, "Tests cover 0.00% of the code", "", 2) # Act actual_score = self.coverage_check.run() @@ -91,7 +91,7 @@ def test_04_translate_score_inside_first_range(self, mocked_report: MagicMock, m # Arrange mocked_run.return_value = True mocked_report.return_value = 22 - expected_score = ScoredCheckResult("Coverage", 0, 2) + expected_score = ScoredCheckResult("Coverage", 0, "Tests cover 22.00% of the code", "", 2) # Act actual_score = self.coverage_check.run() @@ -108,7 +108,7 @@ def test_05_translate_score_right_bound_first_range(self, mocked_report: MagicMo # Arrange mocked_run.return_value = True mocked_report.return_value = 100 / 3 - expected_score = ScoredCheckResult("Coverage", 1, 2) + expected_score = ScoredCheckResult("Coverage", 1, "Tests cover 33.33% of the code", "", 2) # Act actual_score = self.coverage_check.run() @@ -125,7 +125,7 @@ def test_06_translate_score_left_bound_second_range(self, mocked_report: MagicMo # Arrange mocked_run.return_value = True mocked_report.return_value = 100 / 3 + 1 - expected_score = ScoredCheckResult("Coverage", 1, 2) + expected_score = ScoredCheckResult("Coverage", 1, "Tests cover 34.33% of the code", "", 2) # Act actual_score = self.coverage_check.run() @@ -144,7 +144,7 @@ def test_07_translate_score_inside_bound_second_range( # Arrange mocked_run.return_value = True mocked_report.return_value = 50 - expected_score = ScoredCheckResult("Coverage", 1, 2) + expected_score = ScoredCheckResult("Coverage", 1, "Tests cover 50.00% of the code", "", 2) # Act actual_score = self.coverage_check.run() @@ -163,7 +163,7 @@ def test_08_translate_score_right_bound_second_range( # Arrange mocked_run.return_value = True mocked_report.return_value = 100 / 3 * 2 - expected_score = ScoredCheckResult("Coverage", 2, 2) + expected_score = ScoredCheckResult("Coverage", 2, "Tests cover 66.67% of the code", "", 2) # Act actual_score = self.coverage_check.run() @@ -182,7 +182,7 @@ def test_09_translate_score_inside_bound_third_range( # Arrange mocked_run.return_value = True mocked_report.return_value = 75 - expected_score = ScoredCheckResult("Coverage", 2, 2) + expected_score = ScoredCheckResult("Coverage", 2, "Tests cover 75.00% of the code", "", 2) # Act actual_score = self.coverage_check.run() @@ -199,7 +199,7 @@ def test_10_translate_score_max(self, mocked_report: MagicMock, mocked_run: Magi # Arrange mocked_run.return_value = True mocked_report.return_value = 100 - expected_score = ScoredCheckResult("Coverage", 2, 2) + expected_score = ScoredCheckResult("Coverage", 2, "Tests cover 100.00% of the code", "", 2) # Act actual_score = self.coverage_check.run() @@ -226,4 +226,4 @@ def mocked_run_side_effect(*args: list, **_: dict) -> Optional[CompletedProcess] result = self.coverage_check.run() # Assert - self.assertEqual(ScoredCheckResult("Coverage", 2, 2), result) + self.assertEqual(ScoredCheckResult("Coverage", 2, "Tests cover 100.00% of the code", "", 2), result) diff --git a/tests/test_environment.py b/tests/unit/test_environment.py similarity index 100% rename from tests/test_environment.py rename to tests/unit/test_environment.py diff --git a/tests/test_external_resources.py b/tests/unit/test_external_resources.py similarity index 100% rename from tests/test_external_resources.py rename to tests/unit/test_external_resources.py diff --git a/tests/test_files.py b/tests/unit/test_files.py similarity index 100% rename from tests/test_files.py rename to tests/unit/test_files.py diff --git a/tests/test_grader.py b/tests/unit/test_grader.py similarity index 100% rename from tests/test_grader.py rename to tests/unit/test_grader.py diff --git a/tests/test_json_with_templates.py b/tests/unit/test_json_with_templates.py similarity index 100% rename from tests/test_json_with_templates.py rename to tests/unit/test_json_with_templates.py diff --git a/tests/test_main.py b/tests/unit/test_main.py similarity index 100% rename from tests/test_main.py rename to tests/unit/test_main.py diff --git a/tests/test_process.py b/tests/unit/test_process.py similarity index 100% rename from tests/test_process.py rename to tests/unit/test_process.py diff --git a/tests/test_pylint_check.py b/tests/unit/test_pylint_check.py similarity index 84% rename from tests/test_pylint_check.py rename to tests/unit/test_pylint_check.py index 7f8c941..39c3791 100644 --- a/tests/test_pylint_check.py +++ b/tests/unit/test_pylint_check.py @@ -8,7 +8,7 @@ import grader.utils.constants as const from grader.checks.pylint_check import PylintCheck -from grader.checks.abstract_check import CheckError, ScoredCheckResult +from grader.checks.abstract_check import CheckError class TestPylintCheck(unittest.TestCase): @@ -111,14 +111,16 @@ def test_04_translate_score_zero(self, mocked_pylint: MagicMock) -> None: :type mocked_pylint: MagicMock """ # Arrange - mocked_pylint.return_value = CompletedProcess("pylint", 0, self.__create_sample_pylint_output(0)) - expected_score = ScoredCheckResult(self.pylint_check.name, 0, self.pylint_check.max_points) + pylint_output = self.__create_sample_pylint_output(5) + mocked_pylint.return_value = CompletedProcess("pylint", 0, pylint_output) # Act actual_score = self.pylint_check.run() # Assert - self.assertEqual(expected_score, actual_score) + self.assertEqual(1, actual_score.result) + self.assertEqual(self.pylint_check.name, actual_score.name) + self.assertEqual(self.pylint_check.max_points, actual_score.max_score) @patch("grader.utils.process.run") def test_05_translate_score_inside_first_range(self, mocked_pylint: MagicMock) -> None: @@ -129,14 +131,16 @@ def test_05_translate_score_inside_first_range(self, mocked_pylint: MagicMock) - :type mocked_pylint: MagicMock """ # Arrange - mocked_pylint.return_value = CompletedProcess("pylint", 0, self.__create_sample_pylint_output(2.2)) - expected_score = ScoredCheckResult(self.pylint_check.name, 0, self.pylint_check.max_points) + pylint_output = self.__create_sample_pylint_output(2.2) + mocked_pylint.return_value = CompletedProcess("pylint", 0, pylint_output) # Act actual_score = self.pylint_check.run() # Assert - self.assertEqual(expected_score, actual_score) + self.assertEqual(0, actual_score.result) + self.assertEqual(self.pylint_check.name, actual_score.name) + self.assertEqual(self.pylint_check.max_points, actual_score.max_score) @patch("grader.utils.process.run") def test_06_translate_score_right_bound_first_range(self, mocked_pylint: MagicMock) -> None: @@ -149,13 +153,14 @@ def test_06_translate_score_right_bound_first_range(self, mocked_pylint: MagicMo # Arrange mocked_pylint_stdout = self.__create_sample_pylint_output(10 / 3) mocked_pylint.return_value = CompletedProcess("pylint", 0, mocked_pylint_stdout) - expected_score = ScoredCheckResult(self.pylint_check.name, 1, self.pylint_check.max_points) # Act actual_score = self.pylint_check.run() # Assert - self.assertEqual(expected_score, actual_score) + self.assertEqual(1, actual_score.result) + self.assertEqual(self.pylint_check.name, actual_score.name) + self.assertEqual(self.pylint_check.max_points, actual_score.max_score) @patch("grader.utils.process.run") def test_07_translate_score_left_bound_second_range(self, mocked_pylint: MagicMock) -> None: @@ -166,14 +171,16 @@ def test_07_translate_score_left_bound_second_range(self, mocked_pylint: MagicMo :type mocked_pylint: MagicMock """ # Arrange - mocked_pylint.return_value = CompletedProcess("pylint", 0, self.__create_sample_pylint_output(10 / 3 + 0.1)) - expected_score = ScoredCheckResult(self.pylint_check.name, 1, self.pylint_check.max_points) + pylint_output = self.__create_sample_pylint_output(10 / 3 + 0.1) + mocked_pylint.return_value = CompletedProcess("pylint", 0, pylint_output) # Act actual_score = self.pylint_check.run() # Assert - self.assertEqual(expected_score, actual_score) + self.assertEqual(1, actual_score.result) + self.assertEqual(self.pylint_check.name, actual_score.name) + self.assertEqual(self.pylint_check.max_points, actual_score.max_score) @patch("grader.utils.process.run") def test_08_translate_score_inside_bound_second_range(self, mocked_pylint: MagicMock) -> None: @@ -184,14 +191,16 @@ def test_08_translate_score_inside_bound_second_range(self, mocked_pylint: Magic :type mocked_pylint: MagicMock """ # Arrange - mocked_pylint.return_value = CompletedProcess("pylint", 0, self.__create_sample_pylint_output(5)) - expected_score = ScoredCheckResult(self.pylint_check.name, 1, self.pylint_check.max_points) + pylint_output = self.__create_sample_pylint_output(5) + mocked_pylint.return_value = CompletedProcess("pylint", 0, pylint_output) # Act actual_score = self.pylint_check.run() # Assert - self.assertEqual(expected_score, actual_score) + self.assertEqual(1, actual_score.result) + self.assertEqual(self.pylint_check.name, actual_score.name) + self.assertEqual(self.pylint_check.max_points, actual_score.max_score) @patch("grader.utils.process.run") def test_09_translate_score_right_bound_second_range(self, mocked_pylint: MagicMock) -> None: @@ -202,14 +211,16 @@ def test_09_translate_score_right_bound_second_range(self, mocked_pylint: MagicM :type mocked_pylint: MagicMock """ # Arrange - mocked_pylint.return_value = CompletedProcess("pylint", 0, self.__create_sample_pylint_output(10 / 3 * 2)) - expected_score = ScoredCheckResult(self.pylint_check.name, 2, self.pylint_check.max_points) + pylint_output = self.__create_sample_pylint_output(10 / 3 * 2) + mocked_pylint.return_value = CompletedProcess("pylint", 0, pylint_output) # Act actual_score = self.pylint_check.run() # Assert - self.assertEqual(expected_score, actual_score) + self.assertEqual(2, actual_score.result) + self.assertEqual(self.pylint_check.name, actual_score.name) + self.assertEqual(self.pylint_check.max_points, actual_score.max_score) @patch("grader.utils.process.run") def test_10_translate_score_inside_bound_third_range(self, mocked_pylint: MagicMock) -> None: @@ -220,14 +231,16 @@ def test_10_translate_score_inside_bound_third_range(self, mocked_pylint: MagicM :type mocked_pylint: MagicMock """ # Arrange - mocked_pylint.return_value = CompletedProcess("pylint", 0, self.__create_sample_pylint_output(7.5)) - expected_score = ScoredCheckResult(self.pylint_check.name, 2, self.pylint_check.max_points) + pylint_output = self.__create_sample_pylint_output(7.5) + mocked_pylint.return_value = CompletedProcess("pylint", 0, pylint_output) # Act actual_score = self.pylint_check.run() # Assert - self.assertEqual(expected_score, actual_score) + self.assertEqual(2, actual_score.result) + self.assertEqual(self.pylint_check.name, actual_score.name) + self.assertEqual(self.pylint_check.max_points, actual_score.max_score) @patch("grader.utils.process.run") def test_11_translate_score_max(self, mocked_pylint: MagicMock) -> None: @@ -238,14 +251,16 @@ def test_11_translate_score_max(self, mocked_pylint: MagicMock) -> None: :type mocked_pylint: MagicMock """ # Arrange - mocked_pylint.return_value = CompletedProcess("pylint", 0, self.__create_sample_pylint_output(10)) - expected_score = ScoredCheckResult(self.pylint_check.name, 2, self.pylint_check.max_points) + pylint_output = self.__create_sample_pylint_output(10) + mocked_pylint.return_value = CompletedProcess("pylint", 0, pylint_output) # Act actual_score = self.pylint_check.run() # Assert - self.assertEqual(expected_score, actual_score) + self.assertEqual(2, actual_score.result) + self.assertEqual(self.pylint_check.name, actual_score.name) + self.assertEqual(self.pylint_check.max_points, actual_score.max_score) @patch("grader.utils.files.find_all_python_files") def test_12_find_all_python_files_raises_os_error(self, mocked_find: MagicMock) -> None: diff --git a/tests/unit/test_requirements_check.py b/tests/unit/test_requirements_check.py new file mode 100644 index 0000000..37f2f4e --- /dev/null +++ b/tests/unit/test_requirements_check.py @@ -0,0 +1,173 @@ +""" +Unit tests for the RequirementsCheck class in the requirements_check module. +""" + +import unittest +from unittest.mock import patch, MagicMock + +from grader.checks.requirements_check import RequirementsCheck +from grader.checks.abstract_check import ScoredCheckResult +from grader.utils.virtual_environment import VirtualEnvironmentError + + +class TestRequirementsCheck(unittest.TestCase): + """ + Unit tests for the RequirementsCheck class. + """ + + def setUp(self) -> None: + """ + Set up the test case environment. + """ + self.requirements_check = RequirementsCheck("requirements", "sample_dir", 1, is_venv_required=False) + return super().setUp() + + @patch("pathlib.Path.exists") + def test_01_requirements_exist(self, mocked_exists: MagicMock) -> None: + """ + Test that the requirements file exists. + """ + # Arrange + mocked_exists.return_value = True + expected_score = ScoredCheckResult(self.requirements_check.name, 1, "", "", self.requirements_check.max_points) + + # Act + actual_score = self.requirements_check.run() + + # Assert + self.assertEqual(expected_score, actual_score) + + @patch("pathlib.Path.exists") + def test_02_requirements_does_not_exist(self, mocked_exists: MagicMock) -> None: + """ + Test that the requirements file does not exist. + """ + # Arrange + mocked_exists.return_value = False + expected_score = ScoredCheckResult( + self.requirements_check.name, + 0, + "requirements.txt or pyproject.toml not found", + "", + self.requirements_check.max_points, + ) + + # Act + actual_score = self.requirements_check.run() + + # Assert + self.assertEqual(expected_score, actual_score) + + @patch("grader.checks.requirements_check.VirtualEnvironment") + @patch("pathlib.Path.exists") + def test_03_is_checking_install_with_successful_venv_setup( + self, mocked_exists: MagicMock, mocked_venv_class: MagicMock + ) -> None: + """ + Test that when is_checking_install=True and requirements exist, + a virtual environment is created, set up, and torn down successfully. + """ + # Arrange + mocked_exists.return_value = True + mocked_venv_instance = MagicMock() + mocked_venv_class.return_value = mocked_venv_instance + + requirements_check = RequirementsCheck( + "requirements", "sample_dir", 1, is_venv_required=False, is_checking_install=True + ) + expected_score = ScoredCheckResult(requirements_check.name, 1, "", "", requirements_check.max_points) + + # Act + actual_score = requirements_check.run() + + # Assert + self.assertEqual(expected_score, actual_score) + mocked_venv_class.assert_called_once_with("sample_dir", is_keeping_existing_venv=True) + mocked_venv_instance.setup.assert_called_once() + mocked_venv_instance.teardown.assert_called_once() + + @patch("grader.checks.requirements_check.VirtualEnvironment") + @patch("pathlib.Path.exists") + def test_04_is_checking_install_with_failed_venv_setup( + self, mocked_exists: MagicMock, mocked_venv_class: MagicMock + ) -> None: + """ + Test that when is_checking_install=True and venv setup fails, + the check returns 0 score with the error message. + """ + # Arrange + mocked_exists.return_value = True + error_message = "Failed to install dependencies" + mocked_venv_instance = MagicMock() + mocked_venv_instance.setup.side_effect = VirtualEnvironmentError(error_message) + mocked_venv_class.return_value = mocked_venv_instance + + requirements_check = RequirementsCheck( + "requirements", "sample_dir", 1, is_venv_required=False, is_checking_install=True + ) + expected_score = ScoredCheckResult( + requirements_check.name, 0, "", error_message, requirements_check.max_points + ) + + # Act + actual_score = requirements_check.run() + + # Assert + self.assertEqual(expected_score, actual_score) + mocked_venv_class.assert_called_once_with("sample_dir", is_keeping_existing_venv=True) + mocked_venv_instance.setup.assert_called_once() + mocked_venv_instance.teardown.assert_not_called() + + @patch("grader.checks.requirements_check.VirtualEnvironment") + @patch("pathlib.Path.exists") + def test_05_is_checking_install_false_does_not_create_venv( + self, mocked_exists: MagicMock, mocked_venv_class: MagicMock + ) -> None: + """ + Test that when is_checking_install=False, no virtual environment is created + even if requirements file exists. + """ + # Arrange + mocked_exists.return_value = True + + requirements_check = RequirementsCheck( + "requirements", "sample_dir", 1, is_venv_required=False, is_checking_install=False + ) + expected_score = ScoredCheckResult(requirements_check.name, 1, "", "", requirements_check.max_points) + + # Act + actual_score = requirements_check.run() + + # Assert + self.assertEqual(expected_score, actual_score) + mocked_venv_class.assert_not_called() + + @patch("grader.checks.requirements_check.VirtualEnvironment") + @patch("pathlib.Path.exists") + def test_06_is_checking_install_true_but_no_requirements_file( + self, mocked_exists: MagicMock, mocked_venv_class: MagicMock + ) -> None: + """ + Test that when is_checking_install=True but no requirements file exists, + no virtual environment is created. + """ + # Arrange + mocked_exists.return_value = False + + requirements_check = RequirementsCheck( + "requirements", "sample_dir", 1, is_venv_required=False, is_checking_install=True + ) + expected_score = ScoredCheckResult( + requirements_check.name, + 0, + "requirements.txt or pyproject.toml not found", + "", + requirements_check.max_points, + ) + + # Act + actual_score = requirements_check.run() + + # Assert + self.assertEqual(expected_score, actual_score) + mocked_venv_class.assert_not_called() diff --git a/tests/test_structure_check.py b/tests/unit/test_structure_check.py similarity index 93% rename from tests/test_structure_check.py rename to tests/unit/test_structure_check.py index 690760d..8ea7fff 100644 --- a/tests/test_structure_check.py +++ b/tests/unit/test_structure_check.py @@ -33,7 +33,7 @@ def test_01_valid_structure(self, mock_load_structure_file: MagicMock) -> None: mock_element.is_structure_valid.return_value = True mock_element.required = True mock_load_structure_file.return_value = [mock_element] - expected = NonScoredCheckResult(self.structure_check.name, True) + expected = NonScoredCheckResult(self.structure_check.name, True, "Structure is valid", "") # Act result = self.structure_check.run() @@ -48,10 +48,14 @@ def test_02_invalid_required_structure(self, mock_load_structure_file: MagicMock """ # Arrange mock_element = MagicMock() + expected_element_name = "foo" + mock_element.name = expected_element_name mock_element.is_structure_valid.return_value = False mock_element.required = True mock_load_structure_file.return_value = [mock_element] - expected = NonScoredCheckResult(self.structure_check.name, False) + + expected_info = f"Structure for '{expected_element_name}' is invalid." + expected = NonScoredCheckResult(self.structure_check.name, False, expected_info, "") # Act result = self.structure_check.run() @@ -69,7 +73,8 @@ def test_03_invalid_non_required_structure(self, mock_load_structure_file: Magic mock_element.is_structure_valid.return_value = False mock_element.required = False mock_load_structure_file.return_value = [mock_element] - expected = NonScoredCheckResult(self.structure_check.name, True) + + expected = NonScoredCheckResult(self.structure_check.name, True, "Structure is valid", "") # Act result = self.structure_check.run() @@ -84,7 +89,7 @@ def test_04_empty_structure_file(self, mock_load_structure_file: MagicMock) -> N """ # Arrange mock_load_structure_file.return_value = [] - expected = NonScoredCheckResult(self.structure_check.name, True) + expected = NonScoredCheckResult(self.structure_check.name, True, "Structure is valid", "") # Act result = self.structure_check.run() @@ -139,7 +144,7 @@ def test_07_multiple_valid_elements(self, mock_load_structure_file: MagicMock) - mock_element2.required = False mock_load_structure_file.return_value = [mock_element1, mock_element2] - expected = NonScoredCheckResult(self.structure_check.name, True) + expected = NonScoredCheckResult(self.structure_check.name, True, "Structure is valid", "") # Act result = self.structure_check.run() @@ -158,11 +163,14 @@ def test_08_multiple_elements_with_invalid_required(self, mock_load_structure_fi mock_element1.required = True mock_element2 = MagicMock() + expected_element_name = "bar" + mock_element2.name = expected_element_name + expected_info = f"Structure for '{expected_element_name}' is invalid." mock_element2.is_structure_valid.return_value = False mock_element2.required = True mock_load_structure_file.return_value = [mock_element1, mock_element2] - expected = NonScoredCheckResult(self.structure_check.name, False) + expected = NonScoredCheckResult(self.structure_check.name, False, expected_info, "") # Act result = self.structure_check.run() @@ -185,7 +193,7 @@ def test_09_multiple_elements_with_invalid_non_required(self, mock_load_structur mock_element2.required = False mock_load_structure_file.return_value = [mock_element1, mock_element2] - expected = NonScoredCheckResult(self.structure_check.name, True) + expected = NonScoredCheckResult(self.structure_check.name, True, "Structure is valid", "") # Act result = self.structure_check.run() @@ -200,15 +208,21 @@ def test_10_all_invalid_elements(self, mock_load_structure_file: MagicMock) -> N """ # Arrange mock_element1 = MagicMock() + expected_element1_name = "foo" + mock_element1.name = expected_element1_name mock_element1.is_structure_valid.return_value = False mock_element1.required = True mock_element2 = MagicMock() + expected_element2_name = "bar" + mock_element2.name = expected_element2_name mock_element2.is_structure_valid.return_value = False mock_element2.required = False mock_load_structure_file.return_value = [mock_element1, mock_element2] - expected = NonScoredCheckResult(self.structure_check.name, False) + + expected_info = f"Structure for '{expected_element1_name}' is invalid." + expected = NonScoredCheckResult(self.structure_check.name, False, expected_info, "") # Act result = self.structure_check.run() diff --git a/tests/test_structure_validator.py b/tests/unit/test_structure_validator.py similarity index 100% rename from tests/test_structure_validator.py rename to tests/unit/test_structure_validator.py diff --git a/tests/test_tests_check.py b/tests/unit/test_tests_check.py similarity index 87% rename from tests/test_tests_check.py rename to tests/unit/test_tests_check.py index 4173a64..caeae1b 100644 --- a/tests/test_tests_check.py +++ b/tests/unit/test_tests_check.py @@ -43,8 +43,9 @@ def test_01_all_tests_pass(self, mock_pytest_run: MagicMock) -> None: Verify run calculates the correct score when all tests pass. """ # Arrange + expected_info = "Test test_1::test_1 passed.\nTest test_2::test_2 passed." mock_pytest_run.return_value = "PASSED ::test_1::test_1\nPASSED ::test_2::test_2" - expected_score = ScoredCheckResult(self.name, 50.0, self.max_points) + expected_score = ScoredCheckResult(self.name, 50.0, expected_info, "", self.max_points) # Act score = self.tests_check.run() @@ -58,8 +59,9 @@ def test_02_some_tests_fail(self, mock_pytest_run: MagicMock) -> None: Verify run calculates the correct score when some tests fail. """ # Arrange + expected_info = "Test test_1::test_1 passed.\nTest test_2::test_2 failed." mock_pytest_run.return_value = "PASSED ::test_1::test_1\nFAILED ::test_2::test_2" - expected_score = ScoredCheckResult(self.name, 20.0, self.max_points) + expected_score = ScoredCheckResult(self.name, 20.0, expected_info, "", self.max_points) # Act score = self.tests_check.run() @@ -114,7 +116,7 @@ def test_05_empty_test_results(self, mock_pytest_run: MagicMock) -> None: """ # Arrange mock_pytest_run.return_value = "" - expected_score = ScoredCheckResult(self.name, 0.0, self.max_points) + expected_score = ScoredCheckResult(self.name, 0.0, "", "", self.max_points) # Act score = self.tests_check.run() @@ -129,7 +131,7 @@ def test_06_invalid_pytest_output(self, mock_pytest_run: MagicMock) -> None: """ # Arrange mock_pytest_run.return_value = "INVALID OUTPUT" - expected_score = ScoredCheckResult(self.name, 0.0, self.max_points) + expected_score = ScoredCheckResult(self.name, 0.0, "", "", self.max_points) # Act score = self.tests_check.run() @@ -180,7 +182,9 @@ def test_10_class_scored(self, mock_pytest_run: MagicMock) -> None: """ # Arrange mock_pytest_run.return_value = "PASSED ::ClassB::test_1\nPASSED ::ClassB::test_2\nPASSED ::ClassA::test_3" - expected_score = ScoredCheckResult(self.name, 65.0, self.max_points) + expected_info = "Test ClassB::test_1 passed.\nTest ClassB::test_2 passed.\nTest ClassA::test_3 passed." + + expected_score = ScoredCheckResult(self.name, 65.0, expected_info, "", self.max_points) # Act score = self.tests_check.run() @@ -195,7 +199,9 @@ def test_11_test_scored_both_name_and_class(self, mock_pytest_run: MagicMock) -> """ # Arrange mock_pytest_run.return_value = "PASSED ::ClassB::test_1\nPASSED ::ClassB::test_2\nPASSED ::ClassA::test_3" - expected_score = ScoredCheckResult(self.name, 100.0, self.max_points) + expected_info = "Test ClassB::test_1 passed.\nTest ClassB::test_2 passed.\nTest ClassA::test_3 passed." + + expected_score = ScoredCheckResult(self.name, 100.0, expected_info, "", self.max_points) tests_check = RunTestsCheck( self.name, @@ -219,7 +225,8 @@ def test_12_test_default_scored(self, mock_pytest_run: MagicMock) -> None: """ # Arrange mock_pytest_run.return_value = "PASSED ::ClassB::test_1\nPASSED ::ClassB::test_2\nPASSED ::ClassA::test_3" - expected_score = ScoredCheckResult(self.name, 60.0, self.max_points) + expected_info = "Test ClassB::test_1 passed.\nTest ClassB::test_2 passed.\nTest ClassA::test_3 passed." + expected_score = ScoredCheckResult(self.name, 60.0, expected_info, "", self.max_points) tests_check = RunTestsCheck( self.name, diff --git a/tests/test_type_hints_check.py b/tests/unit/test_type_hints_check.py similarity index 90% rename from tests/test_type_hints_check.py rename to tests/unit/test_type_hints_check.py index fd7042c..e930424 100644 --- a/tests/test_type_hints_check.py +++ b/tests/unit/test_type_hints_check.py @@ -92,7 +92,7 @@ def test_04_translate_score_zero(self, mocked_run: MagicMock) -> None: with open(const.MYPY_LINE_COUNT_REPORT, "w", encoding="utf-8") as report_file: report_file.write("0 0 0 100 0") - expected_score = ScoredCheckResult("type_hints", 0, 2) + expected_score = ScoredCheckResult("type_hints", 0, "0.00% of the functions have type hints", "", 2) # Act actual_score = self.type_hints_check.run() @@ -113,7 +113,7 @@ def test_05_translate_score_inside_first_range(self, mocked_run: MagicMock) -> N with open(const.MYPY_LINE_COUNT_REPORT, "w", encoding="utf-8") as report_file: report_file.write("0 0 20 100 0") - expected_score = ScoredCheckResult("type_hints", 0, 2) + expected_score = ScoredCheckResult("type_hints", 0, "20.00% of the functions have type hints", "", 2) # Act actual_score = self.type_hints_check.run() @@ -134,7 +134,7 @@ def test_06_translate_score_right_bound_first_range(self, mocked_run: MagicMock) with open(const.MYPY_LINE_COUNT_REPORT, "w", encoding="utf-8") as report_file: report_file.write("0 0 1 3 0") - expected_score = ScoredCheckResult("type_hints", 1, 2) + expected_score = ScoredCheckResult("type_hints", 1, "33.33% of the functions have type hints", "", 2) # Act actual_score = self.type_hints_check.run() @@ -155,7 +155,7 @@ def test_07_translate_score_left_bound_second_range(self, mocked_run: MagicMock) with open(const.MYPY_LINE_COUNT_REPORT, "w", encoding="utf-8") as report_file: report_file.write("0 0 11 30 0") - expected_score = ScoredCheckResult("type_hints", 1, 2) + expected_score = ScoredCheckResult("type_hints", 1, "36.67% of the functions have type hints", "", 2) # Act actual_score = self.type_hints_check.run() @@ -176,7 +176,7 @@ def test_08_translate_score_inside_bound_second_range(self, mocked_run: MagicMoc with open(const.MYPY_LINE_COUNT_REPORT, "w", encoding="utf-8") as report_file: report_file.write("0 0 15 30 0") - expected_score = ScoredCheckResult("type_hints", 1, 2) + expected_score = ScoredCheckResult("type_hints", 1, "50.00% of the functions have type hints", "", 2) # Act actual_score = self.type_hints_check.run() @@ -197,7 +197,7 @@ def test_09_translate_score_right_bound_second_range(self, mocked_run: MagicMock with open(const.MYPY_LINE_COUNT_REPORT, "w", encoding="utf-8") as report_file: report_file.write("0 0 22 33 0") - expected_score = ScoredCheckResult("type_hints", 2, 2) + expected_score = ScoredCheckResult("type_hints", 2, "66.67% of the functions have type hints", "", 2) # Act actual_score = self.type_hints_check.run() @@ -218,7 +218,7 @@ def test_10_translate_score_inside_bound_third_range(self, mocked_run: MagicMock with open(const.MYPY_LINE_COUNT_REPORT, "w", encoding="utf-8") as report_file: report_file.write("0 0 35 40 0") - expected_score = ScoredCheckResult("type_hints", 2, 2) + expected_score = ScoredCheckResult("type_hints", 2, "87.50% of the functions have type hints", "", 2) # Act actual_score = self.type_hints_check.run() @@ -239,7 +239,7 @@ def test_11_translate_score_max(self, mocked_run: MagicMock) -> None: with open(const.MYPY_LINE_COUNT_REPORT, "w", encoding="utf-8") as report_file: report_file.write("1290 1805 107 107 total") - expected_score = ScoredCheckResult("type_hints", 2, 2) + expected_score = ScoredCheckResult("type_hints", 2, "100.00% of the functions have type hints", "", 2) # Act actual_score = self.type_hints_check.run() @@ -260,7 +260,7 @@ def test_12_translate_score_all_zeros(self, mocked_run: MagicMock) -> None: with open(const.MYPY_LINE_COUNT_REPORT, "w", encoding="utf-8") as report_file: report_file.write("0 0 0 0 0") - expected_score = ScoredCheckResult("type_hints", 0, 2) + expected_score = ScoredCheckResult("type_hints", 0, "", "", 2) # Act actual_score = self.type_hints_check.run() diff --git a/tests/test_virtual_environment.py b/tests/unit/test_virtual_environment.py similarity index 100% rename from tests/test_virtual_environment.py rename to tests/unit/test_virtual_environment.py diff --git a/uv.lock b/uv.lock index 25e7127..3a9e979 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 2 +revision = 3 requires-python = ">=3.10" resolution-markers = [ "python_full_version >= '3.12'", @@ -727,7 +727,7 @@ wheels = [ [[package]] name = "pygrader" -version = "1.7.1" +version = "1.8.0" source = { editable = "." } dependencies = [ { name = "pylint" },