Fix bug in CFGUtils.quasi_topological_sort_nodes

[lockshaw]

The previous implementation would return an empty list of nodes when given a cycle of two nodes. This fixes that.

[ltfish]

LGTM. Can you also add a test case of your observed failure case?

[subwire]

Wow. That's a strange one! Thanks @lockshaw!

I can't check the "real" test case in here, but before I merge, I will add one to binaries-private

[subwire]

Test is covered by, at least https://github.com/angr/angr/pull/1668

[ltfish]

Let's add a unit test before merging this in.

[lockshaw]

@ltfish Sorry that took so long! Unit test added. Where does angr track test dependencies?

[twizmwazin]

@lockshaw Our dependencies are marked in setup.py. But, why are we adding adding this library for a single test? We already use nose for our unit testing, would it be possible for you implement the unit test using that rather than a whole new library?

[lockshaw]

@twizmwazin Are test dependencies marked in setup.py? I imagine nose would be a pretty hard dependency for tests, and I see it referenced nowhere in that file.

Hypothesis is orthogonal to the nose. It would be possible to implement a similar test without it, but would require a non-trivial amount of code that we would then be responsible for maintaining, and would be lower quality than what hypothesis already offers. I added it because using it (ideally for more than just this test) should make bugs like this one a lot more rare. I've had good experiences using it over the last few months, if that counts for anything.

[twizmwazin]

Fair enough. It appears that we do not explicitly mark test dependencies up to this point. setuptools seems to support test dependencies, which would be installed when running python setup.py develop but not python setup.py install. We should probably add this to each project's setup.py.

[lockshaw]

@twizmwazin Do you want to take care of that or should I? I'd like to get this fix merged

[twizmwazin]

I'll add the dependency to our install and rerun the tests.

[ltfish]

You'll want to rebase your branch on top of the latest master first. Some interfaces have changed.