https://pantsbuild.org/ logo
Join Slack
Powered by
`Exception: Type PythonTargetAdaptor is not a memb...
# development
h
Exception: Type PythonTargetAdaptor is not a member of the TestTarget @union ("A union for registration of a testable target type.")
a
that's relevant to another hack i've added for binary creation
(1) you should be able to add a 
UnionRule(PythonTargetAdaptor, TestTarget)
in that file and it should work
(2) the way we assign target adaptors in 
engine_initializer.py
is confusing and looks very legacy and i just deleted some of that logic in my hack branch
h
"In that file" ?
a
sorry, as in, adding that 
UnionRule(...)
expression in any file's 
def rules()
somewhere should fix the issue
h
OK, so I'm trying to run the v2 pytest runner on some of pants's own tests, which I assume is supposed to work...
a
which target type is it?
h
I mean, isn't that what we're doing in CI right now?
w
@happy-kitchen-89482: one of those targets is not a test
a
@hundreds-father-404 ^
h
Ah
a
oh ok thanks stu
h
Hmm
w
relates to https://github.com/pantsbuild/pants/issues/4535 (because target types)
h
that makes sense
But is also not a good user experience. A user will expect 
./pants test src/python::
to do the right thing.
👍 1
w
but also to another question that is implicit right now: 'do we want to preserve the behaviour that we noop when something "isn't testable"?' or do we want to fail
a
yes
h
I think we want to noop
Otherwise there is no reasonable way to "run all the tests under this dir"
w
i'm sure we have github threads about this question.
h
Yeah, btw sorry for rehashing conversations that have already been had (and I appreciate the issue links to those conversations). But I'm finally getting around to personally hacking on the engine myself, so I have some catching up to do...
w
but yea. regardless, connected to #4535. because deciding on the safest way to noop will relate to target types pretty deeply.
a
i will have a PR up that completely changes this exact experience sometime this weekend (not sure if it will be mergeable, but it's quite interesting)
h
Agreed that it should noop. Here, the major usability gain should trump correctness imo
w
the place where noops are more questionable is where nothing matches ... ie 
./pants binary ${thing}::
on a directory that isn't packagable.
👍 1
h
Any pointers on how to go about this? How does a rule decide which targets it can or can't operate on?
w
go about... changing this behaviour in general? or how to run tests locally?
a
UnionRule is how it decides!
w
if you want to change the behaviour in general, some sketching on https://github.com/pantsbuild/pants/issues/4535 (with an eye on https://github.com/pantsbuild/pants/issues/8343) would be good. and yea, what we want to do to "seal" the graph to indicate that it is safe to skip needing an implementation of "testing for target type X" is related to UnionRule, and how and where to install a "default" implementation
and if not a "default" implementation, then a runtime check for "is this type in the union" to let the 
@rule
decide to skip doing something based on union-memberness
h
Right, I was wondering if this is a per-rule thing.
w
@happy-kitchen-89482: in general, rules run based on types
h
Also, it may be the case that the user still expects 
./pants test path/to/a/non-test/target
to fail
w
yes.
h
I.e., the correct capturing of user intent is that the glob itself should mean "all the things that are members of the corresponding union"
w
https://github.com/pantsbuild/pants/blob/44d4ab5e272094314bff498ef14013a53df3a814/src/python/pants/rules/core/test.py#L65-L78 is where we "cast" something to see if it is a member of the 
TestTarget
union. but it isn't fallible, so the run fails if it isn't.
h
In other words, it may be that the current rule behavior is correct, but the interpretation of the glob is not. But that opens up a whole can of worms, especially wrt multiple goals on the cmd line.
w
so two fundamentally different potential approaches 1) add support for a "default" implementation with UnionRules, so that you noop there, 2) add support for checking whether something is in a union at runtime
@happy-kitchen-89482: multiple goals on the cmd line just run one after the other. so a console rule would decide which to operate on.
so two fundamentally different potential approaches 1) add support for a "default" implementation with UnionRules, so that you noop there, 2) add support for checking whether something is in a union at runtime
^ is key
h
right, but we construct the glob once for all goals.
w
@happy-kitchen-89482: but each of them chooses what to do with it.
./pants roots
doesn't consume it.
./pants list
does, but not transitively
h
I'm just wondering if the behavior needs to be different depending on whether the source of the target set was a glob or not.
I.e., 
./pants test <glob>
should run on however many test targets there were, including zero.
But 
./pants test /not/a/test/target
should fail
w
depends on the goal/`@console_rule`, probably.
h
But can it know?
I guess it can
(can it know the origin of the targets, glob or not, I mean)
w
yea. could add that as something computed from the 
Specs
but again, maybe related to #4535. for example: perhaps you want `@console_rule`s to be able to request 
PythonTargets
as their roots.
...mm. bad example, because 
@console_rule
is generic.
but perhaps, members of the 
TestTarget
union as roots
so the signature of https://github.com/pantsbuild/pants/blob/44d4ab5e272094314bff498ef14013a53df3a814/src/python/pants/rules/core/test.py#L27-L28 would become
Copy code
@console_rule
def fast_test(console: Console, test_targets: TestTargets) -> Test:
... and 
TestTargets
would be pre-filtered from the roots.
(as an example.)
BUT, i think that either way, that still relates to my fundamental point above
how should we go about filtering to members of a union
Open in Slack
PreviousNext
Powered by