-
-
Notifications
You must be signed in to change notification settings - Fork 631
feat: add py_pytest_main entrypoint #723
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
Conversation
python/py_pytest_main/pytest.tmpl.py
Outdated
print("Pytest exit code: " + str(exit_code)) | ||
print("Ran pytest.main with " + str(args)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
print("Pytest exit code: " + str(exit_code)) | |
print("Ran pytest.main with " + str(args)) | |
print("Pytest exit code: " + str(exit_code), file=sys.stderr) | |
print("Ran pytest.main with " + str(args), file=sys.stderr) |
I may be missing a reason that we wouldn't send error logs to stderr. But normally you should.
ctx.actions.expand_template( | ||
template = ctx.file.template, | ||
output = ctx.outputs.out, | ||
substitutions = dict(substitutions, **ctx.var), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this **ctx.var
overriding get used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Via --define
generally
@@ -1 +1,2 @@ | |||
requests==2.25.1 | |||
pytest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not pin the version, at least to ~=
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can do, given this is locked by pip_compile and it won't change we shouldn't really care about the version of pytest, so the open ended version here seemed fine, but I'm not strongly opinionated about it.
import pytest | ||
|
||
if __name__ == "__main__": | ||
$$CHDIR$$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a benefit/difference to subbing in an os.chdir(...)
string here vs. passing the directory as the first argument to pytest.main
? (Passing as 1st arg is how we do it at our company)
# Generates an entrypoint for pytest that is picked up by Gazelle for the py_test target below. | ||
py_pytest_main( | ||
name = "__test__", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come py_pytest_main
becomes a dependency to a py_test
, instead of subsuming the py_test
rule?
Wholly replacing the py_test
rule with a macro/rule/rule+macro seems a cleaner user interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This provides an entry point for pytest, the Python test runner, and doesn't force changing loads or existing rules, and keepspy_test
the bazel rule a generic "run a python binary that will return 0 or 1". There are other launchers, perhaps a nose entrypoint etc.
This model also fits with the gazelle generation already present here.
Well that merge went wrong :( |
0213618
to
3f10990
Compare
Signed-off-by: Thulio Ferraz Assis <[email protected]>
What is the status of this PR, is it still being worked on? |
Following the decision that rules_python will not be test-framework-specific, this effort landed in aspect-build/rules_py@6b1fcb7. |
Adds a
py_pytest_main
entrypoint rule for templating a launcher for pytest that works in conjunction with the Gazelle extension.