publiclab / plots2

a collaborative knowledge-exchange platform in Rails; we welcome first-time contributors! :balloon:
https://publiclab.org
GNU General Public License v3.0
958 stars 1.83k forks source link

Resolve coverage reporting issues with CodeCov #6290

Open jywarren opened 5 years ago

jywarren commented 5 years ago

After #5931, @kaustubh-nair opened #5954 to switch our test coverage reporting to CodeCov. But there is some issue with coverage esp with the separately threaded tests we have running now. We'd love help debugging/resolving this if anyone has some experience with Travis and coverage monitoring!

jywarren commented 5 years ago

Ideally we'll be starting to get a nice coverage graph like this!

image

In each PR. Of course, ideally it'd all be green 😄

SidharthBansal commented 4 years ago

@jywarren i think making this as a whole as a hall of fame task is a great idea (Maybe I am wrong. I think this task is removing code coverage errors from whole plots2 repo)

jywarren commented 4 years ago

Yes this'd be great!

On Fri, Dec 13, 2019 at 1:52 PM Sidharth Bansal notifications@github.com wrote:

@jywarren https://github.com/jywarren i think making this as a whole as a hall of fame task is a great idea (Maybe I am wrong. I think this task is removing code coverage errors from whole plots2 repo)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/plots2/issues/6290?email_source=notifications&email_token=AAAF6JZ6GFQLOVD5OKOMUYLQYPKXJA5CNFSM4IWSYACKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG247EY#issuecomment-565563283, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J5WY4G6JM4EFEUBGA3QYPKXJANCNFSM4IWSYACA .

SidharthBansal commented 4 years ago

Published

Uzay-G commented 4 years ago

Hey, I might work on this but was wondering, how can I test my changes to the code? Thanks!

Uzay-G commented 4 years ago

I noticed that for some reason the config file has a section omment instead of comment. This seems a bit trivial but maybe that is a cause of the problem. Also would anyone happen to know when the bot stopped working? That way I could try and pinpoint the problem.

Uzay-G commented 4 years ago

Also does the codecov config need to be in the master branch for the reports to work? I am just trying things out that might work, sorry if I am making gross mistakes :smile:

SidharthBansal commented 4 years ago

@jywarren @Uzay-G do we need some more changes? If there are only 3 lines of code change, I think this should be hard issue. If more work is needed then we can stick with HoF

SidharthBansal commented 4 years ago

Thanks Uzay-G for your help

Uzay-G commented 4 years ago

Thans @SidharthBansal. Yeah you can turn it into a hard one instead :+1:

Uzay-G commented 4 years ago

Now we just need to resolve the conflicts of #5954 and merge into master.

SidharthBansal commented 4 years ago

Please work on any other issue in the meantime. Jeff is on holidays.

jywarren commented 4 years ago

Just referencing a few issues together here. A recent example issue at https://github.com/publiclab/plots2/pull/6875#issuecomment-570288690

Issues possibly related to parallelization documented at https://github.com/publiclab/plots2/issues/5931

And follow-up notes and concerns from this process at #5954 ...

jywarren commented 4 years ago

In https://github.com/publiclab/plots2/pull/6789, coverage was to drop to 50%, but on rebasing, it went up to very little change and all passed. This makes me worry about the parallelization issue?

jywarren commented 4 years ago

OK, coming back here; i think things are OK... notes:

https://docs.codecov.io/docs/merging-reports says it can handle parallel reports already with no config needed.

https://docs.codecov.io/docs/unexpected-coverage-changes has some useful info too.

https://github.com/publiclab/plots2/pull/7061 was compaining that the diff coverage is not as high as the project (32% vs 80%) so I wonder @Uzay-G if we should give 1% flexibility on this, and how we configure this?

jywarren commented 4 years ago

Maybe we can lower the strictness here? https://docs.codecov.io/docs/codecovyml-reference#section-codecov

Also we can ignore some folders if we think that's appropriate...

jywarren commented 4 years ago

Yes, we can set threshold here! https://docs.codecov.io/docs/commit-status#section-threshold

Uzay-G commented 4 years ago

Yeah I think we need to make it more flexible. I will look at this later tonight when I can! :+1:

Uzay-G commented 4 years ago

Ok I can make a pull request for the threshold, do you think we should set it around 5-8%?

jywarren commented 4 years ago

Oh, i was thinking more like 1% - isn't it the "wiggle room" for the overall project coverage to change +/- as a result of the PR? But yes, let's try it out!

On Fri, Jan 3, 2020 at 3:51 PM Uzay-G notifications@github.com wrote:

Ok I can make a pull request for the threshold, do you think we should set it around 5-8%?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/plots2/issues/6290?email_source=notifications&email_token=AAAF6J3DXFDS24PVGF2MMC3Q36QOTA5CNFSM4IWSYACKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEICBXOY#issuecomment-570694587, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J4OTPO7ASGQCC7H76DQ36QOTANCNFSM4IWSYACA .

Uzay-G commented 4 years ago

Oh ok yeah alright! I'll make the pull request now for the threshold! :+1: