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

Change unittest to pytest #192

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Change unittest to pytest #192

merged 2 commits into from
Jan 8, 2025

Conversation

daklauss
Copy link
Collaborator

@daklauss daklauss commented Dec 3, 2024

  • Changed unittest to pytest
  • Use pytest.mark.slow to ignore to complicated tests

removed - [ ] Using Coverage for coverage report to also cover #144

@daklauss daklauss linked an issue Dec 3, 2024 that may be closed by this pull request
3 tasks
@daklauss daklauss self-assigned this Dec 3, 2024
@daklauss daklauss force-pushed the 189-migrate-testing-to-pytest branch from 645b820 to 31a4127 Compare December 3, 2024 11:39
@schmoelder
Copy link
Contributor

For reference, this is how we do it in CADET-Python: https://github.com/cadet/CADET-Python/blob/master/.github/workflows/pipeline.yml

@daklauss daklauss force-pushed the 189-migrate-testing-to-pytest branch 11 times, most recently from fbc1c10 to 7ab2fed Compare December 4, 2024 18:06
@daklauss daklauss changed the base branch from master to dev December 4, 2024 18:09
@daklauss daklauss force-pushed the 189-migrate-testing-to-pytest branch 2 times, most recently from 73f3404 to 3e7f58b Compare December 5, 2024 09:03
@daklauss daklauss changed the title Change unittest to pytest and using coverage Change unittest to pytest Dec 10, 2024
@daklauss daklauss force-pushed the 189-migrate-testing-to-pytest branch from 994a3a7 to da4976b Compare December 10, 2024 13:25
@daklauss
Copy link
Collaborator Author

daklauss commented Dec 10, 2024

Removed coverage for a later issue #144.
grafik
https://github.com/fau-advanced-separations/CADET-Process/actions/runs/12177180699/job/33964452874

Pytest with coverage as i used needs way to long for a simple push (~50%). Considering using it just on merge or action.

Pytest alone should work.

@daklauss daklauss requested a review from schmoelder December 10, 2024 13:44
pyproject.toml Outdated Show resolved Hide resolved
@schmoelder
Copy link
Contributor

This one here could also be marked as slow (for now):

@pytest.mark.parametrize("process", test_cases, indirect=True)

@daklauss daklauss force-pushed the 189-migrate-testing-to-pytest branch from da4976b to 2809904 Compare December 12, 2024 17:36
@daklauss
Copy link
Collaborator Author

Done

This one here could also be marked as slow (for now):

@pytest.mark.parametrize("process", test_cases, indirect=True)

and removed coverage from dependency.

@daklauss daklauss force-pushed the 189-migrate-testing-to-pytest branch 2 times, most recently from 8ad652f to 1aa2228 Compare January 6, 2025 09:14
@daklauss daklauss requested a review from schmoelder January 6, 2025 09:44
@daklauss daklauss force-pushed the 189-migrate-testing-to-pytest branch from fc25df1 to de162b9 Compare January 6, 2025 10:43
@daklauss
Copy link
Collaborator Author

daklauss commented Jan 6, 2025

Still many warnings, but more or less the same as in dev. Tests run overall slightly slower compared to unittest. Otherwise like in dev, now 220 tests were run, 3 skipped.

@daklauss daklauss requested a review from schmoelder January 6, 2025 12:31
@schmoelder
Copy link
Contributor

Any new warnings or simply the same as with unittest?

@daklauss
Copy link
Collaborator Author

daklauss commented Jan 6, 2025

Any new warnings or simply the same as with unittest?

As far as i can tell simply the same as with unittest. Pytest counts all warnings thrown in a run what unittest does not, so i don't have a metric there, but i have found the same warnings in the dev runs. Still found some __init__....

Copy link
Contributor

@schmoelder schmoelder left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor comments / Changes.

.github/workflows/pipeline.yml Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
tests/test_events.py Outdated Show resolved Hide resolved
@schmoelder
Copy link
Contributor

Alrighty, looks good to me. Please rebase and clean up the git history, then we can merge.

@daklauss daklauss force-pushed the 189-migrate-testing-to-pytest branch 2 times, most recently from 7b819d6 to d66ba21 Compare January 7, 2025 10:23
@daklauss
Copy link
Collaborator Author

daklauss commented Jan 7, 2025

Alrighty, looks good to me. Please rebase and clean up the git history, then we can merge.

Done

@daklauss daklauss force-pushed the 189-migrate-testing-to-pytest branch from d66ba21 to 1d4f723 Compare January 7, 2025 11:58
@daklauss daklauss force-pushed the 189-migrate-testing-to-pytest branch from 1d4f723 to 5ae6259 Compare January 7, 2025 12:32
@schmoelder schmoelder merged commit 961edf8 into dev Jan 8, 2025
4 checks passed
@schmoelder schmoelder deleted the 189-migrate-testing-to-pytest branch January 8, 2025 07:23
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.

Migrate testing to pytest
2 participants