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 test coverage #148

Merged
merged 7 commits into from
Nov 18, 2021
Merged

Add test coverage #148

merged 7 commits into from
Nov 18, 2021

Conversation

ziegenberg
Copy link
Collaborator

Adds test coverage data

@ziegenberg ziegenberg added the bug This issue/PR relates to a bug. label Oct 28, 2021
Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
@ziegenberg ziegenberg force-pushed the add-test-coverage branch 2 times, most recently from 3de8eac to 84992b8 Compare October 30, 2021 19:37
@ziegenberg ziegenberg marked this pull request as ready for review October 30, 2021 20:00
@ziegenberg ziegenberg requested a review from ssbarnea as a code owner October 30, 2021 20:00
@ziegenberg
Copy link
Collaborator Author

This is ready for review.

@ziegenberg ziegenberg changed the title [WIP] Add test coverage Add test coverage Oct 30, 2021
Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
@ssbarnea
Copy link
Member

ssbarnea commented Nov 2, 2021

If you want to complete the coverage enablement, try to also upload the data to codecov.io -- You can inspire from https://github.com/ansible-community/ansible-compat/blob/main/.github/workflows/tox.yml#L217-L222 -- it is a little bit dated but still works.

I will enable codecov integration and we will see jobs reporting when coverage goes down.

@ziegenberg
Copy link
Collaborator Author

I will try to add the according workflow steps.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
@ziegenberg
Copy link
Collaborator Author

I will look into this a bit later this week.

@ssbarnea
Copy link
Member

ssbarnea commented Nov 3, 2021

I will look into this a bit later this week.

No hurry, fixing stuff in small steps is the best approach as we never know how much time we have available. I mentioned that only for follow-ups, it was not a required for enablement.

Still, this needs to report green to merge it.

Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

Cannot really review if CI is reporting red.

@ziegenberg
Copy link
Collaborator Author

I'll revert 9387334 for now and will add this via it's own PR later on.

@ziegenberg ziegenberg requested a review from ssbarnea November 18, 2021 13:06
@ssbarnea ssbarnea merged commit 3be927a into pycontribs:main Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants