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

Extend step decorator #94

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions aloe/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
after,
around,
before,
extended_step,
register_placeholder,
step,
)

Expand Down
119 changes: 116 additions & 3 deletions aloe/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,19 +214,24 @@ def load(self, sentence, func):
self.steps[step_re.pattern] = (step_re, func)

try:
if not getattr(func, 'patterns', None):
func.patterns = set()

func.patterns.add(step_re.pattern)
func.sentence = sentence
func.unregister = partial(self.unload, step_re.pattern)
func.unregister = partial(self.unload, *func.patterns)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good! Is there a test for this? Without the extended steps, just registering a function twice and calling unregister should unregister both.

However, where are patterns used?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with the extended_step it can register the function to two different sentences:

@extended_step('I log in as user {STRING}')
def login(self, username):

Will register the login function with the following sentences (one for single quotes the other for double quotes):

I log in as user '([^']*)'
I log in as user "([^"]*)"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand what this is doing. But where is the patterns attribute used, other than right here, where it can be replaced by a local variable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I see it's accumulated on the function. Ignore.

except AttributeError:
# func might have been a bound method, no way to set attributes
# on that
pass

return func

def unload(self, sentence):
def unload(self, *sentences):
"""Remove a mapping for a given step sentence, if it exists."""
try:
del self.steps[sentence]
for sentence in sentences:
del self.steps[sentence]
except KeyError:
pass

Expand Down Expand Up @@ -449,3 +454,111 @@ def decorator(self, function, **kwargs):
around = CallbackDecorator(CALLBACK_REGISTRY, 'around')
before = CallbackDecorator(CALLBACK_REGISTRY, 'before')
# pylint:enable=invalid-name


class ExtendedStep(object):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still looking through the implementation, but I'd really prefer to do what Behave does and register a new sentence parser instead of throwing everything into regular expressions.

"""
An extended step decorator that defines placeholders and allows to easily
assign multiple sentences to a single function.
Useful for sentences that capture a string which is enclosed either in
single or double quotes
"""
CLOSURE = '{%s}'

def __init__(self):
self.single_expression_placeholders = {
'NUMBER': r'(-?\d+(?:\.\d*)?)',
'NON_CAPTURING_STRING': r'|'.join((r'"[^"]*"', r"'[^']*'")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these two for?

'NON_CAPTURING_NUMBER': r'-?\d+(?:\.\d*)?',
}
self.multi_expression_placeholders = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are wrapping everything with regexes, there's no need for this multi thing - you can define a single regex to do this. (Or if you can't, just wrap single into multi instead.)

'STRING': (r'"([^"]*)"', r"'([^']*)'"),
}

def register_placeholder(self, placeholder, *args):
"""Register a placeholder to be used on the extended_step."""

if len(args) > 1:
self.multi_expression_placeholders[placeholder] = args
else:
self.single_expression_placeholders[placeholder] = args[0]

def _replace_placeholders(self, sentence):
"""
Replace placeholders with their associated expressions.
Returns a list with at least one sentences to the product of all
the expressions when one or more multiple-expressions are used.
`replace` is use instead of `format` as the latter interferes with
defining complex custom regexes that contains {}.
"""

for placeholder, expression in \
self.single_expression_placeholders.iteritems():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

items(), this is Python 3.

sentence = sentence.replace(self.CLOSURE % placeholder, expression)

sentences = [sentence]

for placeholder in self.multi_expression_placeholders.keys():
sentences = self._replace_multi_expression_placeholder(
sentences, placeholder
)

return sentences

# pylint:disable=invalid-name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this is not needed after single/multi stuff is merged.

def _replace_multi_expression_placeholder(self, sentences, placeholder):
"""
Replace a placeholder that is associated with multiple expressions.
"""

placeholder_str = self.CLOSURE % placeholder

if placeholder_str not in sentences[0]:
return sentences

expressions = self.multi_expression_placeholders[placeholder]
new_sentences = []

for expression in expressions:
for sentence in sentences:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a comprehension.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, use a comprehension every time you do:

foo = []

for ... in ...:
    foo.append(...)

new_sentences.append(
sentence.replace(placeholder_str, expression)
)

return new_sentences
# pylint:enable=invalid-name

def extended_step(self, sentence):
"""
Creates one or multiple step definitions and associate them to the same
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functions docstrings should be imperative ("Fire the missiles", not "This fires the missiles").

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this is a decorator, which the docs fail to mention.

function.
The sentence can contain placeholders that are replaced by common
expressions/regexes.
A placeholder can be associated with multiple regexes creating in that
way multiple step definitions.
Placeholders are case sensitive and are enclosed in {}.
Common placeholders:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Common" can be interpreted as "most often used - there are more, this is a sample".

{STRING}
{NUMBER}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find the documentation of what does this do. (I suspect I know, but would be nice to see anyway.)

{NON_CAPTURING_STRING}
{NON_CAPTURING_NUMBER}
"""

