Add coverage to project reports.

[kinow]

Hi,

This is based on a pull request submitted for Cylc, but with a few enhancements (which will be applied there too now).

The current coverage reported in Codacy is 3%, but it could be due to some bug in this pull request. Or maybe not all the code can be covered, and so it would be just a matter of ignoring more files... or maybe it is due to some configuration from Codacy that I forgot to change.

I finished this change yesterday, and have been playing with Codacy and Travis until now, trying to tweak the settings to see if there was anything obvious that I was missing. But perhaps somebody else would be interested in having a look at it? (if you compare my/rose Codacy dashboard, there are a small improvement in the metrics due to myself adding more files to be ignored... right now only lib/python/{rose|rosie} are being analyzed - I think).

Before merging it, someone with admin access to the project's Codacy account would have to follow the Codacy docs to enable Travis integration, and also customize what files are ignored in the analysis (e.g. isodatetime, demo, etc, and other folders may be analyzed and considered for coverage statistic).

Cheers
Bruno

[matthewrmshin]

HI @kinow Looking at the list of files with no test coverage, I think there is definitely something not right somewhere. I have expected many of those files to have at least some coverage even under the lean Rose test battery on Travis CI. (We run a fatter test in our site environment to cater for remote host settings etc.)

[kinow]

Hi @matthewrmshin that's what I thought too. It can't be 3%, unless Codacy is perhaps considering the total lines of code (including documentation, tests, etc), or something else is wrong with my settings. I am still validating the settings for Cylc (with 47% right now), and then will try to understand better why the final number here was so low.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@229d8ee). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #2230   +/-   ##
=========================================
  Coverage          ?   84.81%           
=========================================
  Files             ?       93           
  Lines             ?    14761           
  Branches          ?     3200           
=========================================
  Hits              ?    12519           
  Misses            ?     2242           
  Partials          ?        0

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 229d8ee...3cc5c04. Read the comment docs.

[kinow]

Around 20% coverage. I think that's correct, mainly due to some GUI code (which can be omitted/ignored via the omit entry in coveragerc. Cheers, Bruno

[matthewrmshin]

Looking at some individual files (that should be well tested), the reported coverage is still way too low.

[kinow]

If you can point to a few of these files, I can investigate why they are either not appearing, or why their coverage is too low.

[kinow]

