You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@singa.apache.org by GitBox <gi...@apache.org> on 2020/09/15 08:28:16 UTC

[GitHub] [singa] nudles opened a new issue #796: codecov/project fail

nudles opened a new issue #796:
URL: https://github.com/apache/singa/issues/796


   Pls check https://github.com/apache/singa/pull/795/checks?check_run_id=1115935742


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] chrishkchris edited a comment on issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
chrishkchris edited a comment on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-692609188


   @moazreyad 
   Codecov check if the code is covered by test case
   I think will need to update this ignore path of this file https://github.com/apache/singa/blob/dev/.codecov.yml
   
   Reference:
   https://docs.codecov.io/docs/ignoring-paths


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] chrishkchris removed a comment on issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
chrishkchris removed a comment on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-692609188


   @moazreyad 
   Codecov check if the code is covered by test case
   I think will need to update this ignore path of this file https://github.com/apache/singa/blob/dev/.codecov.yml
   
   Reference:
   https://docs.codecov.io/docs/ignoring-paths
   
   oh sorry I was wrong, it says the affected file is tensor.h


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] chrishkchris commented on issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
chrishkchris commented on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-692620610


   @moazreyad It is very strange that the test analyzes the coverage on tensor.h in this PR which is totally unrelated


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] chrishkchris edited a comment on issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
chrishkchris edited a comment on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-692609188


   @moazreyad 
   Codecov check if the code is covered by test case
   I think will need to update this ignore path of this file https://github.com/apache/singa/blob/dev/.codecov.yml
   
   Reference:
   https://docs.codecov.io/docs/ignoring-paths
   
   oh sorry I was wrong, it says the affected file is tensor.h


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] nudles commented on issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
nudles commented on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-693784461


   After fixing the python version issue, code coverage fails again..
   I didn't change the source code in this PR #795 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] moazreyad commented on issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
moazreyad commented on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-692659125


   There is a problem in this PR because it [installs singa in python 3.6 ](https://github.com/nudles/singa/blob/dev/.github/workflows/conda.yaml#L64) and [executes the test cases from python3.7/site-packages](https://github.com/nudles/singa/blob/dev/.github/workflows/conda.yaml#L66) folder which does not work. Please fix this different python versions problem.
   
   Note also that the project coverage threshold can be set in [.codecov.yml](https://github.com/apache/singa/blob/dev/.codecov.yml) like [this](https://docs.codecov.io/docs/commit-status#project-status):
   
   ```
   coverage:
     status:
       project:
         default:
           # basic
           target: auto
           threshold: 5%
   ```
   The value of threshold can be chosen by the development team based on the quality needs. In the best case, every PR should not reduce the coverage because this means it either added code without test case or it removed (or disabled) the test cases for existing code. However, if the team think this is too strict requirement, we may choose the threshold to 5% or even more. But this means that a PR can be allowed to pass even if it reduces the code coverage too much. 
   
   The PR #795 reduces the coverage by 4.97%, so setting the threshold to 5% should make it pass. But this is not recommended solution. We can use it only if the team thinks it is too strict to keep the coverage change always zero or positive. In this case, we may allow threshold of 1% or 2% to pass. But 5% seems high to me and it means there is a lot of code that was not tested and there is a problem. And thanks to the strict code coverage, we found the different python versions problem. Without the strict coverage check, may be this problem will be hidden and will cause other problems later.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] nudles commented on issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
nudles commented on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-694598973


   Thanks for the explanation. 
   I think we can let the reviewer to determine if the newly added code should be covered by unit tests.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] nudles commented on issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
nudles commented on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-693787747


   Seems it does not provide the suggestions on how to improve the code coverage.
   for example, #797 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] chrishkchris commented on issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
chrishkchris commented on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-692609188


   @moazreyad 
   Codecov check if the code is covered by test
   I think will need to update this ignore path of this file https://github.com/apache/singa/blob/dev/.codecov.yml


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] nudles closed issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
nudles closed issue #796:
URL: https://github.com/apache/singa/issues/796


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [singa] moazreyad commented on issue #796: codecov/project fail

Posted by GitBox <gi...@apache.org>.
moazreyad commented on issue #796:
URL: https://github.com/apache/singa/issues/796#issuecomment-694121140


   > After fixing the python version issue, code coverage fails again..
   
   The coverage is now decreased by 0.01% instead of 4.97%. It still fails because we did not specify a threshold, so any decrease any in the coverage will make test fails. To solve this problem, either add a small threshold like 0.1 % to allow small drops in coverage generally in all PRs, or leave the threshold decision to each pull request reviewer to decide if the decrease in the specific PR is fine and it can be merged or it is not fine and must be fixed.
   
   > Seems it does not provide the suggestions on how to improve the code coverage.
   
   Yes, it just reports the coverage results without suggestions on how to fix. This is similar to all the test cases which report the error but of course they do not usually suggest how to fix it. 
   
   To improve the coverage, the developer needs to investigate more how to create the suitable test cases to cover his new code, or how not to prevent current test cases from running (like the problem that we had in #795). 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org