feat: port pytest_test macro #401
Conversation
Comes from caseyduquettesc/rules_python_pytest#13 It's much easier to properly gazelle-generate a BUILD file in this shape rather than the awkward way you're forced to hold our py_pytest_main.
| @@ -0,0 +1,71 @@ | |||
| """A shim for executing pytest that supports test filtering, sharding, and more. | |||
|
|
|||
| Copied from https://github.com/caseyduquettesc/rules_python_pytest/blob/331e0e511130cf4859b7589a479db6c553974abf/python_pytest/pytest_shim.py | |||
There was a problem hiding this comment.
FYI @caseyduquettesc - thank you for writing this!
| if os.environ.get("XML_OUTPUT_FILE"): | ||
| pytest_args.append("--junitxml={xml_output_file}".format(xml_output_file=os.environ.get("XML_OUTPUT_FILE"))) | ||
|
|
||
| # Handle test sharding - requires pytest-shard plugin. |
There was a problem hiding this comment.
Can the presence of the plugin be detected, and an error printed if it's missing?
There was a problem hiding this comment.
Can I file and leave as a follow-up? Right now I'm looking for parity with v1.1.1 https://registry.bazel.build/modules/caseyduquettesc_rules_python_pytest
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| pytest_args = ["--ignore=external"] |
There was a problem hiding this comment.
The other template had different args around disabling caching, check if they are needed here.
| ] + srcs, | ||
| main = shim_label, | ||
| args = [ | ||
| "--capture=no", |
There was a problem hiding this comment.
What's the reason to have this defined here, rather than in the shim?
There was a problem hiding this comment.
I don't know, I didn't write this code. Maybe Casey is willing to be the PR author.
There was a problem hiding this comment.
This absolutely can be in the shim. In the very beginning, the shim was a trivial wrapper around pytest.main and adding default arguments here was less code than in the shim. As more pytest features were hooked up to native Bazel features, the shim gathered more argument parsing logic and I simply missed calling this out in the pull request that first began adding default arguments in the shim. I see no reason this shouldn't be part of pytest_args = ["--ignore=external"] in the shim.
| main = shim_label, | ||
| args = [ | ||
| "--capture=no", | ||
| ] + args + ["$(location :%s)" % x for x in srcs], |
There was a problem hiding this comment.
Do these automatically spill? The arg limit length could be easily hit here.
|
|
||
| if pytest_deps == None: | ||
| pytest_deps = ["@{}//pytest".format(pip_repo)] | ||
| if kwargs.get("shard_count", 1) > 1: |
There was a problem hiding this comment.
The 1 seems unnecessary? Check for None, which would be falsey?
There was a problem hiding this comment.
The user can explicitly say shard_count = 1 which I hoped to account for
| args = [ | ||
| "--capture=no", | ||
| ] + args + ["$(location :%s)" % x for x in srcs], | ||
| deps = deps + pytest_deps, |
There was a problem hiding this comment.
Might want to swap the ordering, so that this levels pytest dependencies will "win" over any defined transitively (perhaps there is a test helper as a py_library somewhere in the graph that consumes a different pytest version)
| else: | ||
| pytest_args.extend(args) | ||
|
|
||
| print(pytest_args, file=sys.stderr) |
There was a problem hiding this comment.
This seems a little spammy, print it if there's some flag set?
|
I have very little time to prepare a talk for Monday and I think you're expecting a higher level of polish than Casey had, so I'm going to try the bazelbuild/rules_python#2044 approach instead |
Comes from caseyduquettesc/rules_python_pytest#13 It's much easier to properly gazelle-generate a BUILD file in this shape rather than the awkward way you're forced to hold our py_pytest_main.