Skip to content

Refactor tests/pytype_test.py#3065

Merged
JelleZijlstra merged 15 commits intopython:masterfrom
Eric-Arellano:refactor-pytype
Jun 19, 2019
Merged

Refactor tests/pytype_test.py#3065
JelleZijlstra merged 15 commits intopython:masterfrom
Eric-Arellano:refactor-pytype

Conversation

@Eric-Arellano
Copy link
Contributor

While working on #3062, the tests/pytype_test.py file has been failing and it was realized that the file could be refactored (#3062 (comment)).

This does the following to try to improve the readability while preserving all original behavior:

  • Mark as Python 3.
  • Use .format() instead of % for string interpolation.
  • Apply Black and isort.
  • Add type hints.
  • Use raise SystemExit() rather than manually managing return codes and calling sys.exit().
  • Move the high level logic into main() instead of pytype_test().
    • Make run_pytype() less generic. It no longer accepts generic args, because it does not need that much flexibility. This allowed removing _parse().
    • Extract out check_subdirs_discoverable() and check_python_exes_runnable.
    • Extract out determine_files_to_test().
  • Invert conditionals for early returns in loops to reduce indentation.
  • Add a print() statement to run with --print-stderr upon failures for more useful output.

It's much easier to understand pytype.io than io, which was shadowing the builtin!
* Set shebang
* Classes are always new type
Results in less implementation details being exposed at the top level of the module.
It makes that function a little bit easier to understand. We don't need to keep re-evaluating the value every time.
… so that pytype_test() is more focused

The logic belonged in main(). In the process, those two functions are refactored.
It is only ever used to do one thing. This allows us to remove the _parse() helper function.
@JelleZijlstra JelleZijlstra requested a review from rchen152 June 17, 2019 03:34
Copy link
Collaborator

@rchen152 rchen152 left a comment

Choose a reason for hiding this comment

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

Thanks for the refactor! I noticed one typo and one issue with the --dry-run command.

@JelleZijlstra JelleZijlstra merged commit 1763f51 into python:master Jun 19, 2019
@Eric-Arellano Eric-Arellano deleted the refactor-pytype branch June 19, 2019 22:20
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