-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add coverage workflow #25
Conversation
Thanks! I would rather get the problems with the test cases solved before including them in the package and setting the coverage GitHub action. I am planning to submit to CRAN rather soon. I did something similar on Sunday setting the coverage action (see recent messages in #10), but did not work. The action itself worked, but the current test cases using 'vdiffr' fail in Ubuntu as the generated .svg files are different in a few cases between Ubuntu and Windows, or rather my Windows 10 machine and GitHub's Ubuntu server. We need to first get the test cases working on Ubuntu with the same saved .svg files than in other systems for CHECK action in Github not to fail. I agree with Daniel that we could protect the failing tests, but I do not know which ones they are. The problem is caused most likely by test cases added by me, possibly, long ago. We would need to run the tests under Ubuntu in a way that we can see the whole output. I will try to find time to run the tests using RStudio cloud to isolate the problem. In the long run I think we should avoid using vdiffr in the tests included in the package build as it is quite fragile when 'ggplot2' gets updated. On the other hand I do not want to dump them completly as they are very useful for debugging visually the plot output. |
Thanks Pedro, if it helps, I can actually run the package tests on my Ubuntu machine and see which are failing. I remember Dinakar mentioning that it was only 2 tests actually. After that, the test case problems are resolved I guess? From our side, having a |
@danielinteractive @cicdguy Daniel, what you suggest is fine with me. Many thanks! The failing tests need to be commented out, though. They not only break the covr action but also the CHECK action. If you can identify them (help very much appreciated!), I am happy to have the covr action set up in GitHub once we know that the tests included in the package pass the CHECK action in all systems, and hopefuly also in CRAN. My aim is to keep the master branch in GitHub clean so that it remains possible to easily submit updates of the package to CRAN when needed. Even more importantly, as installation of the package by users from GitHub has been described in the documentation, the master brach (set as default) should not be broken on any of the supported systems. I appreciate Dinakar's help with covr. My point was that we need to get the tests working before seting up a covr action. Nothing more. Somehow, Dinakar and myself tried similar things almost concurrently. As I mentioned earlier the other question that I find surprising is that covr seems not to be detecting all statements that are actually being tested. This seems to make it impossible to get coverage as measured with covr as high as required. This may be caused by the way I have set up tests in the past or by a bug in 'covr'. RStudio calls, using three colon notation a function not exported from 'devtools'. My long-term aim has been to meet the quality standards of ROpenScience. On the other hand, I do package development mostly on my free time, even though I use the packages I develop at work. This causes some lag in my answers and being afraid of accidentally introducing anything that can become a maintenance burden in the future. This week has been a busy one, so, the late answers, and too short answers written late in the evening. |
cool, thanks Pedro. see #26 for the result |
@cicdguy Please, track recent changes that I made in master. These are needed to include the test cases in the package build. Please, also revert your commit with changes to DESCRIPTION as these changes should no longer be needed as I have revised the tests not to use 'sf' and 'maps'. Thanks! |
Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The coverage action is failing in the pull request, but should work in master. So, I approve it.
@cicdguy @danielinteractive Many thanks! Now the covr action is working. 'testthat' tests are also included in the build and run by the CHECK action. |
Adds code coverage GHA workflow to this package.