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

Refactor filechoosertest_unicode.py #8485

Merged
merged 1 commit into from Dec 8, 2023

Conversation

Julian-O
Copy link
Contributor

@Julian-O Julian-O commented Nov 30, 2023

Was thinking about making changes to filechooser.py, but was not happy with the state of the existing tests.

This refactor includes:

  • Moving imports to the top of the file.
  • Removing unused imports. Importing names in a consistent way.
  • Importing platform from where it is defined in kivy.utils, rather than kivy where it is explicitly not exported.
  • Continue to skip the tests on some platforms, but use pytest.mark.skipif to do so, so pytest knows it has been skipped.
  • Copy-edit comment
  • bfile testing was only doing anything on Python 2. Removed.
  • Renamed variables to be more meaningful now there is not unicode versus bytes.
  • Ran through black so the formatting is neater.
  • Removed spurious for i in range(1):
  • Corrected misspelling exitsfiles
  • Specified exceptions to ignore.
  • Simplified deletion, and made it more robust - one file failure won't stop rest being deleted.

[There was no attempt to improve coverage here - filechooser.py is over 1,000 lines long, and is embarrassingly undertested.]

Maintainer merge checklist

  • Title is descriptive/clear for inclusion in release notes.
  • Applied a Component: xxx label.
  • Applied the api-deprecation or api-break label.
  • Applied the release-highlight label to be highlighted in release notes.
  • Added to the milestone version it was merged into.
  • Unittests are included in PR.
  • Properly documented, including versionadded, versionchanged as needed.

Was thinking about making changes to filechooser.py, but was not happy with the state of the existing tests.

This refactor includes:

* Moving imports to the top of the file.
* Removing unused imports. Importing names the same way.
* Importing platform from where it is defined in `kivy.utils`, rather than `kivy` where it is explicitly not exported.
* Continue to skip the tests on some platforms, but use `pytest.mark.skipif` to do so, so pytest knows it has been skipped.
* Copy-edit comment
* bfile testing was only doing anything on Python 2. Removed.
* Renamed variables to be more meaningful now there is not unicode versus bytes.
* Ran through `black` so the formatting is neater.
* Removed spurious `for i in range(1):`
* Corrected misspelling `exitsfiles`
* Specified exceptions to ignore.
* Simplified deletion, and made it more robust - one file failure won't stop rest being deleted.

[There was no attempt to improve coverage here - filechooser.py is over 1,000 lines long, and is embarrassingly undertested.]
@misl6 misl6 changed the title Refactor filechoosertest_unicode.py Refactor filechoosertest_unicode.py Dec 8, 2023
@misl6 misl6 added the Component: tests/CI Tests, CI, GitHub settings label Dec 8, 2023
@misl6 misl6 added this to the 2.3.0 milestone Dec 8, 2023
Copy link
Member

@misl6 misl6 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@misl6 misl6 merged commit 670cb0c into kivy:master Dec 8, 2023
34 checks passed
@Julian-O Julian-O deleted the refactorfilechoosertest branch December 8, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: tests/CI Tests, CI, GitHub settings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants