https://pantsbuild.org/ logo
Join Slack
Powered by
it's not obvious to me personally that calling `se...
# general
h
it's not obvious to me personally that calling 
set_options_for_scope
is the right way
a
could you elaborate?
h
I mean, as opposed to anything else
a
oh yeah i was just wondering if you had one in mind specifically or if you were seeing issues with using that?
h
if we're using 
set_options_for_scope
all around the codebase already, then it probably should continue to work with rules
a
yes
h
but maybe it's hard to make it work with rules?
I'm still not sure what the 
args
param is doing, just that it makes the test I wrote stop failing
a
i'm not sure what 
args
param you're referring to here
h
It's a param of 
ConsoleRuleTestBase.execute_rule()
it sets cmd-line args
h
ah my bad, I didn't upload the latest version of my code that adds that
h
It bootstraps options from the 
args
and 
env
passed in to it
h
so I'm not sure if it makes sense to just write the tests passing in an 
arg
parameter to 
assert_console_output
or do whatever needs to be done to make 
set_options_for_scope
work for v2 rules
h
Maybe we need to allow simply passing in a dict of options values
Well, 
set_options_for_scope()
is also a bit of a hack in its own right...
But yeah, we need some solution for setting options on rule tests.
That said, this method is specific to console rules
h
so, there should be some method on 
TestBase
that sets options, that subclasses built to test specific things will invoke, right?
or is that general design not the right way to do things?
h
Yeah, that's what 
set_options_for_scope()
is, but it only works for v1 tasks, basically (not quite accurate, but close enough)
In v2 options are just another product, they aren't global state, IIUC
But I think it might be nicer to modify 
ConsoleRuleTestBase.execute_rule()
to take an options dict and and target specs, instead of having it do cmd-line parsing.
But that's for the future, right now that 
args=
solution seems like the right way to go.
Short-term, anyway
h
so maybe use 
args
now and make a ticket to improve the arg-specifciation system
it seems to me like 
execute_rule
probably isn't hte place to be creating an 
OptionsBootstrapper
, since that's useful in places more general than console rules, right?
h
I think so
w
ConsoleRuleTestBase has iirc exactly one user right now... so yea, patches welcome =x
there are multiple ways to test rules though: see https://github.com/pantsbuild/pants/pull/7679#pullrequestreview-235267227
i was going to incorporate that into the readme update, but it was already mostly there in this section: https://github.com/pantsbuild/pants/blob/master/src/python/pants/engine/README.md#registering-rules
4 Views
Open in Slack
PreviousNext
Powered by