ok, I'm designing a plugin that should run before pytest runs (

./pants test

goal) to ensure that services are running. If they are not, then it should bail with helpful instructions on setting up the developer environment. new target:

service

depend on it from python_tests target:

service(name="service-mongo", ... other metadata describing the service here ...)
python_tests(
    name="tests",
    dependencies=["//:service-mongo"],
)

Then, we add an

@rule

with

targets: Targets

in the input that filters down to the

service

targets. That rule can use

Process

or similar to check for that the service is running. then

rules()

would return

[*collect_rules(), UnionRule(TestFieldSet, AssertServiceRequest)]

I think? But is there a way to insert my rule so that it has to run first, before pytest? Am I in the ballpark?

Hello! This is where Pants will need to expose a plugin hook for you, some hook that lets you do arbitrary setup before test runs We don't yet have it but have discussed the need for it and can bump in priority to unblock you

ok. I'm trying to clean up the developer experience as I go. When running tests, I realized I didn't have these services setup, which I believe is a common issue (esp for new contributors), so I think dealing with that UX is the next thing to do before I work on other things. As far as what the interface looks like, this is my first dive into pants plugins, so I'm still digesting.

As far as what the interface looks like, this is my first dive into pants plugins, so I'm still digesting.

Totally fair, I think what's most helpful is sharing what you need the plugin to do, and then we can reverse engineer the interface and implement that plugin hook to unblock you (Also ack you have some comments in the other thread. Most of the Pants team is a bit scattered this week because of Pycon right now, pardon!)

There are a lot of tests that do not have these external deps. Defining it as a dep of

python_tests()

is actually a little odd because only certain files need those services, not all of the test files in a given directory. That might just mean that they should eventually be partitioned into separate directories based on whether they need a service or not.

It doesn't need to be the

dependencies

field to express this, you could add a field to your

python_tests

target like

uses_mongo=True

. Then,

is_applicable()

looks for that field to be True You can add new fields to existing targets: https://www.pantsbuild.org/v2.5/docs/target-api-extending-targets

I don't think I need to adjust the argv or env vars of the process... though I suppose that might be a way to allow the plugin to not just assert that the service is running, but lookup what port the service is using and inject an env var to say how to communicate with it.

Either approach should be quite similar, more about what is ergonomic and understandable for your end users. E.g. do you want a dedicated

./pants help

message for each service, or it's fine to bundle in a single help message.

def is_applicable(cls, tgt):
   return tgt.get(UsesMongoField).value is True

vs

def is_applicable(cls, tgt):
   return "mongo" in tgt.get(UsesServicesField).value

If you go with the string approach, you can use an enum for validation they're using a valid string. https://www.pantsbuild.org/v2.5/docs/target-api-new-fields#stringfield

So in this possible interface:

def my_plugin(test_field_set: PytestFieldSet) -> PluginTestSetup:

PluginTestSetup

is the dataclass that I would populate to adjust the env vars or whatever? And then the pytest runner stuff would take that into account?

Yes, exactly. And thinking more about it, the interface would be:

@rule
def my_plugin(request: PytestPluginSetupRequest) -> PytestPluginSetup:

Where both are dataclasses. What is TBD is what should go into those dataclass fields. To start,

PytestPluginSetupRequest

should probably give you access to the

Target

being run against. It should also make you implement

is_applicable()

. We could start with

PytestPluginSetup

having no fields, and then add new features like "plugins can set env vars" as plugin authors request it

You know, injecting information into a test (like which port to use to talk to a service) would probably be better to lookup in a pytest fixture. So, leaving

PytestPluginSetup

as empty to begin with would probably be the way to go.

Yeah, speaking of which - that is part of why we have not yet added this plugin hook. So far, folks have been able to use conftest.py We generally try to limit adding features without people explicitly asking for them as a way to keep complexity lower and quality higher

So, to prevent pytest from running, and print the helpful instructions, would I throw an error in

