Skip to content

Conversation

@asa
Copy link

@asa asa commented Mar 24, 2024

This adds tooling to setup the pytest requirements() automatically.

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.

Thanks for the contribution!

name = "pytest_requirements",
python_interpreter_target = interpreter,
quiet = False,
requirements_lock = "@rules_python_pytest//:e2e/smoke/requirements.txt",
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think we want production code depending on test code. Are these requirements something we can bring into python_pytest/?

Copy link
Author

Choose a reason for hiding this comment

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

Nope, we don't. This proves the concept. Can certainly reshape this to be its own target. Any suggestions?

load("@pytest_requirements//:requirements.bzl", "install_deps")
install_deps()

#################################################
Copy link
Owner

Choose a reason for hiding this comment

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

Can we add a bzlmod example?

Copy link
Author

Choose a reason for hiding this comment

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

I'll have to cook one up. Haven't done much with bazelmod yet. It's been high friction so far.

Choose a reason for hiding this comment

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

I think it's a bad idea for requirements to be installed by external packages. They aren't constraint-solved with the user's app code, they don't see the lockfile, and they don't have control over the version.
Just expect the user to provide a pytest label (can default to @pip//pytest for convenience)

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