Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generated xml invalid paths for Cobertura report #526

Closed
nedbat opened this issue Oct 17, 2016 · 26 comments
Closed

generated xml invalid paths for Cobertura report #526

nedbat opened this issue Oct 17, 2016 · 26 comments
Labels
bug Something isn't working xml

Comments

@nedbat
Copy link
Owner

nedbat commented Oct 17, 2016

Originally reported by Dirk Thomas (Bitbucket: dirk-thomas, GitHub: dirk-thomas)


When generating the coverage report as a .xml file the source + filename does not add up to a correct path. As a result the Jenkins Cobertura plugin can't show the source of each module: http://ci.ros2.org/job/test_coveragepy/2/cobertura/_/ament_package_dependency_py/

I have created a Jenkins job to reproduce the issue: http://ci.ros2.org/job/test_coveragepy/2/

The steps are fairly simple:

cd  /tmp
python3 -m venv venv
. venv/bin/activate
pip install coverage mock nose
git clone https://github.com/ament/ament_package.git
cd ament_package
python3 -m nose --nocapture --with-coverage --cover-package ament_package --cover-xml --cover-xml-file coverage.xml

The resulting coverage.xml file contains a source entry pointing to the Python module .../ament_package/ament_package. The class entries have a filename like ament_package/dependency.py and a name (dependency.py). The problem is (as far as I can see) that the source + filename doesn't add up to a correct path. Basically the package name folder is repeated in both. The correct full path should be .../ament_package/ament_package/dependency.py.

The full console output can be found here and the full coverage.xml file here.


@nedbat
Copy link
Owner Author

nedbat commented Dec 12, 2016

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


I've been able to run the reproducer and obtain the XML file that matches your description. I'm not able to reproduce the problem because I don't have access to a jenkins running the cobertura plugin. Could you confirm that running without the --cover-package ament_package generates an XML file with links to sources that are correct ?

#!bash

python3 -m nose --nocapture --with-coverage --cover-xml --cover-xml-file coverage.xml

The sources element does not contain the extra package in this case. I've not been able to find the documentation for the expected content of the XML file. The DTD is validated and correct but only set the requirements for the elements and the attributes, not their content.

@nedbat
Copy link
Owner Author

nedbat commented Dec 16, 2016

Original comment by Dirk Thomas (Bitbucket: dirk-thomas, GitHub: dirk-thomas)


