feat(tests): add comprehensive tests for cleanup.py. Closes #699#705
Conversation
|
Method used for coverage: docker exec greedybear_uwsgi pip install coverage
docker exec greedybear_uwsgi coverage run --source=greedybear.cronjobs.cleanup manage.py test tests.greedybear.cronjobs.test_cleanup
docker exec greedybear_uwsgi coverage report |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit tests for the CleanUp cronjob class, which was previously untested. The tests verify that the cleanup job correctly initializes with repositories, calculates retention dates, and calls the appropriate deletion methods.
Changes:
- Added three test methods covering initialization, normal operation with deletions, and edge case with zero deletions
- Achieved 100% code coverage for
greedybear/cronjobs/cleanup.py
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Following the Copilot suggestion, I updated line 78 in tests/greedybear/cronjobs/test_cleanup.py. I replaced the loose |
Extended Code Coverage AnalysisI ran a broader coverage check across the Command: Results:
|
|
Thanks for your coverage check. I'll open an issue for testing |
- Mock IOC_RETENTION, COMMAND_SEQUENCE_RETENTION, and COWRIE_SESSION_RETENTION - Use fixed values (100, 90, 80 days) to ensure tests are environment-independent - Remove dependency on settings module imports - Addresses reviewer feedback about environment variable brittleness
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expected_ioc_date = datetime.now() - timedelta(days=100) | ||
| # Check that the date passed is approximately correct (within 1 second) | ||
| args, _ = ioc_repo.delete_old_iocs.call_args | ||
| self.assertAlmostEqual(args[0], expected_ioc_date, delta=timedelta(seconds=1)) |
There was a problem hiding this comment.
Using assertAlmostEqual with datetime objects is problematic. The assertAlmostEqual method is designed for numeric comparisons, and while datetime objects can be subtracted to produce timedeltas, the comparison logic may not work correctly with timedelta delta values. Instead, manually calculate the difference and assert it's within an acceptable timedelta range, or convert to timestamps before comparison.
There was a problem hiding this comment.
Great catch! I completely agree with Copilot's assessment. I've updated the tests to manually calculate the time difference using .total_seconds() and then assert that it's within the acceptable range using assertLess.
This approach is more explicit and provides clearer error messages if the assertion fails. The updated implementation now shows exactly how many seconds the difference is, making debugging much easier.
All tests still pass with the new approach. ✅
- Replace assertAlmostEqual with manual timedelta calculations - Use assertLess for clearer error messages on failure - Convert to CustomTestCase for proper test fixture inheritance - Addresses Copilot feedback about assertAlmostEqual with datetime objects
Description
This PR implements comprehensive unit tests for greedybear/cronjobs/cleanup.py as requested in issue #699. This logic was previously untested.
The tests ensure that the cleanup job correctly:
Related issues
Closes #699
Type of change
Checklist
develop.Ruff) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.Code Coverage Report
Ran tests for
greedybear/cronjobs/cleanup.py: