Fix the assert_called_once() problem, and avert PYTHONHASHSEED hardcoding

[PiDelport]

This fixes the assert_called_once() false positives (see PR #86, and this article), and makes the tests pass on Python 3.5 (which adds a new check for that problem).

This also improves some assertions to be independent of Python's hashing order, so that the tests now pass deterministically without PYTHONHASHSEED = 0.

[codecov-io]

Current coverage is 96.88%

Merging #89 into master will decrease coverage by -0.81% as of 9415d9d

Powered by Codecov. Updated on successful CI builds.

[econchick]

I think you may need to rebase. Would you also mind squashing all the commits into one?

[econchick]

Also I was under the impression that order wasn't guaranteed for dictionaries, and to get around that, one sets the hash seed.

[freddrake]

Iteration order for a dict is not guaranteed. Solutions include an explicit sort (for lexicographic ordering) or using a collections.OrderedDict (retaining the insertion order, which sometimes has value).

The patch appears to remove control of the hash seed (which I consider fragile at best, esp. if a wide range of Python versions is being targeted), which makes control over ordering more important.

I favor controlling the ordering within the code (either in the library or tests) rather than relying on manipulating the hash algorithm.

[PiDelport]

Cool, rebased and updated!

I could squash the commits, but that would make for a pretty big and mixed diff. I'm not sure how I would summarize all the commit messages without losing context. Should I really do that?

[PiDelport]

Regarding the hash seed, I should add that hard-coding it can be actively dangerous: it prevents the test suite from catching bugs due to accidentally-introduced hash order dependencies in the main code.

Retaining the normal hashing behaviour while testing will at least make such bugs fail the test suite in the same way as during normal execution, which is a good thing.

[freddrake]

@pjdelport: We're in agreement regarding the hash seed; I was attempting to respond to @econchick's comment about ordering of dictionary content, which is affected by hashing (which is informed by the hash seed).

[econchick]

I definitely see that argument, it makes sense.

Last request - could you squash your commits into one, maybe two commits?

[PiDelport]

@econchick: I can squash the commits, but I'm confused: why do we want to do that?

I understand squashing small noisy commits into logical changes, but I've already tried to do that as much as it makes sense to: I fear that squashing them further will make the history a lot more opaque and difficult for future maintainers. Squashing specific codebase-wide fixes like 8d13c66 lose them in the noise of unrelated changes, and make it much harder to figure out what parts of the squashed commit message goes with what parts of the squashed change.

Is there some other reason for preferring squashed commits that I'm missing, or unaware of?

[econchick]

I would say this falls into two commits - one to remove the hash seed, and one to fix the assert call once issue. I see those two as being atomic, and the other commits seem to be just noise IMO.

[PiDelport]

Alright, I squashed them into those two.

I kept a reference to the unsquashed changes in fix-bad-assert_called_once-unsquashed, in case anyone needs to refer to them in the future.

[econchick]

Thank you - very much appreciated!

[PiDelport]

Cool. :)