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

Safer generated test method names. #43

Merged
merged 4 commits into from
Nov 5, 2015
Merged
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
11 changes: 11 additions & 0 deletions genty/genty.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from .genty_args import GentyArgs
from .private import encode_non_ascii_string

REPLACE_FOR_PERIOD_CHAR = '\xb7'


def genty(target_cls):
"""
Expand Down Expand Up @@ -306,6 +308,15 @@ def _build_final_method_name(
if not dataset_name and not repeat_suffix:
return '{0}{1}'.format(method_name, suffix)

if dataset_name:
# Nosetest multi-processing code parses the full test name
# to discern package/module names. Thus any periods in the test-name
# causes that code to fail. So replace any periods with the unicode
# middle-dot character. Yes, this change is applied independent
# of the test runner being used... and that's fine since there is
# no real contract as to how the fabricated tests are named.
dataset_name = dataset_name.replace('.', REPLACE_FOR_PERIOD_CHAR)

# Place data_set info inside parens, as if it were a function call
suffix = '{0}({1})'.format(suffix, dataset_name or "")

Expand Down
11 changes: 11 additions & 0 deletions test/test_example.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@

@genty
class ExampleTests(TestCase):

# This is set so that if nosetest's multi-processing capability is being used, the
# tests in this class can be split across processes.
_multiprocess_can_split_ = True
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It tells nose's multi-processing logic that this single test class can be "split" across multiple processes.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 - let's not commit this unless we plan to use multiprocess in our tox.ini

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have it so that when manually testing... I can tickle the scenario. Without the fix the tests will fail. And with the fix the tests pass. So I think this should be committed. I can add a comment.


def setUp(self):
super(ExampleTests, self).setUp()

Expand Down Expand Up @@ -83,3 +88,9 @@ def test_dataprovider_with_no_dataset(self, data1, data2, data3):
"""
Uses a dataprovider that has no datasets.
"""

@genty_dataset('127.0.0.1')
def test_with_period_char_in_dataset(self, arg):
"""
A dataset with a '.' doesn't screw up nosetests --processes=4
"""
36 changes: 33 additions & 3 deletions test/test_genty.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from mock import patch
import six
from genty import genty, genty_args, genty_dataset, genty_repeat, genty_dataprovider
from genty.genty import REPLACE_FOR_PERIOD_CHAR
from genty.private import encode_non_ascii_string
from test.base_test_case import TestCase

Expand Down Expand Up @@ -154,7 +155,7 @@ def test_decorated(self, value1, value2, value3):
)(),
)

def test_dataprovider_args_can_use_gentry_args(self):
def test_dataprovider_args_can_use_genty_args(self):
@genty
class SomeClass(object):
@genty_dataset(
Expand Down Expand Up @@ -437,18 +438,47 @@ def test_unicode(self, _):
def test_genty_properly_composes_method_with_special_chars_in_dataset_name(self):
@genty
class SomeClass(object):
@genty_dataset(*r'!"#$%&\'()*.+-/:;>=<?@[\]^_`{|}~,')
@genty_dataset(*r'!"#$%&\'()*+-/:;>=<?@[\]^_`{|}~,')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we test dots anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because they get changed to the middle-dot. And that case is tested below.

def test_unicode(self, _):
return 33

instance = SomeClass()

for char in r'!"#$%&\'()*.+-/:;>=<?@[\]^_`{|}~,':
for char in r'!"#$%&\'()*+-/:;>=<?@[\]^_`{|}~,':
self.assertEqual(
33,
getattr(instance, 'test_unicode({0})'.format(repr(char)))()
)

def test_genty_replaces_standard_period_with_middle_dot(self):
# The nosetest multi-processing code parses the full test name
# to discern package/module names. Thus any periods in the test-name
# causes that code to fail. This test verifies that periods are replaced
# with the unicode middle-dot character.
@genty
class SomeClass(object):
@genty_dataset('a.b.c')
def test_period_char(self, _):
return 33

instance = SomeClass()

for attr in dir(instance):
if attr.startswith(encode_non_ascii_string('test_period_char')):
self.assertNotIn(
encode_non_ascii_string('.'),
attr,
"didn't expect a period character",
)
self.assertIn(
encode_non_ascii_string(REPLACE_FOR_PERIOD_CHAR),
attr,
"expected the middle-dot replacement character",
)
break
else:
raise KeyError("failed to find the expected test")

def test_genty_properly_calls_patched_methods(self):
class PatchableClass(object):
@staticmethod
Expand Down