Spaces:
Sleeping
Sleeping
Kyosuke Ichikawa
commited on
fix: Handle None values in _create_progress_html to prevent TypeError (#3)
Browse filesFixed TypeError: '>' not supported between instances of 'NoneType' and 'int'
that occurred when estimated_total_parts was None during session restoration.
Changes:
- Updated _create_progress_html to accept Optional[int] for parameters
- Added null safety checks converting None to 0 before arithmetic operations
- Fixed all usages of current_part and total_parts in time calculations
- Added comprehensive unit tests to verify None handling works correctly
- tests/unit/test_create_progress_html_fix.py +108 -0
- yomitalk/app.py +12 -9
tests/unit/test_create_progress_html_fix.py
ADDED
|
@@ -0,0 +1,108 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
"""
|
| 2 |
+
Test to verify that the _create_progress_html bug has been fixed.
|
| 3 |
+
This tests that None values are handled gracefully.
|
| 4 |
+
"""
|
| 5 |
+
|
| 6 |
+
import time
|
| 7 |
+
from yomitalk.app import PaperPodcastApp
|
| 8 |
+
|
| 9 |
+
|
| 10 |
+
class TestCreateProgressHtmlFix:
|
| 11 |
+
"""Test cases to verify the fix for _create_progress_html with None values."""
|
| 12 |
+
|
| 13 |
+
def setup_method(self):
|
| 14 |
+
"""Set up test fixtures."""
|
| 15 |
+
self.app = PaperPodcastApp()
|
| 16 |
+
|
| 17 |
+
def test_create_progress_html_with_none_total_parts_fixed(self):
|
| 18 |
+
"""
|
| 19 |
+
Test that _create_progress_html handles None total_parts gracefully after the fix.
|
| 20 |
+
"""
|
| 21 |
+
# This should no longer raise a TypeError
|
| 22 |
+
result = self.app._create_progress_html(
|
| 23 |
+
current_part=5,
|
| 24 |
+
total_parts=None, # None should be handled gracefully
|
| 25 |
+
status_message="Test status",
|
| 26 |
+
is_completed=False,
|
| 27 |
+
start_time=time.time(),
|
| 28 |
+
)
|
| 29 |
+
|
| 30 |
+
# Verify we get a valid HTML string
|
| 31 |
+
assert isinstance(result, str)
|
| 32 |
+
assert len(result) > 0
|
| 33 |
+
assert "Test status" in result
|
| 34 |
+
assert "🎵" in result # Should show the progress emoji
|
| 35 |
+
|
| 36 |
+
def test_create_progress_html_with_none_current_part(self):
|
| 37 |
+
"""
|
| 38 |
+
Test that _create_progress_html handles None current_part gracefully.
|
| 39 |
+
"""
|
| 40 |
+
result = self.app._create_progress_html(
|
| 41 |
+
current_part=None, # None should be handled gracefully
|
| 42 |
+
total_parts=10,
|
| 43 |
+
status_message="Test with None current_part",
|
| 44 |
+
is_completed=False,
|
| 45 |
+
start_time=time.time(),
|
| 46 |
+
)
|
| 47 |
+
|
| 48 |
+
assert isinstance(result, str)
|
| 49 |
+
assert "Test with None current_part" in result
|
| 50 |
+
|
| 51 |
+
def test_create_progress_html_with_both_none(self):
|
| 52 |
+
"""
|
| 53 |
+
Test that _create_progress_html handles both None values gracefully.
|
| 54 |
+
"""
|
| 55 |
+
result = self.app._create_progress_html(current_part=None, total_parts=None, status_message="Both None", is_completed=False, start_time=time.time())
|
| 56 |
+
|
| 57 |
+
assert isinstance(result, str)
|
| 58 |
+
assert "Both None" in result
|
| 59 |
+
|
| 60 |
+
def test_create_progress_html_completed_with_none_values(self):
|
| 61 |
+
"""
|
| 62 |
+
Test that completed status works even with None values.
|
| 63 |
+
"""
|
| 64 |
+
result = self.app._create_progress_html(
|
| 65 |
+
current_part=None,
|
| 66 |
+
total_parts=None,
|
| 67 |
+
status_message="Completed test",
|
| 68 |
+
is_completed=True, # Completed should bypass progress calculation
|
| 69 |
+
start_time=time.time(),
|
| 70 |
+
)
|
| 71 |
+
|
| 72 |
+
assert isinstance(result, str)
|
| 73 |
+
assert "Completed test" in result
|
| 74 |
+
assert "✅" in result # Should show completion emoji
|
| 75 |
+
|
| 76 |
+
def test_normal_operation_still_works(self):
|
| 77 |
+
"""
|
| 78 |
+
Test that normal operation with valid integers still works correctly.
|
| 79 |
+
"""
|
| 80 |
+
result = self.app._create_progress_html(current_part=3, total_parts=10, status_message="Normal operation", is_completed=False, start_time=time.time())
|
| 81 |
+
|
| 82 |
+
assert isinstance(result, str)
|
| 83 |
+
assert "Normal operation" in result
|
| 84 |
+
# Should calculate 30% progress (but capped at 95%)
|
| 85 |
+
assert "🎵" in result
|
| 86 |
+
|
| 87 |
+
def test_progress_percentage_calculation_with_valid_values(self):
|
| 88 |
+
"""
|
| 89 |
+
Test that progress percentage is calculated correctly with valid values.
|
| 90 |
+
"""
|
| 91 |
+
result = self.app._create_progress_html(current_part=2, total_parts=4, status_message="Half done", is_completed=False)
|
| 92 |
+
|
| 93 |
+
# 2/4 = 50% progress
|
| 94 |
+
assert "50%" in result or "50 %" in result
|
| 95 |
+
|
| 96 |
+
def test_zero_division_protection(self):
|
| 97 |
+
"""
|
| 98 |
+
Test that zero total_parts doesn't cause division by zero.
|
| 99 |
+
"""
|
| 100 |
+
result = self.app._create_progress_html(
|
| 101 |
+
current_part=5,
|
| 102 |
+
total_parts=0, # This should be handled without division error
|
| 103 |
+
status_message="Zero total",
|
| 104 |
+
is_completed=False,
|
| 105 |
+
)
|
| 106 |
+
|
| 107 |
+
assert isinstance(result, str)
|
| 108 |
+
assert "Zero total" in result
|
yomitalk/app.py
CHANGED
|
@@ -1084,8 +1084,8 @@ class PaperPodcastApp:
|
|
| 1084 |
|
| 1085 |
def _create_progress_html(
|
| 1086 |
self,
|
| 1087 |
-
current_part: int,
|
| 1088 |
-
total_parts: int,
|
| 1089 |
status_message: str,
|
| 1090 |
is_completed: bool = False,
|
| 1091 |
start_time: Optional[float] = None,
|
|
@@ -1094,11 +1094,11 @@ class PaperPodcastApp:
|
|
| 1094 |
Create comprehensive progress display with progress bar, elapsed time, and estimated remaining time.
|
| 1095 |
|
| 1096 |
Args:
|
| 1097 |
-
current_part (int): Current part number
|
| 1098 |
-
total_parts (int): Total number of parts
|
| 1099 |
status_message (str): Status message to display
|
| 1100 |
is_completed (bool): Whether the generation is completed
|
| 1101 |
-
start_time (float): Start time timestamp for calculating elapsed time
|
| 1102 |
|
| 1103 |
Returns:
|
| 1104 |
str: HTML string for progress display
|
|
@@ -1109,7 +1109,10 @@ class PaperPodcastApp:
|
|
| 1109 |
progress_percent = 100
|
| 1110 |
emoji = "✅"
|
| 1111 |
else:
|
| 1112 |
-
|
|
|
|
|
|
|
|
|
|
| 1113 |
emoji = "🎵"
|
| 1114 |
|
| 1115 |
# 経過時間と推定残り時間を計算
|
|
@@ -1121,10 +1124,10 @@ class PaperPodcastApp:
|
|
| 1121 |
|
| 1122 |
if is_completed:
|
| 1123 |
time_info = f" | 完了時間: {elapsed_minutes:02d}:{elapsed_seconds:02d}"
|
| 1124 |
-
elif
|
| 1125 |
# 推定残り時間を計算(現在のペースに基づく)
|
| 1126 |
-
avg_time_per_part = elapsed_time /
|
| 1127 |
-
remaining_parts =
|
| 1128 |
estimated_remaining = avg_time_per_part * remaining_parts
|
| 1129 |
remaining_minutes = int(estimated_remaining // 60)
|
| 1130 |
remaining_seconds = int(estimated_remaining % 60)
|
|
|
|
| 1084 |
|
| 1085 |
def _create_progress_html(
|
| 1086 |
self,
|
| 1087 |
+
current_part: Optional[int],
|
| 1088 |
+
total_parts: Optional[int],
|
| 1089 |
status_message: str,
|
| 1090 |
is_completed: bool = False,
|
| 1091 |
start_time: Optional[float] = None,
|
|
|
|
| 1094 |
Create comprehensive progress display with progress bar, elapsed time, and estimated remaining time.
|
| 1095 |
|
| 1096 |
Args:
|
| 1097 |
+
current_part (Optional[int]): Current part number (None treated as 0)
|
| 1098 |
+
total_parts (Optional[int]): Total number of parts (None treated as 0)
|
| 1099 |
status_message (str): Status message to display
|
| 1100 |
is_completed (bool): Whether the generation is completed
|
| 1101 |
+
start_time (Optional[float]): Start time timestamp for calculating elapsed time
|
| 1102 |
|
| 1103 |
Returns:
|
| 1104 |
str: HTML string for progress display
|
|
|
|
| 1109 |
progress_percent = 100
|
| 1110 |
emoji = "✅"
|
| 1111 |
else:
|
| 1112 |
+
# Handle None values gracefully by treating them as 0
|
| 1113 |
+
safe_current_part = current_part if current_part is not None else 0
|
| 1114 |
+
safe_total_parts = total_parts if total_parts is not None else 0
|
| 1115 |
+
progress_percent = int(min(95, (safe_current_part / safe_total_parts) * 100) if safe_total_parts > 0 else 0)
|
| 1116 |
emoji = "🎵"
|
| 1117 |
|
| 1118 |
# 経過時間と推定残り時間を計算
|
|
|
|
| 1124 |
|
| 1125 |
if is_completed:
|
| 1126 |
time_info = f" | 完了時間: {elapsed_minutes:02d}:{elapsed_seconds:02d}"
|
| 1127 |
+
elif safe_current_part > 0 and not is_completed:
|
| 1128 |
# 推定残り時間を計算(現在のペースに基づく)
|
| 1129 |
+
avg_time_per_part = elapsed_time / safe_current_part
|
| 1130 |
+
remaining_parts = safe_total_parts - safe_current_part
|
| 1131 |
estimated_remaining = avg_time_per_part * remaining_parts
|
| 1132 |
remaining_minutes = int(estimated_remaining // 60)
|
| 1133 |
remaining_seconds = int(estimated_remaining % 60)
|