rules_python icon indicating copy to clipboard operation
rules_python copied to clipboard

Add a pytest rules

Open AutomatedTester opened this issue 3 years ago • 5 comments

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] Does not include precompiled binaries, eg. .par files. See CONTRIBUTING.md for info
  • [x] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • [ ] Bugfix
  • [x] Feature (please, look at the "Scope of the project" section in the README.md file)
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] CI related changes
  • [ ] Documentation content changes
  • [ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • [ ] Yes
  • [x] No

Other information

AutomatedTester avatar Apr 23 '21 12:04 AutomatedTester

Firstly, I would absolutely love to see pytest support in the stock bazel rules.

However, one thing that worries me is capturing the invisible dependency between test files and conftest.py files. Pytest-the-framework will load those to find customizations and fixtures which tests use without a paper-trail. (See pytest's doc).

So if the user is specifying the test, someone (is it them or the framework?) should be declaring those dependencies (conftest.py all the way up). I would argue the framework should, as it's painfully easy to get wrong from the user's perspective.

The same goes for the test suit rule.

joshua-cannon-techlabs avatar Sep 01 '21 13:09 joshua-cannon-techlabs

When someone comes back to this, consider adding extra arguments to change how pytest is changing the import path. I did some investigation as to why pytest tests where failing when I was using protobuf rules here: https://github.com/aignas/bazel_pytest_proto

aignas avatar Dec 28 '21 07:12 aignas

@AutomatedTester are you planning/willing to pick this up again, or would you prefer someone else taking it over?

@everyone else in here: what would be a minimal setup of changes required here to get this merged?

i see the following todo-list:

  • address (all) the review-comments
  • clear up if we want to implicitly supply a pytest and if so how

i would argue we shouldn't aim for adressing all eventualities at first, otherwise this would never get in in any form.

just for clarification: i would be willing to take this over under the condition that there is a willingness for constructive cooperation and eventually compromise.

betaboon avatar May 21 '22 09:05 betaboon

@AutomatedTester are you planning/willing to pick this up again, or would you prefer someone else taking it over?

Happy for someone to take this over, I haven't had time to do it.

AutomatedTester avatar May 23 '22 11:05 AutomatedTester

Heads up that https://github.com/bazelbuild/rules_python/pull/723 may supersede this.

thundergolfer avatar Jun 22 '22 02:06 thundergolfer

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days. Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

github-actions[bot] avatar Dec 19 '22 22:12 github-actions[bot]

This PR was automatically closed because it went 30 days without a reply since it was labeled "Can Close?"

github-actions[bot] avatar Jan 18 '23 22:01 github-actions[bot]

Note, https://docs.aspect.build/rules/aspect_rules_py/docs/rules#py_pytest_main provides missing glue for pytest, thanks to @mattem and @f0rmiga

alexeagle avatar Jul 31 '23 17:07 alexeagle