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

add codecov #107

Merged
merged 15 commits into from
Jul 16, 2024
Merged

add codecov #107

merged 15 commits into from
Jul 16, 2024

Conversation

rrlamichhane
Copy link
Contributor

@rrlamichhane rrlamichhane commented Jul 16, 2024

Please refer to CONTRIBUTING.md for detailed instructions.

Summary

  • Enable codecov
  • Update python versions for CI containers
  • Update lock file

PR Checklist

  • My PR is less than 500 lines of code
  • I have added sufficient comment as docstrings in my code
  • I have made corresponding changes to the documentation
  • I have written unit-tests to test all of my code

Copy link

codecov bot commented Jul 16, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@rrlamichhane rrlamichhane marked this pull request as ready for review July 16, 2024 14:23
@rrlamichhane rrlamichhane requested review from a team as code owners July 16, 2024 14:23
Copy link
Contributor

@snova-zoltanc snova-zoltanc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome, thanks for adding code coverage!

I was looking through code coverage report and sometimes it is annoying like abstract methods are labeled as not being tested, and some edge case error messages are not being tested, is there any way to avoid these being labeled as not covered?

Also some files I don't want to test, for example my custom shuffling implementation it is indeterministic so hard to test, can we ignore these files somehow ?

image

@@ -1,4 +1,5 @@
[![CircleCI](https://dl.circleci.com/status-badge/img/gh/sambanova/generative_data_prep/tree/main.svg?style=svg)](https://dl.circleci.com/status-badge/redirect/gh/sambanova/generative_data_prep/tree/main)
[![codecov](https://codecov.io/gh/sambanova/generative_data_prep/graph/badge.svg?token=9CYRCUOOAO)](https://codecov.io/gh/sambanova/generative_data_prep)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image Looks like this banner is not displaying properly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'll update once this merges to main since it shows the status of main

@rrlamichhane
Copy link
Contributor Author

@snova-zoltanc I'm not sure if we can ignore that but I can look into it in the future.

Also, I don't think its too difficult to cover them, we just need to parametrize one pytest with all these callbacks and see if the appropriate exceptions are raise.

@rrlamichhane rrlamichhane merged commit e9ac8d4 into main Jul 16, 2024
5 checks passed
@rrlamichhane rrlamichhane deleted the feature/enable-codecov-reporting branch July 16, 2024 18:00
@snova-zoltanc
Copy link
Contributor

@snova-zoltanc I'm not sure if we can ignore that but I can look into it in the future.

Also, I don't think its too difficult to cover them, we just need to parametrize one pytest with all these callbacks and see if the appropriate exceptions are raise.

Sounds great, I may ask for some recommendations on this if we plan to increase code coverage in the future.

@rrlamichhane
Copy link
Contributor Author

@snova-zoltanc sounds good; for sure! always happy to help when I can

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.

3 participants