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

Re-write flag values to place generated files in undeclared outputs dir #7

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

amartani
Copy link
Contributor

@amartani amartani commented Aug 5, 2023

Some common pytest plugins, such as pytest-reportlog, pytest-json-report and pytest-html, allow the user to generate reports, accepting a file path as a flag. In Bazel, these generated files can only be accessed if placed inside the test undeclared outputs dir. However, bazel doesn't provide any way to expand environment variable values provided via args.

This commit rewrites flag values for the plugins mentioned above, prepend the test undeclared output dir folder to the value of the corresponding flags. This allows passing relative paths to these flags (via args = [] or via --test_arg), and the shim fixes the flag values.

Some common pytest plugins, such as pytest-reportlog, pytest-json-report and pytest-html, allow the user to generate reports, accepting a file path as a flag. In Bazel, these generated files can only be accessed if placed inside the test undeclared outputs dir. However, bazel doesn't provide any way to expand environment variable values provided in args.

This commit rewrites flag values for the plugins mentioned above, prepend the test undeclared output dir folder to the value of the corresponding flags.
Copy link
Owner

@caseyduquettesc caseyduquettesc left a comment

Choose a reason for hiding this comment

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

Great feature!

@caseyduquettesc caseyduquettesc merged commit 5b8b76a into caseyduquettesc:main Aug 7, 2023
7 checks passed
@amartani
Copy link
Contributor Author

amartani commented Aug 8, 2023

@caseyduquettesc Thanks for accepting the PR!

Small question - when trying out this with pytest-html, I noticed that the html report didn't contain captured output, and this happened due to the rule setting --capture=no.

So, in my repo, I override it back (args = ["--capture=fd"]). But I am curious on why --capture=no was added, since --capture=fd seems to work fine for me. Is there any gotchas that you found with it?

@caseyduquettesc
Copy link
Owner

@amartani Yeah there is a gotcha. If output is captured, --test_output=streamed won't work, or any of the test output options because bazel no longer controls the output.

However, I was just refreshing myself on what capture does and I see that there's an option to both capture and stream the output (--capture=tee-sys), which might address your issue without regressing anything. Would you mind trying out that flag and letting me know if the output shows up in your report?

@amartani
Copy link
Contributor Author

amartani commented Aug 9, 2023

That does work, but I think I would still personally prefer the default pytest behavior by default, and tell people to use --test_output=streamed --test_arg=--capture=no when needed (possibly with a .bazelrc alias for that). I think pytest's default is useful, as it only prints stdout / logs of failing tests (similar to bazel's --test_output=errors but at a test case level), and in a more "organized" way (instead of mixed with pytest's own output).

That said, it's easy to override, both when using the rule from BUILD files and also on the command line, so feel free to keep the current default if it is useful for you.

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.

2 participants