Skip to content

Commit 6ef82aa

Browse files
committed
[validations] fix: prevent cifmw_validations_xml_filter hang
Issue: The validations playbook was hanging indefinitely when creating JUnit XML files for validation results. Root cause: ET.tostring() with encoding='utf-8' returned bytes, which Ansible's copy module couldn't serialize in a template context, causing an indefinite hang. Solution: Changed encoding parameter to 'unicode', which returns str directly, allowing Ansible to process it normally. Testing: - Created reproduction test proving filter returns bytes (problematic) - Applied the fix - Verified filter now returns str - Added 11 comprehensive unit tests covering: * Return type verification (critical for the fix) * Normal test cases (empty, single, multiple) * Malformed input handling (missing fields, long errors) * XML validity and proper escaping * Performance with 100+ tests - All 11 tests PASS - Ansible playbook completes successfully Signed-off-by: Harshita Goel <hgoel@redhat.com> [validations] tests: improve docstrings and add copyright header - Added copyright header (SPDX-License-Identifier: Apache-2.0) - Moved test-specific information from file docstring to individual tests - Removed special characters from docstrings - Removed 'Before fix/After fix' comments - tests describe behavior, not history - Renamed test_returns_string_not_bytes to test_returns_format for clarity - Updated all docstrings to be more concise and descriptive Signed-off-by: Harshita Goel <hgoel@redhat.com> [validations] chore: add copyright header to filter file Signed-off-by: Harshita Goel <hgoel@redhat.com>
1 parent c7a2c0e commit 6ef82aa

2 files changed

Lines changed: 110 additions & 1 deletion

File tree

roles/validations/filter_plugins/cifmw_validations_xml_filter.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
#!/usr/bin/python3
2+
# Copyright (c) 2026 Red Hat, Inc.
3+
# SPDX-License-Identifier: Apache-2.0
4+
25

36
__metaclass__ = type
47

@@ -88,7 +91,7 @@ def __map_xml_results(cls, test_results):
8891
if "error" in data:
8992
ET.SubElement(tc_elm, "failure", attrib={"message": data["error"]})
9093
ET.indent(tree, " ")
91-
return ET.tostring(root_elm, encoding="utf-8", xml_declaration=True)
94+
return ET.tostring(root_elm, encoding="unicode", xml_declaration=True)
9295

9396
def filters(self):
9497
return {
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
#!/usr/bin/python3
2+
# Copyright (c) 2026 Red Hat, Inc.
3+
# SPDX-License-Identifier: Apache-2.0
4+
5+
import pytest
6+
import sys
7+
import xml.etree.ElementTree as ET
8+
9+
sys.path.insert(0, 'roles/validations/filter_plugins')
10+
from cifmw_validations_xml_filter import FilterModule
11+
12+
13+
class TestFilterReturnType:
14+
"""Tests to verify the filter works correctly"""
15+
16+
def setup_method(self):
17+
self.filter_module = FilterModule()
18+
self.filter_func = self.filter_module.filters()['cifmw_validations_xml_filter']
19+
20+
def test_returns_format(self):
21+
"""Verify filter returns str format, not bytes."""
22+
result = self.filter_func({})
23+
assert isinstance(result, str), "Filter must return str, not bytes!"
24+
25+
def test_empty_results(self):
26+
"""Verify filter generates valid XML with empty test results."""
27+
result = self.filter_func({})
28+
assert isinstance(result, str)
29+
assert 'tests="0"' in result
30+
31+
def test_single_passing_test(self):
32+
"""Verify filter correctly formats a single passing test case."""
33+
result = self.filter_func({"test-1": {"time": 1.5}})
34+
assert isinstance(result, str)
35+
assert 'tests="1"' in result
36+
assert 'failures="0"' in result
37+
38+
def test_single_failing_test(self):
39+
"""Verify filter correctly formats a single failing test case."""
40+
result = self.filter_func({"test-1": {"time": 1.0, "error": "Test failed"}})
41+
assert 'failures="1"' in result
42+
43+
def test_multiple_mixed_tests(self):
44+
"""Verify filter correctly formats multiple passing and failing tests."""
45+
result = self.filter_func({
46+
"test-1": {"time": 1.0},
47+
"test-2.yml": {"time": 2.0, "error": "failed"},
48+
"test-3.yaml": {"time": 1.5}
49+
})
50+
assert 'tests="3"' in result
51+
assert 'failures="1"' in result
52+
53+
def test_valid_xml_output(self):
54+
"""Verify filter generates valid, parseable XML with correct structure."""
55+
result = self.filter_func({"test-1": {"time": 1.0, "error": "err"}, "test-2": {"time": 2.0}})
56+
root = ET.fromstring(result)
57+
assert root.tag == 'testsuites'
58+
ts = root.find('testsuite')
59+
assert ts.attrib['tests'] == '2'
60+
assert ts.attrib['failures'] == '1'
61+
62+
63+
class TestMalformedInput:
64+
"""Tests to verify filter handles edge cases and malformed input gracefully"""
65+
66+
def setup_method(self):
67+
self.filter_module = FilterModule()
68+
self.filter_func = self.filter_module.filters()['cifmw_validations_xml_filter']
69+
70+
def test_missing_time_field(self):
71+
"""Verify filter handles test results with missing time field."""
72+
result = self.filter_func({"test-1": {"error": "some error"}, "test-2": {"time": 1.5}})
73+
assert isinstance(result, str)
74+
assert 'tests="2"' in result
75+
76+
def test_very_long_error_message(self):
77+
"""Verify filter handles very long error messages (5000+ characters)."""
78+
long_error = "x" * 5000
79+
result = self.filter_func({"test-1": {"time": 1.0, "error": long_error}})
80+
assert isinstance(result, str)
81+
root = ET.fromstring(result)
82+
assert root is not None
83+
84+
def test_xml_special_characters_in_error(self):
85+
"""Verify filter properly escapes XML special characters in error messages."""
86+
result = self.filter_func({"test-1": {"time": 1.0, "error": "<tag> & special > chars"}})
87+
assert isinstance(result, str)
88+
root = ET.fromstring(result)
89+
assert root is not None
90+
91+
def test_unicode_characters(self):
92+
"""Verify filter handles unicode characters in test names and messages."""
93+
result = self.filter_func({"test-unicode": {"time": 1.0, "error": "Error message"}})
94+
assert isinstance(result, str)
95+
assert 'tests="1"' in result
96+
97+
def test_large_number_of_tests(self):
98+
"""Verify filter performs well with large number of test cases (100+)."""
99+
large_test_data = {f"test-{i}": {"time": 0.1 * i} for i in range(100)}
100+
result = self.filter_func(large_test_data)
101+
assert isinstance(result, str)
102+
assert 'tests="100"' in result
103+
104+
105+
if __name__ == "__main__":
106+
pytest.main([__file__, "-v"])

0 commit comments

Comments
 (0)