Skip to content
This repository was archived by the owner on May 6, 2026. It is now read-only.

fix: skip missing media files instead of crashing #113

Merged
aalemayhu merged 1 commit into from
Apr 12, 2026
Merged

fix: skip missing media files instead of crashing #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion helpers/write_apkg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,11 @@ def _write_new_apkg(deck_payloads, media_files):
"""
decks, first_deck_id = create_decks(deck_payloads)
package = Package(decks)
package.media_files = media_files
existing_media = [f for f in media_files if os.path.exists(f)]
if len(existing_media) < len(media_files):
missing = set(media_files) - set(existing_media)
Comment on lines +150 to +151

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

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

missing is computed via set(media_files) - set(existing_media), which collapses duplicates and produces a non-deterministic ordering. Since media_files can contain duplicates (e.g., same image referenced by multiple cards), the reported count/list can be inaccurate and confusing. Consider computing missing as an ordered list (e.g., [f for f in media_files if not os.path.exists(f)]) and using that for both the count and the message.

Suggested change
if len(existing_media) < len(media_files):
missing = set(media_files) - set(existing_media)
missing = [f for f in media_files if not os.path.exists(f)]
if missing:

Copilot uses AI. Check for mistakes.
print(f"Skipping {len(missing)} missing media file(s): {missing}", file=sys.stderr)
package.media_files = existing_media

sanitized_name = sanitize_filename(deck_payloads[0]["name"]) if deck_payloads else "default"
temp_path = write_package_to_temp_file(package)
Expand Down
33 changes: 30 additions & 3 deletions tests/test_write_apkg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,17 @@ def test_write_new_apkg_with_media(self, mock_replace, mock_package):
"desc": "Test Description",
"notes": [note]
}
media_files = ['test.jpg', 'audio.mp3']


with tempfile.TemporaryDirectory() as tmpdir:
img_path = os.path.join(tmpdir, 'test.jpg')
audio_path = os.path.join(tmpdir, 'audio.mp3')
open(img_path, 'w').close()
open(audio_path, 'w').close()
media_files = [img_path, audio_path]

with mock.patch('os.getcwd', return_value=tmpdir):
_write_new_apkg([deck_payload], media_files)

mock_package.assert_called_once()
package_instance = mock_package.return_value
self.assertEqual(package_instance.media_files, media_files)
Expand All @@ -110,6 +115,28 @@ def test_write_new_apkg_with_media(self, mock_replace, mock_package):
mock_package.return_value.write_to_file.assert_called_once()
mock_replace.assert_called_once()

@mock.patch('helpers.write_apkg.Package')
@mock.patch('helpers.write_apkg.os.replace')
def test_write_new_apkg_skips_missing_media(self, mock_replace, mock_package):
note = Note(model=self.test_model, fields=['Q1', 'A1'])
deck_payload = {
"id": 1234567890,
"name": "Test Deck",
"desc": "Test Description",
"notes": [note]
}

with tempfile.TemporaryDirectory() as tmpdir:
existing_path = os.path.join(tmpdir, 'exists.jpg')
open(existing_path, 'w').close()
media_files = [existing_path, 'image.png']
Comment on lines +131 to +132

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

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

This test uses a relative path ('image.png') to represent a missing media file. If a file with that name exists in the process working directory (e.g., developer machine/CI workspace), the test can become flaky. Use an absolute path inside tmpdir (e.g., os.path.join(tmpdir, 'image.png')) without creating it to guarantee it is missing.

Suggested change
open(existing_path, 'w').close()
media_files = [existing_path, 'image.png']
missing_path = os.path.join(tmpdir, 'image.png')
open(existing_path, 'w').close()
media_files = [existing_path, missing_path]

Copilot uses AI. Check for mistakes.

with mock.patch('os.getcwd', return_value=tmpdir):
_write_new_apkg([deck_payload], media_files)

package_instance = mock_package.return_value
self.assertEqual(package_instance.media_files, [existing_path])

@mock.patch('helpers.write_apkg.Package')
@mock.patch('helpers.write_apkg.os.replace')
def test_write_new_apkg_empty_deck_list(self, mock_replace, mock_package):
Expand Down
Loading