From 6ef82aa2289d55fa198e1cf509be5c4d96e22140 Mon Sep 17 00:00:00 2001 From: Harshita Goel Date: Tue, 16 Jun 2026 16:08:04 +0530 Subject: [PATCH] [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 [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 [validations] chore: add copyright header to filter file Signed-off-by: Harshita Goel --- .../cifmw_validations_xml_filter.py | 5 +- .../test_cifmw_validations_xml_filter.py | 106 ++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 roles/validations/tests/unit/plugins/filter/test_cifmw_validations_xml_filter.py diff --git a/roles/validations/filter_plugins/cifmw_validations_xml_filter.py b/roles/validations/filter_plugins/cifmw_validations_xml_filter.py index 01bbbcd1d..2fcbad733 100755 --- a/roles/validations/filter_plugins/cifmw_validations_xml_filter.py +++ b/roles/validations/filter_plugins/cifmw_validations_xml_filter.py @@ -1,4 +1,7 @@ #!/usr/bin/python3 +# Copyright (c) 2026 Red Hat, Inc. +# SPDX-License-Identifier: Apache-2.0 + __metaclass__ = type @@ -88,7 +91,7 @@ def __map_xml_results(cls, test_results): if "error" in data: ET.SubElement(tc_elm, "failure", attrib={"message": data["error"]}) ET.indent(tree, " ") - return ET.tostring(root_elm, encoding="utf-8", xml_declaration=True) + return ET.tostring(root_elm, encoding="unicode", xml_declaration=True) def filters(self): return { diff --git a/roles/validations/tests/unit/plugins/filter/test_cifmw_validations_xml_filter.py b/roles/validations/tests/unit/plugins/filter/test_cifmw_validations_xml_filter.py new file mode 100644 index 000000000..36e851599 --- /dev/null +++ b/roles/validations/tests/unit/plugins/filter/test_cifmw_validations_xml_filter.py @@ -0,0 +1,106 @@ +#!/usr/bin/python3 +# Copyright (c) 2026 Red Hat, Inc. +# SPDX-License-Identifier: Apache-2.0 + +import pytest +import sys +import xml.etree.ElementTree as ET + +sys.path.insert(0, 'roles/validations/filter_plugins') +from cifmw_validations_xml_filter import FilterModule + + +class TestFilterReturnType: + """Tests to verify the filter works correctly""" + + def setup_method(self): + self.filter_module = FilterModule() + self.filter_func = self.filter_module.filters()['cifmw_validations_xml_filter'] + + def test_returns_format(self): + """Verify filter returns str format, not bytes.""" + result = self.filter_func({}) + assert isinstance(result, str), "Filter must return str, not bytes!" + + def test_empty_results(self): + """Verify filter generates valid XML with empty test results.""" + result = self.filter_func({}) + assert isinstance(result, str) + assert 'tests="0"' in result + + def test_single_passing_test(self): + """Verify filter correctly formats a single passing test case.""" + result = self.filter_func({"test-1": {"time": 1.5}}) + assert isinstance(result, str) + assert 'tests="1"' in result + assert 'failures="0"' in result + + def test_single_failing_test(self): + """Verify filter correctly formats a single failing test case.""" + result = self.filter_func({"test-1": {"time": 1.0, "error": "Test failed"}}) + assert 'failures="1"' in result + + def test_multiple_mixed_tests(self): + """Verify filter correctly formats multiple passing and failing tests.""" + result = self.filter_func({ + "test-1": {"time": 1.0}, + "test-2.yml": {"time": 2.0, "error": "failed"}, + "test-3.yaml": {"time": 1.5} + }) + assert 'tests="3"' in result + assert 'failures="1"' in result + + def test_valid_xml_output(self): + """Verify filter generates valid, parseable XML with correct structure.""" + result = self.filter_func({"test-1": {"time": 1.0, "error": "err"}, "test-2": {"time": 2.0}}) + root = ET.fromstring(result) + assert root.tag == 'testsuites' + ts = root.find('testsuite') + assert ts.attrib['tests'] == '2' + assert ts.attrib['failures'] == '1' + + +class TestMalformedInput: + """Tests to verify filter handles edge cases and malformed input gracefully""" + + def setup_method(self): + self.filter_module = FilterModule() + self.filter_func = self.filter_module.filters()['cifmw_validations_xml_filter'] + + def test_missing_time_field(self): + """Verify filter handles test results with missing time field.""" + result = self.filter_func({"test-1": {"error": "some error"}, "test-2": {"time": 1.5}}) + assert isinstance(result, str) + assert 'tests="2"' in result + + def test_very_long_error_message(self): + """Verify filter handles very long error messages (5000+ characters).""" + long_error = "x" * 5000 + result = self.filter_func({"test-1": {"time": 1.0, "error": long_error}}) + assert isinstance(result, str) + root = ET.fromstring(result) + assert root is not None + + def test_xml_special_characters_in_error(self): + """Verify filter properly escapes XML special characters in error messages.""" + result = self.filter_func({"test-1": {"time": 1.0, "error": " & special > chars"}}) + assert isinstance(result, str) + root = ET.fromstring(result) + assert root is not None + + def test_unicode_characters(self): + """Verify filter handles unicode characters in test names and messages.""" + result = self.filter_func({"test-unicode": {"time": 1.0, "error": "Error message"}}) + assert isinstance(result, str) + assert 'tests="1"' in result + + def test_large_number_of_tests(self): + """Verify filter performs well with large number of test cases (100+).""" + large_test_data = {f"test-{i}": {"time": 0.1 * i} for i in range(100)} + result = self.filter_func(large_test_data) + assert isinstance(result, str) + assert 'tests="100"' in result + + +if __name__ == "__main__": + pytest.main([__file__, "-v"])