Skip to content
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

inconsistencies with CodeCov rejections #810

Open
jywarren opened this issue Jul 5, 2019 · 12 comments · Fixed by #830 or #837 · May be fixed by #908
Open

inconsistencies with CodeCov rejections #810

jywarren opened this issue Jul 5, 2019 · 12 comments · Fixed by #830 or #837 · May be fixed by #908

Comments

@jywarren
Copy link
Member

jywarren commented Jul 5, 2019

Possibly in relation to #747, I'm seeing that some PRs are blocked if they have no tests, which is fine, but that when adding tests,

  1. we can't seem to get the Codecov reported coverage for the PR to change
  2. CodeCov's info seems inconsistent; initially this PR (Small UI fix for All maps #801) reflected a drop in coverage to the sessions controller,in CodeCov's comment. But after rebasing, now it shows a drop in the Annotations controller:

image

Why might CodeCov be varying in its reporting? cc @kaustubh-nair

Thank you!

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

I'm afraid this is blocking Mapknitter 3 launch, so I'm going to try to turn down the strictness of CodeCov blocking PRs for coverage... at least for now!

But I don't think it's really working properly -- for example, https://github.com/publiclab/mapknitter/pull/811/files really won't drop coverage by almost 5% -- that can't be right!

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

OK, so i turned patch off:

coverage:
  status:
    project: on
    patch: off

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

jywarren@9b92a90

@jywarren
Copy link
Member Author

jywarren commented Jul 5, 2019

OK, so for example, this doesn't make sense - we aren't substantially changing anything but an HTML fragment, but it's reporting a near 7% drop in coverage. I need to merge that PR to publish MapKnitter so I think i'll override with my admin privileges, because the 7% drop can't be correct.

https://codecov.io/gh/publiclab/mapknitter/compare/0d686d8073cd945b08ab7557ad93676be7ab58fc...e4268dc0efa0304b4fd57ab60706052674705610/diff

Thanks for any help!

@kaustubh-nair
Copy link
Member

This is really weird.. It makes sense that coverage shouldn't drop by two percent for #801 I generated the reports locally to confirm this.

@kaustubh-nair
Copy link
Member

But I think we should turn on patch and put project off. Since patch will denote the test coverage of the pull request. And project is the one failing in those affected pull requests.

@kaustubh-nair
Copy link
Member

Some files' coverage drops to 0%. Maybe there is some corruption with the reports? Maybe we could try using a different generator, if the problem still persists

@jywarren
Copy link
Member Author

jywarren commented Jul 8, 2019 via email

@kaustubh-nair
Copy link
Member

Reopening because we're still facing this in #867
There's a possibility we're experiencing race conditions since we've got tests for models in both functional and unit tests, but I've got no idea how to verify that. simplecov-ruby/simplecov#559

@kaustubh-nair
Copy link
Member

@alaxalves @publiclab/mapknitter-reviewers Need some help with this issue. Haven't found anything to solve it after a lot of trying
FYI this issue happens only on travis, and not locally. So there's a strong possibility it's related to splitting the tests and codecov is not properly merge those partial reports

@kaustubh-nair kaustubh-nair linked a pull request Aug 5, 2019 that will close this issue
@sashadev-sky
Copy link
Member

@kaustubh-nair this is a complete shot in the dark but remember this custom rake task file mapknitter used to use to run its tests: https://github.com/publiclab/mapknitter/blob/main/lib/tasks/test_unit.rake

Perhaps you can use it to verify if the problem is a race condition - by running with Travis the different configurations it offers?

I remember it helped uncover a previous race condition issue in our tests months ago!

@kaustubh-nair
Copy link
Member

Oooh might me a possibility!
Let me have a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants