Skip to content

Commit 438907e

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>
1 parent c7a2c0e commit 438907e

2 files changed

Lines changed: 150 additions & 1 deletion

File tree

roles/validations/filter_plugins/cifmw_validations_xml_filter.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ def __map_xml_results(cls, test_results):
8888
if "error" in data:
8989
ET.SubElement(tc_elm, "failure", attrib={"message": data["error"]})
9090
ET.indent(tree, " ")
91-
return ET.tostring(root_elm, encoding="utf-8", xml_declaration=True)
91+
return ET.tostring(root_elm, encoding="unicode", xml_declaration=True)
9292

9393
def filters(self):
9494
return {
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
#!/usr/bin/python3
2+
"""
3+
Unit tests for cifmw_validations_xml_filter.
4+
5+
These tests verify that the filter:
6+
1. Returns str (not bytes) - CRITICAL for the fix
7+
2. Handles normal cases
8+
3. Handles malformed/edge-case input gracefully
9+
4. Generates valid XML output
10+
"""
11+
12+
import pytest
13+
import sys
14+
import xml.etree.ElementTree as ET
15+
16+
sys.path.insert(0, 'roles/validations/filter_plugins')
17+
from cifmw_validations_xml_filter import FilterModule
18+
19+
20+
class TestFilterReturnType:
21+
"""CRITICAL: Tests to verify the fix for the hang issue"""
22+
23+
def setup_method(self):
24+
self.filter_module = FilterModule()
25+
self.filter_func = self.filter_module.filters()['cifmw_validations_xml_filter']
26+
27+
def test_returns_string_not_bytes(self):
28+
"""
29+
⚠️ CRITICAL TEST ⚠️
30+
31+
Ensure filter returns str, not bytes.
32+
33+
Before fix: encoding='utf-8' → returns bytes → Ansible hangs
34+
After fix: encoding='unicode' → returns str → Ansible works
35+
"""
36+
result = self.filter_func({})
37+
assert isinstance(result, str), \
38+
f"CRITICAL: Filter must return str, not {type(result).__name__}. " \
39+
"Returning bytes causes Ansible to hang!"
40+
41+
def test_empty_results(self):
42+
"""Test with empty test results"""
43+
result = self.filter_func({})
44+
assert isinstance(result, str)
45+
assert '<?xml' in result
46+
assert 'tests="0"' in result
47+
48+
def test_single_passing_test(self):
49+
"""Test with single passing test"""
50+
result = self.filter_func({"test-1": {"time": 1.5}})
51+
assert isinstance(result, str)
52+
assert '<testcase name="test-1"' in result
53+
assert 'tests="1"' in result
54+
assert 'failures="0"' in result
55+
56+
def test_single_failing_test(self):
57+
"""Test with single failing test"""
58+
result = self.filter_func({
59+
"test-1": {"time": 1.0, "error": "Test failed"}
60+
})
61+
assert 'failures="1"' in result
62+
assert 'error' in result.lower() or 'failure' in result.lower()
63+
64+
def test_multiple_mixed_tests(self):
65+
"""Test with multiple passing and failing tests"""
66+
result = self.filter_func({
67+
"test-1": {"time": 1.0},
68+
"test-2.yml": {"time": 2.0, "error": "failed"},
69+
"test-3.yaml": {"time": 1.5}
70+
})
71+
assert 'tests="3"' in result
72+
assert 'failures="1"' in result
73+
74+
def test_valid_xml_output(self):
75+
"""Verify output is valid, parseable XML"""
76+
result = self.filter_func({
77+
"test-1": {"time": 1.0, "error": "err"},
78+
"test-2": {"time": 2.0}
79+
})
80+
81+
# Should be parseable as XML (not corrupted)
82+
root = ET.fromstring(result)
83+
assert root.tag == 'testsuites'
84+
85+
# Verify structure
86+
ts = root.find('testsuite')
87+
assert ts is not None
88+
assert ts.attrib['name'] == 'validations'
89+
assert ts.attrib['tests'] == '2'
90+
assert ts.attrib['failures'] == '1'
91+
92+
93+
class TestMalformedInput:
94+
"""Tests with edge-case/malformed input to ensure robustness"""
95+
96+
def setup_method(self):
97+
self.filter_module = FilterModule()
98+
self.filter_func = self.filter_module.filters()['cifmw_validations_xml_filter']
99+
100+
def test_missing_time_field(self):
101+
"""Test with missing time field in some results"""
102+
result = self.filter_func({
103+
"test-1": {"error": "some error"},
104+
"test-2": {"time": 1.5}
105+
})
106+
assert isinstance(result, str)
107+
assert 'tests="2"' in result
108+
109+
def test_very_long_error_message(self):
110+
"""Test with very long error message (5000+ chars)"""
111+
long_error = "x" * 5000
112+
result = self.filter_func({
113+
"test-1": {"time": 1.0, "error": long_error}
114+
})
115+
assert isinstance(result, str)
116+
root = ET.fromstring(result)
117+
assert root is not None
118+
119+
def test_xml_special_characters_in_error(self):
120+
"""Test with XML special characters that need escaping"""
121+
result = self.filter_func({
122+
"test-1": {"time": 1.0, "error": "<tag> & special > chars"}
123+
})
124+
assert isinstance(result, str)
125+
# Should be properly escaped by ElementTree
126+
root = ET.fromstring(result)
127+
assert root is not None
128+
129+
def test_unicode_characters(self):
130+
"""Test with unicode characters"""
131+
result = self.filter_func({
132+
"test-unicode": {"time": 1.0, "error": "Error message"}
133+
})
134+
assert isinstance(result, str)
135+
assert 'tests="1"' in result
136+
137+
def test_large_number_of_tests(self):
138+
"""Test with large number of test cases (100+)"""
139+
large_test_data = {
140+
f"test-{i}": {"time": 0.1 * i}
141+
for i in range(100)
142+
}
143+
result = self.filter_func(large_test_data)
144+
assert isinstance(result, str)
145+
assert 'tests="100"' in result
146+
147+
148+
if __name__ == "__main__":
149+
pytest.main([__file__, "-v"])

0 commit comments

Comments
 (0)