Test backfill for high-risk code + two latent 500 fixes (#1278 item 5)#1339
Merged
Conversation
Backfill regression tests for the delete_unused_files management command, which runs on every container start and deletes media files -- the highest- risk untested path in the app (zero prior coverage). Tests run against a throwaway MEDIA_ROOT and pin: - orphans deleted, DB-referenced files kept (pubs/talks/posters) - easy-thumbnails _detail cache files preserved (owned by thumbnail_cleanup) - accurate (count, bytes) tally from the delete helper - no crash on empty media, no DB rows, or an empty pdf_file FileField Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Found while backfilling tests (item 5). pdf_file is nullable, but the thumbnail-generation block in Artifact.save() called os.path.basename(self.pdf_file.name) unconditionally on every non-first save. self.pdf_file.name is None when the field is empty, raising TypeError. Any second save of a PDF-less artifact hit it (an admin edit, or the authors_changed m2m signal re-saving to rename files). Wrap the block in 'if self.pdf_file:' -- no PDF means no thumbnail to generate. Adds a DB-backed regression test (red before, green after). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GET every public page route (built via reverse so renamed url names fail here too) and assert 200. Catches the NoReverseMatch / template- AttributeError / missing-context-key regression class that only surfaces through the real URL/view/template stack. Covers: index, people, publications, awards, projects, news_listing, view_project_people, member (by id + name), member_artifacts (all 4 types), project detail, news item (by id + slug). The two media routes are excluded (dedicated tests already cover them). Fixtures form a small connected graph so populated template branches render, not just empty states. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Found by the view smoke-sweep (item 5). Project.start_date is nullable, but get_project_dates_str accessed self.start_date.year unconditionally, so visiting the public page of a project with no start_date 500'd with AttributeError. Return '' (template prints date_str as-is, so it renders nothing). Adds regression tests for all four branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tem 5) Backfill fast SimpleTestCase coverage for previously-untested pure helpers: - timeutils: humanize_duration buckets, ends_with_year, remove_trailing_year - ml_utils: slugify_max, school/department abbreviations, get_video_embed (youtube/vimeo/unknown), clean_forum_name, get_closest_match, weighted_choice - fileutils: is_image, get_ckeditor_image_filename, get_filename_no_ext, and the canonical get_filename_for_artifact naming (suffix, ext, truncation) 43 tests, no DB. Star Wars image helpers and filesystem-touching fileutils (thumbnail/PDF) stay covered by their existing/integration tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Backfill the two previously-untested Person.save() behaviors: - Star Wars fallback: empty image / easter_egg get a random figure; provided images are left untouched. Redirects MEDIA_ROOT to a temp dir and patches the picker so the branch is exercised without touching real media. - bio_datetime_modified: stamped on create, updated when bio changes, and left alone on unrelated edits. url_name derivation is already covered by test_recompute_url_names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Implements item 5 of the testing roadmap (#1278): backfill tests for the high-risk untested code the issue flagged, using the coverage report from item 4 to target. Pure test additions, except two one-line model guards for real bugs the new tests surfaced (see below).
Coverage: 59% → 69% of the
websiteapp. Full suite: 343 tests, green (1 pre-existing skip).Two latent 500s found & fixed (tests-first)
The backfill did exactly what the issue predicted — it caught real, production-reachable crashes:
Artifact.save()crashed on an emptypdf_file.pdf_fileis nullable, but the thumbnail block ranos.path.basename(self.pdf_file.name)(i.e.basename(None)) on every non-first save. Any second save of a PDF-less artifact — an admin edit, or theauthors_changedm2m signal re-saving to rename files — raisedTypeError. Guarded withif self.pdf_file:.get_project_dates_str()500'd on a nullstart_date.start_dateis nullable, but the method didself.start_date.yearunconditionally, so visiting such a project's public page raisedAttributeError. Returns""now (the template printsdate_strverbatim). Caught by the new view smoke-sweep on its first run.Each fix has a red→green regression test.
Tests added
test_delete_unused_files.py— the command runs on every container start and deletes media files (0% → 72%). Runs against a throwawayMEDIA_ROOT: orphans deleted, DB-referenced files kept (pubs/talks/posters), easy-thumbnails_detailcache preserved, accurate (count, bytes), no crash on empty media / no rows / emptypdf_file.test_view_smoke.py— GET every public page route (built viareverse, so a renamed url name fails here) → 200. Catches the NoReverseMatch / template-AttributeError / missing-context-key class.test_timeutils.py(100%),test_fileutils.py(87%),test_ml_utils.py— pure-util unit tests.test_person_save.py— Star Wars image fallback +bio_datetime_modifiedside effects.ml_utilslands at 51% by design: the model-/datetime.now-coupled helpers (sort_projects_*,choose_banners*) are left for integration-style tests rather than brittle mocks.Not in scope
Item 6 (Pa11y in CI) and item 7 (JS) — separate PRs.
UI screenshots
None — no UI/template markup changed. The two fixes are server-side guards that prevent 500s; there is no visual change to capture.
Version
Bumped
ML_WEBSITE_VERSION→2.12.2(carries the two 500 fixes).🤖 Generated with Claude Code