Skip to content

Cherry-pick #103: Fix Push AV Stream Verification player reading stale field names #104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rquidute merged 1 commit into from
Jul 21, 2026
+82 −6
Merged

Cherry-pick #103: Fix Push AV Stream Verification player reading stale field names #104

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
46 changes: 46 additions & 0 deletions tests/test_run/camera/test_camera_http_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,52 @@ def test_do_post_unknown_path_sends_404(self):
assert handler._error_code == 404


# ---------------------------------------------------------------------------
# VideoStreamingHandler.serve_player - Push AV Stream Verification template
# ---------------------------------------------------------------------------


@pytest.mark.unit
class TestServePlayerPushAVTemplate:
"""Regression tests for issue #1051: the Push AV Server returns each stream's
uploaded files under valid_uploads/error_uploads (list of {file_path, reasons?}),
not the legacy files/valid_files/invalid_files shape. The rendered template's
JS must read the current field names or the video player stays blank even
when the DUT has successfully uploaded content."""

def _render(self):
handler = _make_handler(
path="/",
server_attrs={
"prompt_options": {"PASS": 1, "FAIL": 2},
"prompt_text": "Verify the video stream",
"is_push_av_verification": True,
"push_av_server_url": "https://192.168.0.53:1234",
},
)
handler.serve_player()
return handler.wfile.getvalue().decode("utf-8")

def test_renders_without_template_error(self):
html_content = self._render()
assert "Template error" not in html_content
assert "<!DOCTYPE html>" in html_content or "<html>" in html_content

def test_reads_valid_and_error_uploads_fields(self):
html_content = self._render()
assert "stream.valid_uploads" in html_content
assert "stream.error_uploads" in html_content
assert "file_path" in html_content

def test_no_longer_relies_solely_on_legacy_file_fields(self):
"""The old field names may still appear as a fallback, but the current
server field names must be checked first."""
html_content = self._render()
valid_uploads_idx = html_content.index("stream.valid_uploads")
valid_files_idx = html_content.index("stream.valid_files")
assert valid_uploads_idx < valid_files_idx


# ---------------------------------------------------------------------------
# VideoStreamingHandler.handle_response
# ---------------------------------------------------------------------------
Expand Down
42 changes: 36 additions & 6 deletions th_cli/test_run/camera/push_av_stream_verification.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,19 @@
nonConformingSection.style.display = 'none';
}}

// Push AV Server returns each stream's uploaded files as valid_uploads/error_uploads,
// each entry being an object with a file_path (and reasons, for error_uploads).
// Older server responses used plain filename arrays under files/valid_files/invalid_files -
// keep supporting those too so this page degrades gracefully against either shape.
function getStreamFilePaths(stream) {{
if (stream.valid_uploads || stream.error_uploads) {{
const validPaths = (stream.valid_uploads || []).map(u => u?.file_path).filter(Boolean);
const invalidPaths = (stream.error_uploads || []).map(u => u?.file_path).filter(Boolean);
return [...validPaths, ...invalidPaths];
}}
return stream.files || [...(stream.valid_files || []), ...(stream.invalid_files || [])];
}}

function selectStream(index) {{
if (!streamsData || !streamsData[index]) {{
console.error('Stream data not found for index:', index);
Expand All @@ -535,8 +548,7 @@
const stream = streamsData[index];
selectedStreamId = stream.id || index;

// Combine valid_files and invalid_files (or use files if present)
const allFiles = stream.files || [...(stream.valid_files || []), ...(stream.invalid_files || [])];
const allFiles = getStreamFilePaths(stream);

// Update selected styling
document.querySelectorAll('.stream-item').forEach(item => {{
Expand Down Expand Up @@ -582,9 +594,14 @@
const streamNum = parseInt(stream.id) + 1;
streamContentsTitle.textContent = `Stream ${{streamNum}} Contents`;

// Combine valid and invalid files (or use files if present)
const validFiles = stream.valid_files || [];
const invalidFiles = stream.invalid_files || [];
const hasUploadsShape = !!(stream.valid_uploads || stream.error_uploads);
const validFiles = hasUploadsShape
? (stream.valid_uploads || []).map(u => u?.file_path).filter(Boolean)
: (stream.valid_files || []);
const invalidUploads = hasUploadsShape ? (stream.error_uploads || []) : [];
const invalidFiles = hasUploadsShape
? invalidUploads.map(u => u?.file_path).filter(Boolean)
: (stream.invalid_files || []);
const allFiles = stream.files || [...validFiles, ...invalidFiles];

// Display files
Expand All @@ -611,13 +628,26 @@

// Display conformance status
if (invalidFiles.length > 0) {{
nonConformingContent.innerHTML = `<div style="color: #c62828; text-align: center;">Found ${{invalidFiles.length}} non-conforming file(s)</div>`;
const reasons = invalidUploads
.flatMap(u => u?.reasons || [])
.filter(Boolean);
Comment on lines +631 to +633

@gemini-code-assist gemini-code-assist Bot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If u?.reasons is a single string instead of an array of strings, flatMap will treat the string as an iterable and split it into individual characters. To make this more robust and defensive, we should explicitly check if reasons is an array before flattening.

Suggested change
const reasons = invalidUploads
.flatMap(u => u?.reasons || [])
.filter(Boolean);
const reasons = invalidUploads
.flatMap(u => {{
const r = u?.reasons;
return Array.isArray(r) ? r : (r ? [r] : []);
}})
.filter(Boolean);

const reasonsHtml = reasons.length > 0
? `<br><small>${{reasons.map(r => html_escape(r)).join('<br>')}}</small>`
: '';
nonConformingContent.innerHTML =
`<div style="color: #c62828; text-align: center;">Found ${{invalidFiles.length}} non-conforming file(s)${{reasonsHtml}}</div>`;
}} else {{
nonConformingContent.innerHTML = '<div class="conforming-message">All files conform to Matter Spec.</div>';
}}
nonConformingSection.style.display = 'block';
}}

function html_escape(str) {{
const div = document.createElement('div');
div.textContent = str;
return div.innerHTML;
}}

let currentDashPlayer = null; // Track current player instance

function playStream(streamUrl) {{
Expand Down
Loading