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

Replace unittest with pytest #90

Merged
merged 2 commits into from
Feb 9, 2024
Merged

Replace unittest with pytest #90

merged 2 commits into from
Feb 9, 2024

Conversation

Alopalao
Copy link

@Alopalao Alopalao commented Feb 7, 2024

Closes #88

Summary

Replace unittest with pytest

Local Tests

Run tox

End-to-End Tests

N/A

@Alopalao Alopalao requested a review from a team as a code owner February 7, 2024 20:36
@Alopalao Alopalao marked this pull request as draft February 7, 2024 20:39
@Alopalao Alopalao marked this pull request as ready for review February 8, 2024 23:53
@Alopalao
Copy link
Author

Alopalao commented Feb 8, 2024

Overall number of test have been reduced drastically from 400 to 247 but the percentage of coverage is the same. This is because every child of tests/unit/test_utils.py now does not call a second time every test from its parent. Is this correct?

@viniarck
Copy link
Member

viniarck commented Feb 9, 2024

Overall number of test have been reduced drastically from 400 to 247 but the percentage of coverage is the same. This is because every child of tests/unit/test_utils.py now does not call a second time every test from its parent. Is this correct?

Prev  (stmt / miss / percent) : 5990    373    94%
After (stmt / miss / percent) : 5795    375    94%

@Alopalao indeed the number of displayed collected test cases changed, but yes, coverage is equivalent, maybe with unitest.TestCase it was counting more than once in certain cases due to inheritance base class also being a test.

I've also looked into a detailed diff comparing the execution and I noticed no relevant difference. By the way, pretty neat how StructTest ended up, relatively minimal refactoring in that part while still compatible.

Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

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

Excellent PR, Aldo.

LGTM. This will be shipped soon.

@viniarck viniarck merged commit 2e6ef12 into master Feb 9, 2024
2 checks passed
@viniarck viniarck deleted the chore/pytest branch February 9, 2024 17:01
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.

chore: Replace TestCase with pytest test suite or test case
2 participants