added codecov GH action

bashir2 commented 4 months ago

Description of what I changed

Attempting to address issue #666.

E2E test

TESTED:

Ran mvn locally and verified that the Jacoco report generated locally makes sense.

Checklist: I completed these to help reviewers :)

[x] I have read and will follow the review process.
[x] I am familiar with Google Style Guides for the language I have coded in.

No? Please take some time and review Java and Python style guides.
[x] My IDE is configured to follow the Google code styles.

No? Unsure? -> configure your IDE.
[ ] I have added tests to cover my changes. (If you refactored existing code that was well tested you do not have to add tests)
[x] I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.
[x] All new and existing tests passed.
[x] My pull request is based on the latest changes of the master branch.

No? Unsure? -> execute command git pull --rebase upstream master

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 50.16%. Comparing base (a57f13b) to head (4bf1cf0). Report is 154 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #973 +/- ## ============================================= + Coverage 34.04% 50.16% +16.11% - Complexity 291 606 +315 ============================================= Files 71 86 +15 Lines 3818 5073 +1255 Branches 437 632 +195 ============================================= + Hits 1300 2545 +1245 + Misses 2397 2282 -115 - Partials 121 246 +125 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bashir2 commented 4 months ago

@chandrashekar-s please note that the numbers reported by Codecov are not accurate but I am guessing it is due to having a bad baseline. I like to merge this PR since the local Jacoco report makes sense; we will see if in future PRs the Codecov issue will be fixed or not.

chandrashekar-s commented 4 months ago

Hi @bashir2 thanks for the changes. I see that there are couple of errors during the Upload coverage to codecov phase of codecov run, can you please have a look.

bashir2 commented 4 months ago

Hi @bashir2 thanks for the changes. I see that there are couple of errors during the Upload coverage to codecov phase of codecov run, can you please have a look.

Thanks @chandrashekar-s for the review and for noting the errors. I am not sure what the root cause of 500s are but when I was debugging this on Friday, there were certainly runs with no such errors. For example this is a codecov action run with no such errors (done before the Dockerfile commit). As commented above, my guess is that these are due to having a bad baseline, but I am not sure. So if you don't mind, I am going to merge this and if the issue does not go away, I'll resume debugging codecov's upload issue.

chandrashekar-s commented 4 months ago

Hi @bashir2 thanks for the changes. I see that there are couple of errors during the Upload coverage to codecov phase of codecov run, can you please have a look.

Thanks @chandrashekar-s for the review and for noting the errors. I am not sure what the root cause of 500s are but when I was debugging this on Friday, there were certainly runs with no such errors. For example this is a codecov action run with no such errors (done before the Dockerfile commit). As commented above, my guess is that these are due to having a bad baseline, but I am not sure. So if you don't mind, I am going to merge this and if the issue does not go away, I'll resume debugging codecov's upload issue.

Sure, please go ahead and merge the PR. Also, I see that the latest codecov build is successful and the test coverage reported now is ~50%. The incremental report is not accurate as of now, may be because as you pointed out the base report is lagging behind a lot.

LovjeetVyas commented 4 months ago

@bashir2 , @chandrashekar-s guys needed your help with the authentication I am stuck in, actually in token response am getting a key expires_in that is in string "3600" but google oauth2 requires it in long and thus its generatig a error after "fetching the first batch of (resouce)" in buildfhirsearchpipeline as when i traced it back in the fetchUtil there is tokenresponse that is requiring expiresinsecond that is our expires_in field that is in string format but it requires it in long as per google's oauth2 doc thus generating a value error and that field being catched as illegalargument

if you guys can make some time for this it would turn out to be much helpful for me

chandrashekar-s commented 4 months ago

@bashir2 , @chandrashekar-s guys needed your help with the authentication I am stuck in, actually in token response am getting a key expires_in that is in string "3600" but google oauth2 requires it in long and thus its generatig a error after "fetching the first batch of (resouce)" in buildfhirsearchpipeline as when i traced it back in the fetchUtil there is tokenresponse that is requiring expiresinsecond that is our expires_in field that is in string format but it requires it in long as per google's oauth2 doc thus generating a value error and that field being catched as illegalargument

if you guys can make some time for this it would turn out to be much helpful for me

Hi @LovjeetVyas, I have replied to your query in this issue. Also, can you please file a new issue, so that it can be discussed there.

google / fhir-data-pipes