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

Fix setting _p_changed when removing from small Python BTrees/TreeSets. #13

Merged
merged 3 commits into from
May 19, 2015
Merged

Fix setting _p_changed when removing from small Python BTrees/TreeSets. #13

merged 3 commits into from
May 19, 2015

Conversation

jamadden
Copy link
Member

@jamadden jamadden commented May 6, 2015

Sadly, the fix for #11 was incomplete (sorry) and failed to handle removal, which bit us testing our application under PyPy. This PR handles removal and adds test cases for it.

Example test failures before the fix:

======================================================================
FAIL: testRemoveInSmallMapSetsChanged (BTrees.tests.test_OOBTree.OOBTreePyTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "//BTrees/BTrees/tests/common.py", line 1179, in testRemoveInSmallMapSetsChanged
    self.assertTrue(t._p_changed)
AssertionError: False is not true

======================================================================
FAIL: testRemoveInSmallSetSetsChanged (BTrees.tests.test_OOBTree.OOTreeSetPyTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "//BTrees/BTrees/tests/common.py", line 1421, in testRemoveInSmallSetSetsChanged
    self.assertTrue(t._p_changed)
AssertionError: False is not true

----------------------------------------------------------------------

Twice, out of many test runs, I saw the following test failure for Python2.7. I'm pretty sure it's unrelated:

======================================================================
ERROR: test_LP294788 (BTrees.tests.testBTrees.BugFixes)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "//BTrees/BTrees/tests/testBTrees.py", line 355, in test_LP294788
    id = trandom.choice(list(ids.keys()))
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/random.py", line 275, in choice
    return seq[int(self.random() * len(seq))]  # raises IndexError if seq is empty
IndexError: list index out of range

----------------------------------------------------------------------

@jamadden
Copy link
Member Author

It turns out that plain xSet objects have the same issue as the Tree versions did in #11. I added a fix and tests to this PR.

- TBD
- Fix _p_changed when removing items from small pure-Python
BTrees/TreeSets and when adding to Sets. See:
https://github.com/zopefoundation/BTrees/issues/13
Copy link
Member

Choose a reason for hiding this comment

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

Don't we also re-fix #11? Should be mentioned here (so that folks who experienced the bug in 4.1.2 know to expect that it is fixed).

Copy link
Member Author

Choose a reason for hiding this comment

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

Issue 11 was about adding to the Tree versions; that didn't change here. This issue handles adding to the non-Tree Sets, as well as removing from Trees. I'm not sure how to reword the change entry to make it more clear, but I'm open to any suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

OK, I do see that it mentions the sets.

@tseaver
Copy link
Member

tseaver commented May 19, 2015

I'm good to merge this once the changelog is updated.

tseaver added a commit that referenced this pull request May 19, 2015
Fix setting _p_changed when removing from small Python BTrees/TreeSets.
@tseaver tseaver merged commit 8e91d91 into zopefoundation:master May 19, 2015
@tseaver
Copy link
Member

tseaver commented May 19, 2015

Released via: https://pypi.python.org/pypi/BTrees/4.1.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants