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

Enable coverage in CI #992

Merged
merged 15 commits into from
Aug 16, 2022
Merged

Enable coverage in CI #992

merged 15 commits into from
Aug 16, 2022

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Aug 12, 2022

Related to #977.

Draft mode since it's not close to be reviewed :D

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 12, 2022

The documentation is not available anymore as the PR was closed or merged.

@Wauplin Wauplin marked this pull request as ready for review August 16, 2022 09:51
@Wauplin
Copy link
Contributor Author

Wauplin commented Aug 16, 2022

Update: code coverage is now working as expected on the ./src/huggingface_face folder !

Coverage is measured at ~80% (see codecov report). At the moment, most branch/compare sections of codecov are broken. I suspect this is because it tries to compare against main branch that has cov reports only on the tests/ folder. I tried to "erase repo coverage content" from the settings page since previous reports were unrelevant but I think I don't have the rights to do it.

I also made a refactoring of python-tests.yml workflow to leverage even more the github matrix and avoid duplication of steps. I think it would be easier to review it directly from the file instead of looking at the diff. In theory I kept exactly the same steps for each build (+add the cov reports).

@LysandreJik @osanseviero @adrinjalali could you please have a look at it ? Thanks in advance 🙂 🙏

EDIT: I also enabled comments from codecov in PRs. Let's see and if it's too annoying we will remove them.

@Wauplin
Copy link
Contributor Author

Wauplin commented Aug 16, 2022

The workaround I used to get proper coverage is to run tests from the ./src folder and using python -m pytest instead of pytest. My understanding is that it "forces" python to import huggingface_hub from the current directory in priority (since ./src/huggingface_hub has same name) instead of the installed one.

Then I updated the "./tests/" target to "../tests/" and created a "../coverage.xml" report.

@Wauplin Wauplin changed the title [WIP] enable coverage in CI Enable coverage in CI Aug 16, 2022
Copy link
Contributor

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

BTW, the paths-ignore could also be removed, we've moved those folders out of this repo.

Thankss a bunch for this. Let's see what it does then.

.github/workflows/python-tests.yml Show resolved Hide resolved
codecov.yml Show resolved Hide resolved
@adrinjalali
Copy link
Contributor

I tried to "erase repo coverage content" from the settings page since previous reports were unrelevant but I think I don't have the rights to do it.

I couldn't figure out how to give you those permissions, but I deleted the content there.

@Wauplin Wauplin merged commit 880408a into main Aug 16, 2022
@Wauplin Wauplin deleted the 977-enable-code-coverage-in-ci branch August 16, 2022 16:10
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