test: add unit tests for GameXBlock#20
Conversation
- Updated test paths in GitHub Actions workflow and Makefile to reflect new directory structure. - Created new test settings file for Django tests. - Added comprehensive unit tests for GamesXBlock core functionality, handlers, and utilities. - Implemented tests for flashcards and matching handlers. - Added tests for internationalization support and toggle functions. - Updated URLs configuration for tests.
027f85c to
1ce4c0b
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive unit test coverage for the GameXBlock project along with supporting test infrastructure, Spanish translations, API documentation, and minor code quality improvements.
Key Changes
- Added full test suite with 80%+ coverage requirement using pytest and pytest-django
- Introduced CI/CD pipeline with GitHub Actions for automated testing across Python 3.11 and Django 3.2/4.2
- Added Spanish (es_419) translations and updated locale configuration
Reviewed changes
Copilot reviewed 25 out of 28 changed files in this pull request and generated 24 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_games.py | Core GamesXBlock tests covering initialization, defaults, views, handlers, and field scopes |
| tests/test_utils.py | Tests for storage utilities including custom storage configuration and image deletion |
| tests/test_translations.py | Tests for i18n support covering English and Spanish translations |
| tests/test_toggles.py | Tests for waffle flag functionality with exception handling |
| tests/handlers/test_common_handlers.py | Tests for shared handlers including encryption, settings, image upload, and obfuscation |
| tests/handlers/test_matching_handlers.py | Tests for matching game handlers including student view, game completion, and key mapping |
| tests/handlers/test_flashcards_handlers.py | Tests for flashcards game handlers including student view rendering |
| tests/settings.py | Django test settings configuration (has ROOT_URLCONF path issue) |
| tests/urls.py | Empty URL configuration placeholder for tests |
| pytest.ini | Pytest configuration with 80% coverage requirement and test discovery patterns |
| .coveragerc | Coverage configuration excluding migrations, tests, and locale files |
| .github/workflows/test.yml | CI/CD workflow for running tests and quality checks with codecov integration |
| test-requirements.txt | Test dependencies including pytest, faker, and code quality tools |
| requirements/base.txt | Base runtime dependencies for the XBlock |
| requirements/test.txt | Test requirements extending base requirements |
| requirements/ci.txt | CI-specific requirements for coverage reporting |
| Makefile | Added targets for test execution, coverage reporting, and quality checks |
| API.md | Comprehensive API documentation (has multiple constant value mismatches) |
| MANIFEST.in | Package manifest for distribution |
| .gitignore | Updated to exclude test and coverage artifacts |
| games/utils.py | Added docstring to delete_image function |
| games/toggles.py | Fixed trailing whitespace issues |
| games/handlers/flashcards.py | Added blank line for PEP 8 compliance |
| games/locale/config.yaml | Updated locale configuration structure (has potential configuration issue) |
| games/locale/es_419/LC_MESSAGES/django.po | Spanish translation file (has outdated dates) |
| games/locale/es_419/LC_MESSAGES/django.mo | Compiled Spanish translations |
Comments suppressed due to low confidence (5)
tests/handlers/test_common_handlers.py:6
- Import of 'MagicMock' is not used.
from unittest.mock import Mock, patch, MagicMock
tests/handlers/test_flashcards_handlers.py:4
- Import of 'json' is not used.
import json
tests/handlers/test_flashcards_handlers.py:5
- Import of 'MagicMock' is not used.
from unittest.mock import Mock, patch, MagicMock
tests/handlers/test_matching_handlers.py:5
- Import of 'MagicMock' is not used.
from unittest.mock import Mock, patch, MagicMock
tests/test_toggles.py:4
- Import of 'MagicMock' is not used.
from unittest.mock import patch, MagicMock
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Key configuration values (from `games/constants.py`): | ||
|
|
||
| - **`MATCHES_PER_PAGE`**: 6 - Number of term-definition pairs per page in matching game | ||
| - **`RANDOM_STRING_LENGTH`**: 16 - Length of random keys for obfuscation |
There was a problem hiding this comment.
The API documentation states that RANDOM_STRING_LENGTH is 16, but in the actual code (games/constants.py line 55), it's defined as 8. The documentation should be updated to reflect the correct value of 8.
| - **`RANDOM_STRING_LENGTH`**: 16 - Length of random keys for obfuscation | |
| - **`RANDOM_STRING_LENGTH`**: 8 - Length of random keys for obfuscation |
| # Directories to search for translatable strings | ||
| locales: | ||
| - games/locale | ||
| locales_dir: games/locale |
There was a problem hiding this comment.
The key locales_dir should typically be locales according to standard Django i18n configuration. The value 'games/locale' appears to be a directory path rather than a list of locale codes. Please verify this configuration matches the expected format for the translation tooling being used.
| locales_dir: games/locale |
| """ | ||
| import json | ||
| import hashlib | ||
| from unittest.mock import Mock, patch, MagicMock |
There was a problem hiding this comment.
The import MagicMock is not used in this test file. Consider removing it to keep the imports clean.
| from unittest.mock import Mock, patch, MagicMock | |
| from unittest.mock import Mock, patch |
| ### Settings-Scoped Fields | ||
| - **`display_name`** (string): Display name for the XBlock. Default: `"Games XBlock"`. | ||
| - **`game_type`** (string): Type of game - either `"flashcards"` or `"matching"`. Default: `"matching"`. | ||
| - **`is_shuffled`** (boolean): Whether cards should be shuffled. Default: `true`. |
There was a problem hiding this comment.
The API documentation states that is_shuffled has a default value of true, but in the actual code (games/constants.py line 24), DEFAULT.IS_SHUFFLED is defined as False. The documentation should be updated to reflect the correct default value of false.
| import json | ||
| from unittest.mock import Mock, patch, MagicMock |
There was a problem hiding this comment.
The imports json and MagicMock are not used in this test file. Consider removing them to keep the imports clean.
| import json | |
| from unittest.mock import Mock, patch, MagicMock | |
| from unittest.mock import Mock, patch |
| Tests for common handlers. | ||
| """ | ||
| import json | ||
| import hashlib |
There was a problem hiding this comment.
Import of 'hashlib' is not used.
| import hashlib |
|
|
||
| from games.games import GamesXBlock | ||
| from games.handlers.common import CommonHandlers | ||
| from games.constants import GAME_TYPE, DEFAULT |
There was a problem hiding this comment.
Import of 'DEFAULT' is not used.
| from games.constants import GAME_TYPE, DEFAULT | |
| from games.constants import GAME_TYPE |
| """ | ||
| Tests for flashcards and matching handlers. | ||
| """ | ||
| import json |
There was a problem hiding this comment.
Import of 'json' is not used.
| import json |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 28 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (4)
tests/handlers/test_common_handlers.py:6
- Import of 'MagicMock' is not used.
from unittest.mock import Mock, patch, MagicMock
tests/handlers/test_flashcards_handlers.py:4
- Import of 'json' is not used.
import json
tests/handlers/test_flashcards_handlers.py:5
- Import of 'MagicMock' is not used.
from unittest.mock import Mock, patch, MagicMock
tests/handlers/test_matching_handlers.py:5
- Import of 'MagicMock' is not used.
from unittest.mock import Mock, patch, MagicMock
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from faker import Faker | ||
|
|
||
|
|
||
| class TestTranslations(TestCase): | ||
| """Tests for translation support.""" | ||
|
|
||
| def setUp(self): | ||
| """Set up test fixtures.""" | ||
| self.fake = Faker() | ||
|
|
There was a problem hiding this comment.
The Faker import and self.fake initialization are unused in this test file. Consider removing them to keep the code clean.
| from faker import Faker | |
| class TestTranslations(TestCase): | |
| """Tests for translation support.""" | |
| def setUp(self): | |
| """Set up test fixtures.""" | |
| self.fake = Faker() | |
| class TestTranslations(TestCase): | |
| """Tests for translation support.""" | |
| def setUp(self): | |
| """Set up test fixtures.""" |
| def test_student_view_with_multiple_pages(self, mock_resource_string): | ||
| """Test student view with multiple pages of cards.""" | ||
| mock_resource_string.return_value = b'<div>{{ total_pages }}</div>' | ||
| # Add enough cards to create multiple pages (6 cards per page by default) |
There was a problem hiding this comment.
The comment states "6 cards per page by default" but the actual CONFIG.MATCHES_PER_PAGE constant is set to 5 (see games/constants.py:57). Update this to "5 cards per page by default".
| # Add enough cards to create multiple pages (6 cards per page by default) | |
| # Add enough cards to create multiple pages (5 cards per page by default) |
|
|
||
| # Coverage reporting | ||
| coverage==7.4.0 | ||
| codecov==2.1.13 |
There was a problem hiding this comment.
The codecov Python package (version 2.1.13 from 2021) is deprecated and unnecessary since the GitHub Actions workflow uses codecov/codecov-action@v4 which handles coverage upload directly. Consider removing this dependency.
| codecov==2.1.13 |
| """ | ||
| Tests for games toggles. | ||
| """ | ||
| from unittest.mock import patch, MagicMock |
There was a problem hiding this comment.
Import of 'MagicMock' is not used.
| from unittest.mock import patch, MagicMock | |
| from unittest.mock import patch |
Description
Jira