my_plugin

?

Yes, exactly

Hmm. I wonder if the results of the plugin would be cached such that any pytest that

uses_mongo

would all fail if the first one fails, but only print the message once. Or does raising in a plugin kill pants at that point?

I wonder if the results of the plugin would be cached such that any pytest that uses_mongo would all fail if the first one fails, but only print the message once.

You can control the caching behavior of running a Process. It defaults to not caching failures, and you can change to always cache no matter what, or never cache. Here, you probably never want to cache because it's very side-effecty seeing if a server is running

Or does raising in a plugin kill pants at that point?

Yeah, raising an exception kills Pants, the daemon too iirc

And what happens if multiple targets are run in parallel and they all raise at about the same time? lots of noise / repeated error message?

Should only be a single error, one exception cancels the work of everything else

Sounds like your PytestPluginSetup hook is the way to go for my usecase 🙂

Indeed, it does sound that way! Will try to get something out soon for 2.6.0.dev0 And I agree, for now, I think Keep It Simple and have no fields on

PytestPluginSetup

dataclass - the hook is only intended for side effects

If you go with the string approach, you can use an enum for validation they're using a valid string. https://www.pantsbuild.org/v2.5/docs/target-api-new-fields#stringfield

I would actually be using a

StringSequenceField

, not just a

StringField

, which means

valid_choices

/ enum is not available, right? https://github.com/pantsbuild/pants/blob/2.5.x/src/python/pants/engine/target.py#L1169-L1177 https://github.com/pantsbuild/pants/blob/2.5.x/src/python/pants/engine/target.py#L1108

Oh yeah you're right. In that case, you could add your own custom validation though. And document in

help

message what is valid

There we go, I have a plugin that adds

uses=[]

field to

python_tests()

. It doesn't do much, but its a start 🙂

That looks great! To fill in

mongo_running

, I recommend first answering how would you do this w/o Pants, e.g. what bash command would you run? Then it becomes fairly mechanical to port to

Process

yup. I might do it within python instead of shelling out. dunno yet. 🙂

as in, something like

subprocess.run()

? Reading file system? Reminder that rules need to avoid side effects outside of using Pants's intrinsics for those things so that caching works properly

No, like checking to see if something is listening on the port, health-check-like.

Got it. If done that way, Pants does not yet have an intrinsic to do that beyond shelling. It is side-effecty, so not safe to cache. But you can use

@_uncacheable_rule

instead of

@rule

so that the rule never gets memoized, which allows you to do this safely

does using

Process

somehow make it cacheable?

Yep! Good question, and your understanding is correct. What you'll want is a dedicated

@rule

specifically to find if it's running or not. Your skeleton will look something like:

@dataclass(frozen=True)
class MongoStatus:
   is_running: bool

@rule
async def mongo_status() -> MongoStatus:
  return MongoStatus(True)

@rule
async def prep_pytest(request: ..., mongo_status: MongoStatus) ->  ...:
   if not mongo_status.is_running:
       ...

The engine will ensure that the

@rule

mongo_status

only runs once.

Hmm. With

setup_py()

, avoiding the pain of merging makes sense. But here we're not going to modify anything, so there isn't really anything to merge. I actually figured I would do one hook for each service so that I can keep all the instructions and service health checking in separate files. If we only do one hook, then I'd have to do some kind of dispatch pattern which isn't as clear to implement with rules. If there is a restriction, then I think that would come into play if something can update the env vars or args of pytest, So maybe the restriction is, only one

PytestPluginSetup

can be returned that changes something?

Okay, sounds good! I'm cool with allowing multiple implementations And in fact, I'm realizing that we could refactor this nifty mechanism of Pants to use this new plugin hook if we allow the result type to have a

digest: Digest

field to add files to the process's chroot. https://www.pantsbuild.org/v2.5/docs/python-test-goal#testing-your-packaging-pipeline

makes sense 🙂