Skip to content

Assessment item/exercise related fixes and tests#5072

Merged
bjester merged 2 commits intolearningequality:unstablefrom
rtibbles:exercises_are_good_for_you_they_say
May 29, 2025
Merged

Assessment item/exercise related fixes and tests#5072
bjester merged 2 commits intolearningequality:unstablefrom
rtibbles:exercises_are_good_for_you_they_say

Conversation

@rtibbles
Copy link
Copy Markdown
Member

@rtibbles rtibbles commented May 20, 2025

Summary

  • Adds new tests for Perseus archive file generation
  • Adds new tests for image resizing
  • Adds regression test for mistake that has been in the codebase for 4+ years courtesy of me, where we were checking for the incorrect placeholder to discover embedded files in assessment items
  • Updates assessment item endpoint to look for the correct place holder - the one that is actually used in the frontend
  • Updates assessment item endpoint and tests for cases where file object does not exist, but file does exist, and case where neither exist

References

This was mostly motivated by having some solid regression tests in place in preparation for #4878

Reviewer guidance

Do the perseus file generation tests provide appropriate coverage, and correctly test the behaviour? Note I did use Claude (as noted in code comments) to generate these tests, but I also carefully reviewed them, and made a lot of updates to the non-image-resize tests.

Does the placeholder being used now match up to the one we use in the frontend? https://github.com/learningequality/studio/blob/unstable/contentcuration/contentcuration/frontend/shared/views/MarkdownEditor/constants.js#L12 <-- this is the one used in the frontend, and also by ricecooker: https://github.com/learningequality/ricecooker/blob/develop/ricecooker/classes/questions.py#L226

@rtibbles
Copy link
Copy Markdown
Member Author

This needs an update to fix linting, and also add a test case for a file existing in cloud storage, but not having an appropriately queryable file object (i.e. their orphaned file object got cleaned up by garbage collection).

@rtibbles rtibbles force-pushed the exercises_are_good_for_you_they_say branch 3 times, most recently from 9489d90 to 0112c54 Compare May 27, 2025 20:05
Only automatically write attached files to perseus file for raw perseus questions.
@rtibbles rtibbles force-pushed the exercises_are_good_for_you_they_say branch from 0112c54 to f28e07d Compare May 27, 2025 20:55
Copy link
Copy Markdown
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see one instance of IMG_PLACEHOLDER in contentcuration/contentcuration/tests/test_rest_framework.py

Should it be there?

@rtibbles rtibbles force-pushed the exercises_are_good_for_you_they_say branch from f28e07d to 131f3bf Compare May 28, 2025 23:01
…rly attached during updates.

Delete outdated, skipped assessment item tests.
@rtibbles rtibbles force-pushed the exercises_are_good_for_you_they_say branch from 131f3bf to 402cb28 Compare May 28, 2025 23:03
@rtibbles
Copy link
Copy Markdown
Member Author

I see one instance of IMG_PLACEHOLDER in contentcuration/contentcuration/tests/test_rest_framework.py

Should it be there?

Looks like the whole test case was being skipped (by me, 5 years ago) so I've just deleted it.

Copy link
Copy Markdown
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:

@bjester bjester merged commit 04e15ad into learningequality:unstable May 29, 2025
13 checks passed
@rtibbles rtibbles deleted the exercises_are_good_for_you_they_say branch June 3, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants