search

ionelmc / python-redis-lock

Lock context manager implemented via redis SET NX EX and BLPOP.
https://pypi.python.org/pypi/python-redis-lock
BSD 2-Clause "Simplified" License
551 stars 77 forks source link

Fix absence of signalization to waiters on reset #29

Closed AndreiPashkin closed 8 years ago

AndreiPashkin commented 8 years ago

Fixes #26

ionelmc commented 8 years ago

There's one code style issue and one "weird" failure:

=================================== FAILURES ===================================

_______________________________ test_no_overlap ________________________________

tests/test_redis_lock.py:201: in test_no_overlap

    wait_for_strings(proc.read, TIMEOUT, 'Getting %r ...' % name)

.tox/2.7-1.6-nocover/lib/python2.7/site-packages/process_tests.py:239: in wait_for_strings

    seconds, check_strings

E   AssertionError: Waited 10.00secs but ["Getting 'lock:foobar' ..."] did not appear in output in the given order !

----------------------------- Captured stdout call -----------------------------

*********** OUTPUT ***********

******************************

============================ pytest-warning summary ============================

WI1 /home/travis/build/ionelmc/python-redis-lock/.tox/2.7-1.6-nocover/lib/python2.7/site-packages/pytest_capturelog.py:171 'pytest_runtest_makereport' hook uses deprecated __multicall__ argument

WC1 /home/travis/build/ionelmc/python-redis-lock/tests/test_redis_lock.py cannot collect test class 'TestProcess' because it has a __init__ constructor

=========================== short test summary info ============================

FAIL tests/test_redis_lock.py::test_no_overlap

SKIP [13] /home/travis/build/ionelmc/python-redis-lock/.tox/2.7-1.6-nocover/lib/python2.7/site-packages/_pytest/doctest.py:165: all tests skipped by +SKIP option

===== 1 failed, 25 passed, 13 skipped, 2 pytest-warnings in 70.41 seconds ======

ERROR: InvocationError: '/home/travis/build/ionelmc/python-redis-lock/.tox/2.7-1.6-nocover/bin/py.test -vv --ignore=src'

___________________________________ summary ____________________________________

ERROR:   2.7-1.6-nocover: commands failed

I think it's just one of those failures caused by Travis being incredibly slow.

AndreiPashkin commented 8 years ago

Wait a sec.

AndreiPashkin commented 8 years ago

"Weird" error again, in different venv.

ionelmc commented 8 years ago

I think I may have written some bad code in that test ...

Do you see anything wrong with this https://github.com/ionelmc/python-redis-lock/blob/master/tests/helper.py#L47-L56 ?

AndreiPashkin commented 8 years ago

Refactored test_no_overlap, check it.

AndreiPashkin commented 8 years ago

Yeah! It works!

ionelmc commented 8 years ago

I've raised the timeouts in the old test_no_overlap and it seems to always pass now. I've rebased your code on top of that and it also pass.

https://github.com/ionelmc/python-redis-lock/commits/improve-test-no-overlap https://travis-ci.org/ionelmc/python-redis-lock/builds/93839233

That branch has the old test and your new test. I'm a bit weary of removing test code :grimacing:

Let me know if you're fine with having that in master.

AndreiPashkin commented 8 years ago

Included your code and refactored my version to not use notify_all and also be faster.

AndreiPashkin commented 8 years ago

BTW, what software did you used to make the diagram?

ionelmc commented 8 years ago

It's just google docs: https://docs.google.com/drawings/d/1XZWt0HKdzilDeVp8xbml-lLaXngP3JTVG7f-yE8W8bo

I'm gonna update that in a bit.

ionelmc commented 8 years ago

Alright, thank you for the changes!

AndreiPashkin commented 8 years ago

:shipit::+1: