Courses, Units, Lessons, and KPUB#653
Courses, Units, Lessons, and KPUB#653rtibbles wants to merge 3 commits intolearningequality:mainfrom
Conversation
bjester
left a comment
bjester
left a comment
There was a problem hiding this comment.
Needs to update the le_utils version
|
Updated! |
| """A file with .kpub extension that isn't a valid zip should fail.""" | ||
| temp_file = tempfile.NamedTemporaryFile(suffix=".kpub", delete=False) | ||
| temp_file.write(b"not a zip file") | ||
| temp_file.close() | ||
|
|
||
| try: | ||
| handler = KPUBConversionHandler() | ||
| with pytest.raises(InvalidFileException) as exc_info: | ||
| handler.validate_archive(temp_file.name) | ||
|
|
||
| assert "zip" in str(exc_info.value).lower() | ||
| finally: | ||
| os.unlink(temp_file.name) |
There was a problem hiding this comment.
I presume these are LLM generated?
6 out of 7 tests follow pretty much the same pattern, with this test being the outlier. A test case would eliminate the redundancy.
Although, the approach is somewhat poor. It could use a context manager without delete=False and eliminate the try / finally / unlink. With that change, a test case isn't quite the dramatic cleanup.
| """ | ||
| # Text must be non-empty and not just whitespace | ||
| # Pattern from le_utils schema: ^\s*\S[\s\S]*$ | ||
| if not self.text or not re.match(r"^\s*\S[\s\S]*$", self.text): |
There was a problem hiding this comment.
I believe this regex would pass with just \t or \n
| @questions.setter | ||
| def questions(self, value): | ||
| """Setter to allow base Node class initialization (ignored for UnitNode).""" | ||
| # UnitNode manages questions through test_questions, so ignore base class assignment | ||
| pass |
There was a problem hiding this comment.
I presume this is for consistency with ExerciseNode, but they have no shared interface, besides declaring this property.
|
|
||
| def test_variants_are_distinct(self): | ||
| """VARIANT_A and VARIANT_B have different values.""" | ||
| assert VARIANT_A != VARIANT_B |
There was a problem hiding this comment.
Surely a code review is sufficient for this 😅
| assert result.content_node_metadata is not None | ||
| assert result.content_node_metadata["kind"] == content_kinds.DOCUMENT | ||
| finally: | ||
| os.unlink(temp_archive.name) |
There was a problem hiding this comment.
Similar to the other kpub tests
2 participants
Summary
Adds support for uploading KPUB files
Adds Courses, Units, and Lessons nodes with validation
Adds example courses script for two sample courses.
References
Fixes #638
Reviewer guidance