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

[KED-950] Turn on the branch coverage for unit tests #267

Closed
wants to merge 4 commits into from

Conversation

Cheukting
Copy link

Description

According issue #243 branch test on coverage needed to be turned on

Development notes

  • branch test has been added on .coveragerc
  • Adding test to bump coverage back to 100% (WIP)

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change and added my name to the list of supporting contributions in the RELEASE.md file
  • Added tests to cover my changes

Notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.

  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.

  • I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

@921kiyo
Copy link
Contributor

921kiyo commented Feb 28, 2020

@Cheukting Thank you for working on this! Let us know if you need any help/questions on this :)

@neomatrix369
Copy link
Contributor

@Cheukting proud of your work! Always learning so quickly!

@yetudada yetudada changed the title [WIP] Adding branch testing (C1) to coverage (gh-issue #243) [Draft] [KED-950] Turn on the branch coverage for unit tests Mar 9, 2020
@yetudada
Copy link
Contributor

Hey @Cheukting 😄 We want to review your PR and merge it in for the next release. Is it ready to come out of draft mode?

@Cheukting
Copy link
Author

Cheukting commented Apr 16, 2020

Hey @yetudada sorry for leaving it open for so long. I am afraid there are too many tests (more than I can handle) that are needed to bump the new coverage to 100% Please feel free to assign it to someone else or close it if you see fit.

@neomatrix369
Copy link
Contributor

neomatrix369 commented Apr 16, 2020

Hey @yetudada sorry for leaving it open for so long. I am afraid there are too many tests (more than I can handle) that are needed to bump the new coverage to 100% Please feel free to assign it to someone else or close it if you see fit.

I'm happy to help but sounds like it's many hands needed to help finish the task, so I will try to do as much as I can.

@Cheukting do you mind zipping the code-coverage folder generated by coverage or pycov and share it here (drop the zip file into the comment) so I can take a quick look at it and find low-hanging fruits and the inflection points.

@Cheukting
Copy link
Author

@neomatrix369 can you just pull my commits?

@mzjp2
Copy link
Contributor

mzjp2 commented Apr 16, 2020

You should be able to see the coverage output here: https://circleci.com/gh/quantumblacklabs/kedro/8974

@neomatrix369
Copy link
Contributor

neomatrix369 commented Apr 16, 2020

@neomatrix369 can you just pull my commits?

@Cheukting I have cloned your pull request now, thanks. I'll take it from here.

@neomatrix369
Copy link
Contributor

You should be able to see the coverage output here: https://circleci.com/gh/quantumblacklabs/kedro/8974

@mzjp2 I don't see a coverage html or xml report generated after the tests finish, this is useful for local/remote machine consumption. Happy to use the link you shared in the meanwhile.

@mzjp2
Copy link
Contributor

mzjp2 commented Apr 16, 2020

You should be able to see the coverage output here: https://circleci.com/gh/quantumblacklabs/kedro/8974

@mzjp2 I don't see a coverage html or xml report generated after the tests finish, this is useful for local/remote machine consumption. Happy to use the link you shared in the meanwhile.

Gotcha. If you run make test yourself locally it should generate a xml report (and a .coverage file too, I think?) but you're right that on CI it just shows the CLI output (it does generate the report but we don't surface it as an artifact on our CI build).

@neomatrix369
Copy link
Contributor

You should be able to see the coverage output here: https://circleci.com/gh/quantumblacklabs/kedro/8974

@mzjp2 I don't see a coverage html or xml report generated after the tests finish, this is useful for local/remote machine consumption. Happy to use the link you shared in the meanwhile.

Gotcha. If you run make test yourself locally it should generate a xml report (and a .coverage file too, I think?) but you're right that on CI it just shows the CLI output (it does generate the report but we don't surface it as an artifact on our CI build).

No worries, I'll take care of it and see what I can do, I have done something similar in the past, a HTML based report is a lot more productive to work with, and we can close the PR quicker ;)

@yetudada yetudada changed the title [Draft] [KED-950] Turn on the branch coverage for unit tests [KED-950] Turn on the branch coverage for unit tests May 18, 2020
@mzjp2
Copy link
Contributor

mzjp2 commented May 18, 2020

I'll go ahead and close this for now, seeing as the work has been overtaken and done in #327 -- thanks all! :)

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

Successfully merging this pull request may close these issues.

5 participants