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

Improve Code Coverage/Add to CI #3475

Open
michael-hawker opened this issue Sep 14, 2020 · 6 comments
Open

Improve Code Coverage/Add to CI #3475

michael-hawker opened this issue Sep 14, 2020 · 6 comments

Comments

@michael-hawker
Copy link
Member

thoughts on this PR? Should we investigate code coverage on the unit tests for these types of detailed scenarios to make sure we've got all cases exercised? I don't think we have a nice code-coverage thing setup as part of the Toolkit anywhere, eh?

@michael-hawker Regarding this, as a side note, I saw that ReSharper includes a Unit Test Coverage tool that's quite easy to use. It's not perfect (eg. it sometimes skips a line, and it doesn't seem to deal with compiler directives properly, but AFAIK code coverage tool are known not to be 100% accurate), but it should be nice to at least have a point of reference to check locally until we eventually setup some CI tool for this as well, as you mentioned. Here's what I'm getting right now on master:

image

I can definitely make a PR to improve the coverage even more 😊
As a path forward, I was thinking something like:

  • Use this Test Coverage tool individually on the new added features to ensure the new APIs being added are properly covered
  • Use this on master and make a separate PR to fill in the missing tests for the already existing APIs

How does this sound? Just not sure when to make the various PRs to minimize conflicts, assuming that's a concern.
As in, should we maybe merge at least #3351 before branching off from master again for this new PR? 🤔

Let me know what you think!

Originally posted by @Sergio0694 in #3380 (comment)

@ghost ghost added the needs triage 🔍 label Sep 14, 2020
@ghost
Copy link

ghost commented Sep 14, 2020

Hello michael-hawker, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@michael-hawker
Copy link
Member Author

To clarify, I don't think the CI should be gated on this; however, a report should be generated so we can inspect it easily.

@Sergio0694
Copy link
Member

@michael-hawker Just as an FYI (not sure if it might be useful as a reference), ImageSharp is using the CodeCov bot for this.
For instance, here's a post that it generated in one of my PRs: SixLabors/ImageSharp#1314 (comment)

image

I mean that one might be a bit overkill, but I thought I'd mention it 😄

@michael-hawker
Copy link
Member Author

@Sergio0694 that's pretty cool, it's this one right? @azchohfi thoughts? Seems like an easy win?

@Sergio0694
Copy link
Member

@michael-hawker Yup, that's the one! 👍

@azchohfi
Copy link
Contributor

This one is a github extension, not an azure devops, but it is possible to do the same in azure devops. As long as it exports a cobertura file (lets avoid JaCoCo), we can easily integrate it with ADO.

@michael-hawker michael-hawker modified the milestones: 7.0, 7.1 Jan 6, 2021
@ghost ghost modified the milestone: 7.1 Jan 6, 2021
@michael-hawker michael-hawker modified the milestones: 7.1, 7.2/8.0? Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants