-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add option to split by class/module + cleanup #4
base: master
Are you sure you want to change the base?
Changes from 3 commits
66f937b
0e808e8
b10b053
79be7d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
language: python | ||
env: | ||
- TOX_ENV=py26 | ||
- TOX_ENV=py27 | ||
- TOX_ENV=pypy | ||
- TOX_ENV=nose-1-0 | ||
- TOX_ENV=nose-1-3 | ||
# commands to install dependencies | ||
install: | ||
- pip install tox --use-mirrors | ||
# commands to run | ||
script: | ||
- tox -e $TOX_ENV |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
"""Distribute your nose tests across multiple machines, hassle-free""" | ||
VERSION = (0, 1, 2, '') | ||
VERSION = (0, 1, 3, '') | ||
__version__ = '.'.join(map(str, VERSION[0:3])) + ''.join(VERSION[3:]) | ||
__author__ = 'Wes Winham' | ||
__contact__ = '[email protected]' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
|
||
logger = logging.getLogger('nose.plugins.distributed_nose') | ||
|
||
|
||
class DistributedNose(Plugin): | ||
""" | ||
Distribute a test run, shared-nothing style, by specifying the total number | ||
|
@@ -103,9 +104,33 @@ def _options_are_valid(self): | |
|
||
return True | ||
|
||
def getCannotSplit(self, testObject): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So it looks like what we're actually returning here is returning either the class or the module if we can't split it, right? I think we could simplify this code and reduce the duplication inside Maybe this could be called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great suggestion and I even uncovered some bugs/weird behaviors while refactoring. This code will split at the class level even if the module specifies no splitting, but that's fixed in the next commit. |
||
obj_self = getattr(testObject, '__self__', None) | ||
klass = None | ||
|
||
if obj_self is not None: | ||
klass = getattr(testObject, '__class__', None) | ||
else: | ||
if hasattr(testObject, 'im_class'): | ||
klass = testObject.im_class | ||
|
||
if klass is not None: | ||
if not getattr(klass, '_distributed_can_split_', True): | ||
return klass | ||
|
||
filepath, module, call = test_address(testObject) | ||
return module if not getattr(module, '_distributed_can_split_', True) else None | ||
|
||
def validateName(self, testObject): | ||
filepath, module, call = test_address(testObject) | ||
|
||
# By default, we assume modules can be split, but if not, assign all tests in the module to one node | ||
cannot_split = self.getCannotSplit(testObject) | ||
if cannot_split: | ||
node = self.hash_ring.get_node('%s' % cannot_split) | ||
if node != self.node_id: | ||
return False | ||
|
||
node = self.hash_ring.get_node('%s.%s' % (module, call)) | ||
if node != self.node_id: | ||
return False | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,18 @@ | ||
[tox] | ||
envlist = py26,py27,py33,pypy,nose-0-11,nose-1-0,docs | ||
envlist = py26,py27,pypy,nose-1-0,nose-1-3 | ||
|
||
[testenv] | ||
deps = | ||
nose>=1.2.1,<1.3 | ||
commands = | ||
python setup.py nosetests | ||
|
||
[testenv:nose-0-11] | ||
basepython = python2.6 | ||
deps = | ||
nose<1.0 | ||
|
||
[testenv:nose-1-0] | ||
basepython = python2.6 | ||
deps = | ||
nose>=1.0,<1.1 | ||
|
||
[testenv:docs] | ||
changedir = docs | ||
[testenv:nose-1-3] | ||
basepython = python2.6 | ||
deps = | ||
sphinx | ||
commands = | ||
sphinx-build -W -b html -d {envtmpdir}/doctrees . {envtmpdir}/html | ||
nose>=1.3,<1.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're changing the behavior of class-based splitting here, right? in 1.2., two methods on the same TestCase will always end up on the same node. With this change, they won't.
Personally, my primary use case is to run a bunch of Django test cases. For those, I want to keep classes together because I use fixtures and running two tests with the same fixtures is faster because we do a transaction rollback. Updating all of my test cases to include
_distributed_can_split_ = False
would be pretty rough.How do you feel about leaving the current default behavior where tests are not split across classes?
Either way, I think the new version should be 0.2.0 since we're adding a new feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another possibility would be adding a command line argument to change the default split behavior. I'd generally rather avoid that, though, if we can come to a consensus.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with bumping the version to 0.2.0.
My primary case also involves wanting to keep classes together, but I don't think the current code necessarily does that. Lucky for me, all my test classes already derive from my own subclass of TestCase, so I just put
_distributed_can_split_
= False in the base class.I ran:
And I see tests from TC1 and from TC2 being split among the nodes.
Anyway, that's why I coded it up that way, to keep it from being a breaking change.