Skip to content

refactor test setups towards fixtures and hinting#968

Merged
BoboTiG merged 17 commits intogorakhargosh:masterfrom
altendky:refactor_test_setups
Mar 15, 2023
Merged

refactor test setups towards fixtures and hinting#968
BoboTiG merged 17 commits intogorakhargosh:masterfrom
altendky:refactor_test_setups

Conversation

@altendky
Copy link
Copy Markdown
Contributor

@altendky altendky commented Mar 14, 2023

This is an attempt to switch to well hinted pytest fixtures that mypy and flake8 will understand with as little change to the actual tests as possible. This does not necessarily represent the pattern I might normally apply. That can come later, if at all.

Draft for:

  • feedback on the direction
  • application to other setup fixtures

@altendky altendky marked this pull request as draft March 14, 2023 14:07
@BoboTiG
Copy link
Copy Markdown
Collaborator

BoboTiG commented Mar 14, 2023

Well, that's a great job :)
Could we move all those stuff (fixtures + mypy-specific code) to conftest.py?

@BoboTiG
Copy link
Copy Markdown
Collaborator

BoboTiG commented Mar 14, 2023

FTR if it's too time consuming, it's also OK to skip Mypy in tests. It's up to you :)

@altendky
Copy link
Copy Markdown
Contributor Author

Yes, they are expected to move to conftest.py to be shared. I personally very much like to have tests fully hinted. It's, well, a first test of the hinting.

@BoboTiG
Copy link
Copy Markdown
Collaborator

BoboTiG commented Mar 14, 2023

Fully agree 👍🏻

@altendky altendky marked this pull request as ready for review March 14, 2023 22:00
@altendky altendky mentioned this pull request Mar 14, 2023
3 tasks
@BoboTiG
Copy link
Copy Markdown
Collaborator

BoboTiG commented Mar 14, 2023

It seems there are regressions in macOS tests.

FTR:

  • tests/test_0_watchmedo.py::test_auto_restart_subprocess_termination[False] is known to be a failure on Windows, and sometimes (quite often) on macOS too.
  • tests/test_0_watchmedo.py::test_auto_restart_on_file_change_debounce is a frequent failure on all Oses

@BoboTiG
Copy link
Copy Markdown
Collaborator

BoboTiG commented Mar 14, 2023

I think we could just skip pypy tests on Windows, they never make it (enven if it's temporary, maybe someone later will find some time/motivation to dig into that problem).

And now that I see soo many PRs (💪🏻) the release job being run every time may not be a good idea finally, WDYT?

@altendky
Copy link
Copy Markdown
Contributor Author

I think we could just skip pypy tests on Windows, they never make it.

I guess we could also document testing status and plans in a dedicated issue instead of email as I had initiated. Either way.

@altendky
Copy link
Copy Markdown
Contributor Author

And now that I see soo many PRs (💪🏻) the release job being run every time may not be a good idea finally, WDYT?

Personally, I wouldn't. I don't want to find that broken when I'm trying to make a release. But, I haven't really looked into that one to see what's going on there.

@BoboTiG
Copy link
Copy Markdown
Collaborator

BoboTiG commented Mar 14, 2023

I'll open issues for the tests 👍🏻

@altendky
Copy link
Copy Markdown
Contributor Author

To be clear, I meant that for general test work. Regressions here I'll just look at here.

@BoboTiG
Copy link
Copy Markdown
Collaborator

BoboTiG commented Mar 14, 2023

Yup, I got it :)

@altendky
Copy link
Copy Markdown
Contributor Author

I do expect to put some time into the state of the tests, but figured I'd wait until I heard something about the status. Even if that is 'they are having trouble and someone needs to do the research as to what they need because we don't know yet'.

@BoboTiG
Copy link
Copy Markdown
Collaborator

BoboTiG commented Mar 15, 2023

Ready to merge on my end.

@altendky
Copy link
Copy Markdown
Contributor Author

Should I add more xfails or flake retries for these tests? But, I'm not aware of anything else to be done here.

@BoboTiG
Copy link
Copy Markdown
Collaborator

BoboTiG commented Mar 15, 2023

It's OK then. Those tests are "pure randoms" if I may.

@BoboTiG BoboTiG merged commit 764a234 into gorakhargosh:master Mar 15, 2023
@BoboTiG
Copy link
Copy Markdown
Collaborator

BoboTiG commented Mar 15, 2023

That's an impressive job, thanks a lot @altendky 🍾

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.

2 participants