launchpad-dev team mailing list archive

[Maris Fogels <maris.fogels@xxxxxxxxxxxxx>]

On 09/29/2010 02:25 PM, Aaron Bentley wrote:
> On 09/21/2010 06:09 PM, Robert Collins wrote:
>> yes, see the existing tests that do it ;) I suggest the memcache ones
>> as an example. There is an XXX there that needs fixing too.
> 
> You mean this?
> 
>     >>> ignore = getFeatureStore().add(FeatureFlag(
>     ...     scope=u'default', flag=u'memcache', value=u'disabled',
>     ...     priority=1))
>     >>> empty_request = LaunchpadTestRequest()
>     >>> per_thread.features = FeatureController(
>     ...     ScopesFromRequest(empty_request).lookup)
> 
> I think there's a lot of room for improvement here.  If feature flags
> are meant to be pervasive, I think we should be able to depend on their
> existence in tests, not need to create them explicitly-- otherwise,
> we'll get test failures because the FeatureController hasn't been
> initialized.  (Or worse, we'll accidentally leak FeatureControllers into
> other tests.)  If performance is an issue, we can make a
> LazyFeatureController that won't cost anything unless we use a FeatureFlag.
> 
> There's also a significant disconnect between how flags are queried and
> how they are set.  Since they are queried via FeatureController, I
> searched in vain for a method to set them there.
> 
> Also, features.getFeatureFlag is a function, not a method, so it should
> not have a camelCase name.
> 
> Aaron

I have a branch in development that adds a much easier to use test helper for
setting feature flags during development.  Unfortunately I am blocked on the
flags not being set properly during one peculiar pagetest (if anyone is able to
help me with this, please feel free to ping me on IRC).  Once I figure out how
to get the flags to work in my branch, then I will land it and update the list
with instructions for using the new helpers.


-- 
Māris Fogels -- https://launchpad.net/~mars
Launchpad.net -- cross-project collaboration and hosting

Attachment: signature.asc
Description: OpenPGP digital signature