Hmmm, the coverage reported in some files is indeed quite strange (e.g. https://codecov.io/gh/metomi/rose/src/report-test-coverage/lib/python/rose/config_tree.py). Not sure why so many imports are not covered, some for-loops are, others not... really confusing. Let me have another look tomorrow morning and update it again 👍

[matthewrmshin]

This one for example should be well tested:
https://codecov.io/gh/metomi/rose/src/master/lib/python/rose/env_cat.py

[kinow]

Fixed!!!

And we have 84.92% covered.

The problem was that source worked with directories, but it wasn't happy about rose and rosie for some reason.

So I found a user who reported a workaround by using include. Tried that today, and didn't work.

Well, turns out source accepts ./lib/python, but include uses that value as regex apparently and that doesn't work.

Replaced by *lib/python/rose* and same for rosie, and now it's working fine. Found it after searching the internet, and by looking at the debug output.

Well done for the devs for such a good test coverage. It seems quite easy to reach 95%, just by writing tests for DAO's and other simple classes.

Ready for review again.

Thanks

[matthewrmshin]

Pretty cool. Minor suggestion for subprocess.call.

[oliver-sanders]

It would be good to get this branch square with it's Cylc equivalent, are we going to copy across the .codecov.yml?

[kinow]

Well noted @oliver-sanders ! Forgot to add that file. Just edited the last commit to include the file based on Cylc's. But set the minimum to 84%, allowing 1% decrease in pull requests. Will leave it up to you guys to decide the best values.

At work, when I pushed this change, I had to trigger the Travis build a couple of times until it succeeded.

Now I changed only the .codecov.yml, but some tests are randomly failing. Tried 6 builds, triggered the last one now. Are the tests normally this unstable, or is it something I am missing in my pull request, or perhaps some change that I missed in .travis.yml?

[matthewrmshin]

Test will be fixed by #2257. Rose test battery is normally quite stable.

[oliver-sanders]

Great!

[oliver-sanders]

Ah, just noticed something.

Since you've found a way of combining the coverage reports of multiple Travis-CI jobs in Cylc, would it be possible to go back to running the functional and documentation tests in separate jobs. Useful, particularly for documentation only changes.

[matthewrmshin]

@kinow @oliver-sanders Any chance that this can be finalised? Do we need to do anything to turn on Codecov properly? Etc.?

[kinow]

@matthewrmshin had lost track of this one, thanks for the remainder.

@oliver-sanders

Since you've found a way of combining the coverage reports of multiple Travis-CI jobs in Cylc, would it be possible to go back to running the functional and documentation tests in separate jobs. Useful, particularly for documentation only changes.

Would you like to take another look and see what else needs changing? I can update it before the weekend.

[oliver-sanders]

Would you like to take another look and see what else needs changing?

@kinow I can make this change in #2222 if necessary but I don't possess your expertise regards combining coverage reports in different jobs. This diff is is what I was thinking (restores parallelism and fits in with #2222), would the coverage still work?

diff --git a/.travis.yml b/.travis.yml
index 617363cf..3cc9a099 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -34,19 +34,22 @@ install:
     - sudo apt-get update
     - sudo apt-get install -y subversion
 
-script:
-    - export COVERAGE_PROCESS_START="${TRAVIS_BUILD_DIR}/.coveragerc"
-    # ./bin/rose will change PYTHONPATH, so we cannot rely on that for sitecustomize.py
-    # but instead we can move the sitecustomize.py to the place used as PYTHONPATH
-    - cp "${TRAVIS_BUILD_DIR}/.travis/sitecustomize.py" ./lib/python
-    - export PYTHONPATH="${TRAVIS_BUILD_DIR}/.travis"
-    - coverage run .travis/cover.py
-    - rm ./lib/python/sitecustomize.py
-
-after_script:
-    - rose make-docs --venv --dev --strict clean linkcheck doctest html slides latexpdf
-
 after_success:
     - coverage combine --append
     - coverage xml --ignore-errors
     - bash <(curl -s https://codecov.io/bash)
+
+jobs:
+    include:
+    - name: "Test Battery"
+      script:
+        - export COVERAGE_PROCESS_START="${TRAVIS_BUILD_DIR}/.coveragerc"
+        # ./bin/rose will change PYTHONPATH, so we cannot rely on that for sitecustomize.py
+        # but instead we can move the sitecustomize.py to the place used as PYTHONPATH
+        - cp "${TRAVIS_BUILD_DIR}/.travis/sitecustomize.py" ./lib/python
+        - export PYTHONPATH="${TRAVIS_BUILD_DIR}/.travis"
+        - coverage run .travis/cover.py
+        - rm ./lib/python/sitecustomize.py
+    - name: "Documentation"
+      script:
+        - rose make-docs --venv --dev --strict clean linkcheck doctest html slides latexpdf

[kinow]

Hi @oliver-sanders ,

I think your patch looks good, and I thought it would work, but only one way of knowing for sure 🙂

https://travis-ci.org/kinow/rose/jobs/461563649, failing due to docs, but successfully uploaded coverage report to Codecov: https://codecov.io/gh/kinow/rose/branch/report-test-coverage

Coverage at 85%. Will submit another pull for the docs issues if it's something simple.

Branch updated, ready for review again 😁

Cheers
Bruno

[kinow]

Oh, @oliver-sanders , also looks like Travis has been having some issues with APT repositories. It failed several times trying to retrieve texlive dependencies. So I updated to the latest Ubuntu distro available in Travis-CI, xenial, and it worked (Cylc is using it too).

It was failing with something similar to

Get:121 http://us-east-1.ec2.archive.ubuntu.com/ubuntu trusty/main amd64 libconfig-inifiles-perl all 2.82-1 [40.0 kB]
Err:113 http://us-east-1.ec2.archive.ubuntu.com/ubuntu trusty/main amd64 texlive-latex-extra-doc all 2013.20140215-2
  Connection failed
Get:113 http://us-east-1.ec2.archive.ubuntu.com/ubuntu trusty/main amd64 texlive-latex-extra-doc all 2013.20140215-2 [317 MB]
Err:113 http://us-east-1.ec2.archive.ubuntu.com/ubuntu trusty/main i386 texlive-latex-extra-doc all 2013.20140215-2
  Connection failed
Err:113 http://us-east-1.ec2.archive.ubuntu.com/ubuntu trusty/main amd64 texlive-latex-extra-doc all 2013.20140215-2
  Connection failed
Err:113 http://us-east-1.ec2.archive.ubuntu.com/ubuntu trusty/main i386 texlive-latex-extra-doc all 2013.20140215-2
  Connection failed
Err:113 http://us-east-1.ec2.archive.ubuntu.com/ubuntu trusty/main i386 texlive-latex-extra-doc all 2013.20140215-2
  Connection failed
Fetched 416 MB in 53s (7,834 kB/s)
E: Failed to fetch http://us-east-1.ec2.archive.ubuntu.com/ubuntu/pool/main/t/texlive-extra/texlive-latex-extra-doc_2013.20140215-2_all.deb  Connection failed
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?
The command "sudo apt-get install -y at build-essential gfortran heirloom-mailx python-pip python-dev graphviz libgraphviz-dev python-jinja2 python-sqlalchemy libxml-parser-perl libconfig-inifiles-perl libdbi-perl libdbd-sqlite3-perl latexmk texlive texlive-generic-extra texlive-latex-extra texlive-fonts-recommended

[kinow]

Another pull request with the dist=xenial was merged, so am rebasing it and will push-force to remove the last commit from this branch (which was also including the change).

Will take a look at Travis to confirm everything is working fine 👍

[kinow]

git is beautiful. git rebase -i HEAD~2, remove the last commit, then git rebase upstream/master and everything works so nicely. And think that just had to spend a few minutes struggling with a local SVN merge 😡

And thanks for @matthewrmshin 's fix for the line length, this PR is again ready for review. Travis happy 👍

[oliver-sanders]

All good here :)