def decorator(func):
"""Register a function as a step using the parsed sentence."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sentences, perhaps?


sentences = self._replace_placeholders(sentence)

for parsed_sentence in sentences:
step(parsed_sentence)(func)

return decorator


EXTENDED_STEP = ExtendedStep()

# These are functions, not constants
# pylint:disable=invalid-name
register_placeholder = EXTENDED_STEP.register_placeholder
extended_step = EXTENDED_STEP.extended_step
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes API stink. (What if you add another way of parsing steps, extended_extended_step?)

A better way would be to do @step("I have a NUMBER", parser=step.NICE (or step.REGEX)), or again look at Behave.

# pylint:enable=invalid-name
102 changes: 102 additions & 0 deletions tests/unit/test_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
from aloe.registry import (
CallbackDecorator,
CallbackDict,
ExtendedStep,
PriorityClass,
StepDict,
STEP_REGISTRY,
)
from aloe.exceptions import (
StepLoadingError,
Expand Down Expand Up @@ -267,6 +269,7 @@ def step(): # pylint:disable=missing-docstring
steps.step(step.sentence)(step)

assert_matches(steps, "My step 1", (step, ('1',), {}))
# pylint:enable=no-member


class CallbackDictTest(unittest.TestCase):
Expand Down Expand Up @@ -561,3 +564,102 @@ def prepare_hooks():
self.assertEqual([item for (item,) in sequence], [
'wrapped',
])


class ExtendedStepTest(unittest.TestCase):
"""
Test extended step.
"""

def setUp(self):
self.extended_step = ExtendedStep()
self.single_expression_placeholders = \
self.extended_step.single_expression_placeholders
self.multi_expression_placeholders = \
self.extended_step.multi_expression_placeholders

self.extended_step.register_placeholder('TEST1', 't1')
self.extended_step.register_placeholder('TEST2', 't2_1', 't2_2')

def test_register_placeholder(self):
"""Test registering placeholders."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not test anything useful. How can I actually use those TEST placeholders?


self.single_expression_placeholders.update({'TEST': 't1'})
self.multi_expression_placeholders.update({'TEST2': ('t2_1', 't2_2')})

self.assertDictEqual(
self.extended_step.single_expression_placeholders,
self.single_expression_placeholders
)

self.assertDictEqual(
self.extended_step.multi_expression_placeholders,
self.multi_expression_placeholders
)

def test_replace_placeholders(self):
"""Test replacing placeholders in string."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not test private methods. This does not show me how to use the custom placeholders, and does not guarantee that they will actually work (no one says _replace_placeholders is actually used).


sentences = [
'{TEST1}',
'{TEST2}',
'{TEST1}-{TEST2}',
'{TEST1}-TEST2',
'{test1}-{test2}',
'{NUMBER}',
'{STRING}',
'{NON_CAPTURING_STRING}',
'{NON_CAPTURING_NUMBER}',
'{TEST2}-{STRING}',
]

results = [
('t1', ),
('t2_1', 't2_2'),
('t1-t2_1', 't1-t2_2'),
('t1-TEST2', ),
('{test1}-{test2}', ),
(r'(-?\d+(?:\.\d*)?)', ),
(r'"([^"]*)"', r"'([^']*)'"),
(r'|'.join((r'"[^"]*"', r"'[^']*'")), ),
(r'-?\d+(?:\.\d*)?', ),
(r't2_1-"([^"]*)"',
r't2_2-"([^"]*)"',
r"t2_1-'([^']*)'",
r"t2_2-'([^']*)'",
)
]

for index, sentence in enumerate(sentences):
self.assertItemsEqual(
self.extended_step._replace_placeholders(sentence), # pylint:disable=protected-access
results[index]
)

def test_extended_step_func(self):
"""Test extended_step function."""

def step(): # pylint:disable=missing-docstring
pass

steps = STEP_REGISTRY
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not the same way as all the other tests?


# Load
self.extended_step.extended_step(r'My step {NUMBER}')(step)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how the custom placeholders should be tested.


assert_matches(steps, "My step 1", (step, ('1',), {}))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so it doesn't even convert the number to a number?


# Unload
step.unregister() # pylint:disable=no-member

# Load
self.extended_step.extended_step(r'My step {STRING}')(step)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just register two different steps and don't bother with unregistering.


assert_matches(steps, "My step 'one'", (step, ('one',), {}))
assert_matches(steps, 'My step "two"', (step, ('two',), {}))

# Unload
step.unregister() # pylint:disable=no-member

assert_no_match(steps, "My step 'one'")
assert_no_match(steps, 'My step "two"')