cobertura: avoid duplicate file names in files()

[jdef]

some coverage reports include metrics for multiple classes within the same file. without this change, redundant rows are generated for such reports.

[aconrad]

Thanks @jdef! Can you please provide a screenshot and coverage report for me to understand your duplicates scenario better? Aren't filenames unique?

[aconrad]

Hum, I think I see what you mean. A sample coverage report would still help if you can provide one. Also if you could write a test for it, it would be greatly appreciated.

Add a test to this file https://github.com/aconrad/pycobertura/blob/master/tests/test_cobertura.py, and include a sample coverage report (ideally a small one to spare the size of the git repository). When you call cobertura = make_cobertura() you can pass it the path to the XML file you included that contains duplicates:

pycobertura/tests/utils.py

Line 4 in c692f26

def make_cobertura(xml=SOURCE_FILE, *args, **kwargs):

[aconrad]

Can you prevent using sorted() to optimize runtime? Maybe something like:

seen_filenames = set()
filenames = []

for el in self.xml.xpath("//class"):
    filename = el.attrib['filename']
    if filename in seen_filenames:
        continue
    seen_filenames.add(filename)
    filenames.append(filename)

return filenames

[jdef]

OK, I'll try that. My initial impression was that sorting strings shouldn't be that intense. I suppose it's probably a bigger issue on a busy CI server that's processing many large projects at the same time?

…

On Fri, Mar 16, 2018 at 1:57 PM, Alexandre Conrad ***@***.***> wrote: ***@***.**** requested changes on this pull request. ------------------------------ In pycobertura/cobertura.py <#82 (comment)>: > @@ -234,7 +234,8 @@ def files(self): """ Return the list of available files in the coverage report. """ - return [el.attrib['filename'] for el in self.xml.xpath("//class")] + return list(sorted(set( + [el.attrib['filename'] for el in self.xml.xpath("//class")]))) Can you prevent using sorted() to optimize runtime? Maybe something like: seen_filenames = set() filenames = [] for el in self.xml.xpath("//class"): filename = el.attrib['filename'] if filename in seen_filenames: continue seen_filenames.add(filename) filenames.append(filename) return filenames — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#82 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACPVLDxDRrKOWmR8BVD49M0ATJiulsyMks5te_z3gaJpZM4St6sO> .

-- James DeFelice 585.241.9488 (voice) 650.649.6071 (fax)

[jdef]

the clock in my VM is off so GH shows the commits out of order, sorry about that

[jdef]

@aconrad feedback addressed, PTAL

[jdef]

ping

[aconrad]

Thanks for your patience and ping @jdef. I meant to find some time to test your change myself. Looks good to me!

[jdef]

Thanks for merging. Is there a timeline for the next release of this project?

[aconrad]

Just released 0.10.2