From cbc848491620e3f558817c3fb90dd6c639fc95f2 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 24 Mar 2025 13:00:05 +0100 Subject: [PATCH] Do not consider method-lines as relevant in cobertura The etree API is a bit weird, because the `iter()` method will iterate over matching elements in the whole subtree, instead of only over child elements. Which we actually use extensively, as we are not fully traversing the xml structure of the various formats, but just iterate over the relevant elements, regardless of their nesting. However for cobertura this might not be the right thing to do. As a `class` can have `lines>line`, but also a `class>methods>method` can have `lines>line` as well. We are only interested in the directly declared `lines>line`, not the ones nested within `methods` however. --- Along with fixing this for the cobertura processor, this also changes some tests to use the proper class. We use the `lxml` library in production, but a ton of tests were using the `xml` library, which has a mostly compatible API, but differs in some small details. For example, `lxml` has the `iterchildren` method, while `xml` does not. Along with changing to the proper `lxml` library, some tests also changed their inputs to `bytes`, as the `lxml` library is otherwise throwing an error when trying to parse an xml declaration that defines an `encoding` from a python `str`, which is utf-8 by definition. --- services/report/languages/cobertura.py | 4 +++- .../report/languages/tests/unit/test_clover.py | 10 +++++++--- .../languages/tests/unit/test_cobertura.py | 12 ++++++++---- .../report/languages/tests/unit/test_csharp.py | 4 ++-- .../report/languages/tests/unit/test_csharp2.py | 4 ++-- .../report/languages/tests/unit/test_jacoco.py | 16 +++++++++++----- .../languages/tests/unit/test_jetbrainsxml.py | 2 +- .../languages/tests/unit/test_scoverage.py | 2 +- services/report/languages/tests/unit/test_vb.py | 4 ++-- services/report/languages/tests/unit/test_vb2.py | 2 +- 10 files changed, 38 insertions(+), 22 deletions(-) diff --git a/services/report/languages/cobertura.py b/services/report/languages/cobertura.py index a6e9d2e4e..e725554a3 100644 --- a/services/report/languages/cobertura.py +++ b/services/report/languages/cobertura.py @@ -71,7 +71,9 @@ def from_xml(xml: Element, report_builder_session: ReportBuilderSession) -> None "`create_coverage_file` with pre-fixed path is infallible" ) - for line in _class.iter("line"): + lines = next(_class.iterchildren("lines"), None) + lines = lines.iter("line") if lines else [] + for line in lines: _line = line.attrib ln: str | int = _line["number"] if ln == "undefined": diff --git a/services/report/languages/tests/unit/test_clover.py b/services/report/languages/tests/unit/test_clover.py index e06d1128e..94df76ae4 100644 --- a/services/report/languages/tests/unit/test_clover.py +++ b/services/report/languages/tests/unit/test_clover.py @@ -1,8 +1,8 @@ import datetime -import xml.etree.cElementTree as etree from time import time import pytest +from lxml import etree from helpers.exceptions import ReportExpiredException from services.report.languages import clover @@ -77,7 +77,9 @@ def fixes(path): return path report_builder_session = create_report_builder_session(path_fixer=fixes) - clover.from_xml(etree.fromstring(xml % int(time())), report_builder_session) + clover.from_xml( + etree.fromstring((xml % int(time())).encode()), report_builder_session + ) report = report_builder_session.output_report() processed_report = self.convert_report_to_better_readable(report) @@ -144,4 +146,6 @@ def fixes(path): def test_expired(self, date): report_builder_session = create_report_builder_session() with pytest.raises(ReportExpiredException, match="Clover report expired"): - clover.from_xml(etree.fromstring(xml % date), report_builder_session) + clover.from_xml( + etree.fromstring((xml % date).encode()), report_builder_session + ) diff --git a/services/report/languages/tests/unit/test_cobertura.py b/services/report/languages/tests/unit/test_cobertura.py index ab2d70406..6ec96dac9 100644 --- a/services/report/languages/tests/unit/test_cobertura.py +++ b/services/report/languages/tests/unit/test_cobertura.py @@ -1,9 +1,9 @@ import datetime import os -import xml.etree.cElementTree as etree from time import time import pytest +from lxml import etree from helpers.exceptions import ReportExpiredException from services.path_fixer import PathFixer @@ -534,13 +534,17 @@ def test_use_source_for_filename_if_multiple_sources_first_and_second_works(self def test_empty_filename(): - xml = """ + xml = b""" - + + + - + + + """ diff --git a/services/report/languages/tests/unit/test_csharp.py b/services/report/languages/tests/unit/test_csharp.py index e590855a7..9761e79d0 100644 --- a/services/report/languages/tests/unit/test_csharp.py +++ b/services/report/languages/tests/unit/test_csharp.py @@ -1,11 +1,11 @@ -import xml.etree.cElementTree as etree +from lxml import etree from services.report.languages import csharp from test_utils.base import BaseTestCase from . import create_report_builder_session -xml = """ +xml = b""" diff --git a/services/report/languages/tests/unit/test_csharp2.py b/services/report/languages/tests/unit/test_csharp2.py index 385c2ca3f..6bd0cbdd8 100644 --- a/services/report/languages/tests/unit/test_csharp2.py +++ b/services/report/languages/tests/unit/test_csharp2.py @@ -1,11 +1,11 @@ -import xml.etree.cElementTree as etree +from lxml import etree from services.report.languages import csharp from test_utils.base import BaseTestCase from . import create_report_builder_session -xml = """ +xml = b""" diff --git a/services/report/languages/tests/unit/test_jacoco.py b/services/report/languages/tests/unit/test_jacoco.py index cbf704bc6..46c9fec88 100644 --- a/services/report/languages/tests/unit/test_jacoco.py +++ b/services/report/languages/tests/unit/test_jacoco.py @@ -1,9 +1,9 @@ import datetime import logging -import xml.etree.cElementTree as etree from time import time import pytest +from lxml import etree from pytest import LogCaptureFixture from helpers.exceptions import ReportExpiredException @@ -69,7 +69,9 @@ def fixes(path): report_builder_session = create_report_builder_session(path_fixer=fixes) with self.caplog.at_level(logging.WARNING, logger=jacoco.__name__): - jacoco.from_xml(etree.fromstring(xml % int(time())), report_builder_session) + jacoco.from_xml( + etree.fromstring((xml % int(time())).encode()), report_builder_session + ) assert ( self.caplog.records[-1].message @@ -102,7 +104,9 @@ def fixes(path): current_yaml={"parsers": {"jacoco": {"partials_as_hits": True}}}, path_fixer=fixes, ) - jacoco.from_xml(etree.fromstring(xml % int(time())), report_builder_session) + jacoco.from_xml( + etree.fromstring((xml % int(time())).encode()), report_builder_session + ) report = report_builder_session.output_report() processed_report = self.convert_report_to_better_readable(report) @@ -134,7 +138,7 @@ def test_multi_module(self, module, path): """ % module - ) + ).encode() def fixes(path): if module == "a": @@ -162,4 +166,6 @@ def test_expired(self, date): report_builder_session = create_report_builder_session() with pytest.raises(ReportExpiredException, match="Jacoco report expired"): - jacoco.from_xml(etree.fromstring(xml % date), report_builder_session) + jacoco.from_xml( + etree.fromstring((xml % date).encode()), report_builder_session + ) diff --git a/services/report/languages/tests/unit/test_jetbrainsxml.py b/services/report/languages/tests/unit/test_jetbrainsxml.py index f55d67164..6170e9975 100644 --- a/services/report/languages/tests/unit/test_jetbrainsxml.py +++ b/services/report/languages/tests/unit/test_jetbrainsxml.py @@ -1,4 +1,4 @@ -import xml.etree.cElementTree as etree +from lxml import etree from services.report.languages import jetbrainsxml diff --git a/services/report/languages/tests/unit/test_scoverage.py b/services/report/languages/tests/unit/test_scoverage.py index 44c4decd8..b33647858 100644 --- a/services/report/languages/tests/unit/test_scoverage.py +++ b/services/report/languages/tests/unit/test_scoverage.py @@ -1,4 +1,4 @@ -import xml.etree.cElementTree as etree +from lxml import etree from services.report.languages import scoverage from test_utils.base import BaseTestCase diff --git a/services/report/languages/tests/unit/test_vb.py b/services/report/languages/tests/unit/test_vb.py index fd02c1ce0..dcdc00a33 100644 --- a/services/report/languages/tests/unit/test_vb.py +++ b/services/report/languages/tests/unit/test_vb.py @@ -1,11 +1,11 @@ -import xml.etree.cElementTree as etree +from lxml import etree from services.report.languages import vb from test_utils.base import BaseTestCase from . import create_report_builder_session -txt = """ +txt = b""" diff --git a/services/report/languages/tests/unit/test_vb2.py b/services/report/languages/tests/unit/test_vb2.py index 001de5a9f..6830a5ab5 100644 --- a/services/report/languages/tests/unit/test_vb2.py +++ b/services/report/languages/tests/unit/test_vb2.py @@ -1,4 +1,4 @@ -import xml.etree.cElementTree as etree +from lxml import etree from services.report.languages import vb2 from test_utils.base import BaseTestCase