I just ran a third build without the argument (http://ci.ros2.org/job/test_coveragepy/3/). The result is "interesting". The breakdown list contains multiple rows - the relevant ones are "." and "ament_package". For the second one the sources are shown correctly. For the first one the problem is still the same. I think the build no. 2 (with the argument) only showed the "." entry which didn't work in.

I hope that helps...

@nedbat
Copy link
Owner Author

nedbat commented Dec 20, 2016

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


Note: https://pypi.python.org/pypi/diff_cover/0.9.9 relies on XML and is another way to verify the output is valid.

@nedbat
Copy link
Owner Author

nedbat commented Dec 20, 2016

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


@dirk-thomas I'm looking at http://ci.ros2.org/job/test_coveragepy/3/console : this is very useful, thanks.

@nedbat
Copy link
Owner Author

nedbat commented Dec 29, 2016

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


codecov uses the xml report as well.

#!bash

        say "    ${e}->${x} Running coverage xml"
        if [ "$(coverage xml -i)" != "No data to report." ];

I wonder how it is parsed and if the code parsing it is publicly available.

@nedbat
Copy link
Owner Author

nedbat commented Jan 6, 2017

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


@dirk-thomas I noticed your test run is using coverage 3.4. Would it be possible to upgrade to coverage 4.3.1 and verify if the issue still exists ?

#!bash

Installing coverage-3.4 script to /var/lib/jenkins/jobs/test_coveragepy/workspace/venv/bin

@nedbat
Copy link
Owner Author

nedbat commented Jan 6, 2017

Original comment by Dirk Thomas (Bitbucket: dirk-thomas, GitHub: dirk-thomas)


I was surprised since it installs the package from PIP. Therefore I added a pip freeze call to the latest build (http://ci.ros2.org/job/test_coveragepy/5/console). It shows that it installed coverage==4.3.1. So it looks like that the two lines during the installation are kind of misleading:

Installing coverage-3.4 script to /var/lib/jenkins/jobs/test_coveragepy/workspace/venv/bin
Installing coverage3 script to /var/lib/jenkins/jobs/test_coveragepy/workspace/venv/bin

@nedbat
Copy link
Owner Author

nedbat commented Jan 6, 2017

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


@dirk-thomas thanks for checking. And the XML file consistently displays the coverage version as well. It's not a version mismatch then.

#!xml

<coverage branch-rate="0" line-rate="0.3515" timestamp="1483719279629" version="4.3.1">

@nedbat
Copy link
Owner Author

nedbat commented Jan 6, 2017

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


@dirk-thomas could you please try to run coverage run -m nose ; coverage xml instead of python3 -m nose --nocapture --with-coverage --cover-xml --cover-xml-file coverage.xml ? The result is different when I run it locally. If it displays correctly we will know that the nose coverage plugin does something that is not right. If the output does not display well... we will be even more confused ;-)

@nedbat
Copy link
Owner Author

nedbat commented Jan 6, 2017

Original comment by Dirk Thomas (Bitbucket: dirk-thomas, GitHub: dirk-thomas)


Please see the latest build (http://ci.ros2.org/job/test_coveragepy/6/consoleFull) with the modified invocation.

@nedbat
Copy link
Owner Author

nedbat commented Jan 7, 2017

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


@dirk-thomas interesting. Do you know of a project successfully using the Cobertura Coverage Report jenkins plugin with coverage.py ? It would be helpful to see how it looks. I'll analysing this new run and try to guess what's going on.

@nedbat
Copy link
Owner Author

nedbat commented Jan 7, 2017

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


@dirk-thomas is there a way to see which version of the cobertura coverage report plugin you are using ? I'd like to read the sources to get a sense of what it expects.

@nedbat
Copy link
Owner Author

nedbat commented Jan 7, 2017

Original comment by Dirk Thomas (Bitbucket: dirk-thomas, GitHub: dirk-thomas)


Sorry, I don't know any other project using the cobertura plugin with coverage.py.

The Jenkins instance is using version 1.9.8 of the Cobertura plugin which is the latest release according to https://wiki.jenkins-ci.org/display/JENKINS/Cobertura+Plugin

@nedbat
Copy link
Owner Author

nedbat commented Jan 12, 2017

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


@dirk-thomas could you please try a run with coverage run --source=ament_package -m nose ; coverage xml ? That will change paths to be relative instead of absolute. It should not change anything in theory. Except if jenkins moves the directory around and the absolute paths is no longer valid. If, as I suspect, this change does not improve things, at least it will tell us the problem is unrelated to absolute vs relative path names.

@nedbat
Copy link
Owner Author

nedbat commented Jan 12, 2017

Original comment by Dirk Thomas (Bitbucket: dirk-thomas, GitHub: dirk-thomas)


I ran http://ci.ros2.org/job/test_coveragepy/7/ with this command and the result looks much better now, right?

@nedbat
Copy link
Owner Author

nedbat commented Jan 12, 2017

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


It does ! However the source is still unavailable. A little bit closer to figuring this out ;-)

@nedbat
Copy link
Owner Author

nedbat commented Apr 30, 2017

@dirk-thomas any newer information on this? I'm not sure what change you're looking for in coverage.py.

@nedbat
Copy link
Owner Author

nedbat commented May 2, 2017

Original comment by Dirk Thomas (Bitbucket: dirk-thomas, GitHub: dirk-thomas)


I don't have any newer information on this. In the current state I still don't see the sources (instead: "Source code is unavailable").

@nedbat
Copy link
Owner Author

nedbat commented May 3, 2017

OK, the nose plugin is doing something weird. The coverage xml command produces the correct results. I'll take a look at what the plugin is doing and see if there is a way to have both work.

@nedbat
Copy link
Owner Author

nedbat commented May 3, 2017

Original comment by bmerry (Bitbucket: bmerry, GitHub: bmerry)


I can confirm that using coverage xml after running the nose plugin works for me (with the Jenkins Cobertura plugin), while using nosetests with --cover-xml generates broken paths.

The nose plugin actually generates nicer filenames e.g. mypackage/myfile.py instead of venv/lib/python2.7/site-packages/mypackage/myfile.py; but unfortunately the plugin sets to the source to venv/lib/python2.7/site-packages/mypackage instead of venv/lib/python2.7/site-packages, so the combined paths are wrong.

@nedbat
Copy link
Owner Author

nedbat commented May 4, 2017

This is fixed in a84b62abc670 (bb). Thanks.

@nedbat
Copy link
Owner Author

nedbat commented May 4, 2017

Original comment by Dirk Thomas (Bitbucket: dirk-thomas, GitHub: dirk-thomas)


I don't think that the referenced commit fixes this. The problem described in this ticket is not related to __init__.py files. The problem is that the two pieces of information (source and filename) do not add up to a correct path. Therefore I think this should be reopened.

@nedbat
Copy link
Owner Author

nedbat commented May 5, 2017

This also happens without the nose plugin if you use source = ament_package in the .coveragerc file, but not if you specify it on the run command line! :(

@nedbat
Copy link
Owner Author

nedbat commented May 5, 2017

OK, really fixed this time, in 856fecb459de (bb).

@nedbat
Copy link
Owner Author

nedbat commented May 9, 2017

@dirk-thomas Changing this behavior has caused problems for people with multiple sources: https://bitbucket.org/ned/coveragepy/issues/578/incomplete-file-path-in-xml-report Do you have any insight into how we can resolve this in a way that will work for everyone?

@nedbat
Copy link
Owner Author

nedbat commented May 17, 2017

Original comment by Dirk Thomas (Bitbucket: dirk-thomas, GitHub: dirk-thomas)


Sorry, I don't have any helpful information on this.

@nedbat nedbat closed this as completed May 17, 2017
@nedbat nedbat added major bug Something isn't working labels Jun 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working xml
Projects
None yet
Development

No branches or pull requests

1 participant