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

Improve documentation for python_test_utils #14739

Merged

Conversation

Eric-Arellano
Copy link
Contributor

@Eric-Arellano Eric-Arellano commented Mar 9, 2022

@compyman helpfully pointed out that the original docs were confusing with the difference between python_sources vs python_test_utils.

[ci skip-rust]
[ci skip-build-wheels]

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@Eric-Arellano Eric-Arellano requested review from stuhood and benjyw March 9, 2022 01:13
"files vs. production files."
"This target generator is intended for test utility files like `conftest.py` or "
"`my_test_utils.py`. Technically, it generates `python_source` targets in the exact same "
"way as the `python_sources` target generator does, only that the `sources` field has a "
Copy link
Contributor

Choose a reason for hiding this comment

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

Why bother explaining this technicality at all? Isn't it an implementation detail?

Copy link
Contributor Author

@Eric-Arellano Eric-Arellano Mar 9, 2022

Choose a reason for hiding this comment

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

Because our default source globs will not automatically capture my_test_utils.py; you can name test util code anything you want, we can't set a sensible default. So you have to go out of your way to set sources for python_test_utils if you want to separate prod code from test util code.

This paragraph attempts to explain why you might want to do that, but how it's also not strictly necessary.

Fyi the only reason the separation really matters is that depending on a target generator is an alias for depending on all generated targets. If you still want to use the alias mechanism, but w/o the test code.

Copy link

@compyman compyman left a comment

Choose a reason for hiding this comment

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

looks good to me!

@Eric-Arellano Eric-Arellano merged commit 08a4b04 into pantsbuild:main Mar 9, 2022
@Eric-Arellano Eric-Arellano deleted the better-python-test-utils-docs branch March 9, 2022 17:55
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this pull request Mar 9, 2022
[ci skip-rust]
[ci skip-build-wheels]
Eric-Arellano added a commit that referenced this pull request Mar 9, 2022
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