Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for settings assigned as attribute on StateMachine class #1643

Closed
Zac-HD opened this issue Oct 18, 2018 · 4 comments
Closed

Check for settings assigned as attribute on StateMachine class #1643

Zac-HD opened this issue Oct 18, 2018 · 4 comments
Labels
legibility make errors helpful and Hypothesis grokable

Comments

@Zac-HD
Copy link
Member

Zac-HD commented Oct 18, 2018

#1638 allows @settings(...) to be used as a class decorator on state machine classes, which substantially improves the ergonomics. However, the old mechanism of assigning an attribute is still supported. #1642 points out a fairly subtle trap in this behavior though, by fixing it in the docs:

DatabaseComparison.settings = settings(...)  # Wrong!  This doesn't have any effect.
DatabaseComparison.TestCase.settings = settings(...)  # The right way to do it.

Fortunately we can use essentially the same solution as in #1572, defining a custom __setattr__ on a (new) metaclass of GenericStateMachine that will turn the incorrect usage from silent failure into an explicit error with a helpful message (which can also suggest the decorator form).

This should error when the .settings attribute on the class is assigned an instance of settings, but allow other types to be assigned to minimize false-positives. We should not restrict attributes on instances if we can avoid it.

@Zac-HD Zac-HD added the legibility make errors helpful and Hypothesis grokable label Oct 18, 2018
@Zac-HD
Copy link
Member Author

Zac-HD commented Oct 19, 2018

This issue would be a good introduction to Hypothesis for an intermediate programmer who understands metaclasses, or at least can follow the example of hypothesis.settings to customise the behaviour of the class object but not instances thereof.

@nmbrgts
Copy link
Contributor

nmbrgts commented Oct 26, 2018

Hey Zac, I would like to give this a shot! Before I start, I have a question: There is a 2.7/3.x compatibility issue with the way metaclasses are assigned to classes. There are solutions to this in six and future, but it looks like these are not existing dependencies within hypothesis. What would be the best way to proceed?

@Zac-HD
Copy link
Member Author

Zac-HD commented Oct 26, 2018

I would simply copy the implementation of settingsMeta, which provides the __setattr__ for the settings class object.

class settingsMeta(type):
def __init__(self, *args, **kwargs):
super(settingsMeta, self).__init__(*args, **kwargs)
@property
def default(self):
v = default_variable.value
if v is not None:
return v
if hasattr(settings, '_current_profile'):
settings.load_profile(settings._current_profile)
assert default_variable.value is not None
return default_variable.value
def _assign_default_internal(self, value):
default_variable.value = value
def __setattr__(self, name, value):
if name == 'default':
raise AttributeError(
'Cannot assign to the property settings.default - '
'consider using settings.load_profile instead.'
)
elif not (isinstance(value, settingsProperty) or name.startswith('_')):
raise AttributeError(
'Cannot assign hypothesis.settings.%s=%r - the settings '
'class is immutable. You can change the global default '
'settings with settings.load_profile, or use @settings(...) '
'to decorate your test instead.' % (name, value)
)
return type.__setattr__(self, name, value)
class settings(
settingsMeta('settings', (object,), {}) # type: ignore
):

As far as I'm aware this is compatible with both Python2 and Python3, though I could be wrong!

@nmbrgts
Copy link
Contributor

nmbrgts commented Oct 26, 2018

Thanks for clarifying, Zac. I misunderstood how settingsMeta worked to create a metaclass. This does avoid the compatability error that I was thinking of, which was 3.x's metaclass keword argument vs 2.7's __metaclass__ property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legibility make errors helpful and Hypothesis grokable
Projects
None yet
Development

No branches or pull requests

2 participants