Test suite for population-count function, using random input

Question

For the purpose of this discussion, I have the following function:

def countSetBits(num):
  cnt = 0
  while num:
    cnt += num & 0b1
    num >>= 1
  return cnt

It counts the number of set bits in a given num's binary representation. For example, take 10 whose binary rep is 0b1010 -- countSetBits(10) should equal 2.

I have written a "test suite" that generates random input -- numbers of known set bit count and use that to test my countSetBits function.

import random
import unittest

class TestCountSetBits(unittest.TestCase):
    NUM_TESTS = 100
    MAX_SET_BIT_CNT = 10
    def test_suite(self):
        for _ in range(self.NUM_TESTS):
            k = expectedCnt = random.randint(0, self.MAX_SET_BIT_CNT)

            # Generate a random integer with `k` set bits
            setBitPositions = random.sample(range(k * 2), k)
            num = sum(0b1 << pos for pos in setBitPositions)
            computedCnt = countSetBits(num)
            self.assertEqual(computedCnt,
                             expectedCnt,
                             msg=f'Got {computedCnt}, but expected {expectedCnt}')
        print('Success!')

TestCountSetBits().test_suite()

My questions are:

0. Should I use cls instead of self?
1. Is there a more pythonic way to write this "test suite". I may be reading too much into this, but Python Doc's unittest page has a Basic Example in which each def test_* has only one assertEqual. My def test_suite has the assertEqual inside a for loop. Is this bad style?
2. Any suggestions are welcome! Thanks!

Scrolling down the unittest page, I see subTest which may tidy things up:

class TestCountSetBits(unittest.TestCase):
    NUM_TESTS = 100
    MAX_SET_BIT_CNT = 10
    def test_suite(self):
        for _ in range(self.NUM_TESTS):
            k = expectedCnt = random.randint(0, self.MAX_SET_BIT_CNT)

            # Generate a random integer with `k` set bits
            setBitPositions = random.sample(range(k * 2), k)
            num = sum(0b1 << pos for pos in setBitPositions)
            computedCnt = countSetBits(num)

            # Diff starts here
            with self.subTest():
                self.assertEqual(computedCnt,
                                 expectedCnt,
                                 msg=f'Got {computedCnt}, but expected {expectedCnt}')
            # Diff ends here

        print('Success!')

TestCountSetBits().test_suite()

Answer 1

PEP-8

def countSetBits(num):

Ummm, yeah, let me stop you right there. Pythonically we would avoid the java-esque camelCase. And maybe even let the Gentle Reader know we anticipate an int that we can do bit operations on. And describe what we return.

def count_set_bits(num: int) -> int:

In many languages we would conventionally name that pop_count. Ξένη Γήινος observes that this is re-inventing the wheel since the builtin bit_count gives the identical answer much quicker.

Similar snake_case remarks for {expected,computed}_cnt and for set_bit_positions. The unittest module is rather old, and comes from a time when we had fewer clear conventions, hence names like assertEqual.

For extra pythonic brownie points, write a single English sentence as a """docstring""".

---

    def test_suite(self):
        ...
        for _ in range(self.NUM_TESTS):
            ... self.MAX_SET_BIT_CNT ...

0. Should I use cls instead of self?

No.

There are times for such a practice, in your target code. But not in this test code.

BTW, it's fine to invest zero effort in devising a name for each test method. The obvious name for this method would be test_count_set_bits. The module being tested is presumably in count_set_bits.py, and the TestCountSetBits class name you chose is absolutely perfect. Typically there's not a ton of creativity that goes into making up identifiers in a test, and we count that as a good thing, we like simple boring tests.

1. ... has the assertEqual inside a for loop. Is this bad style?

No, not at all, it is just fine as-is.

A unit test should look like AAA, arrange act assert. It could have just a single instance of that, and then we move on to another AAA in the next test method. But here, we have to do a bit of Arrange work before we're ready for the rest, and it makes perfect sense to loop as you have done.

An even simpler test method might start with

        self.assertEqual(count_set_bits(10), 2)
        self.assertEqual(count_set_bits(11), 3)

and that would be perfectly fine, no need for two methods.

We anticipate that most tests are Green, most of the time. What's the downside to having multiple asserts in a test? Some of them won't run in the event of Red failure. So we learn less from that run. It's a tradeoff.

If there is little Arrange overhead, create distinct tests. If during arranging you had to go through a bunch of ceremony, feel free to amortize that over multiple asserts in a single test method.

This is language-agnostic advice. There's nothing python specific about it.

EDIT

Let me revisit the two lines I proposed. The first line came straight from your English language example where you expressed ten as 10 in decimal notation, and then I slightly tweaked the second line. I'd like to respond to your comment about "a lot of Arrange code" relative to small amount of target code.

I feel a better way for me to express those two lines would be:

        self.assertEqual(count_set_bits(0b1010), 2)
        self.assertEqual(count_set_bits(0b1011), 3)

Now the Gentle Reader can visually scan the input that we have Arranged and immediately see the answer. That is to say, this test is now "obvious".

I count half a dozen characters of Arrange, much less than the several lines which your very nice and far more powerful test wound up using. Both my boring test and your flexible test are good; both belong in the test suite, as they do different things. The small test has didactic value while your longer test tries to exercise a larger portion of the (infinite!) space of valid input values. Also, significantly, the short test instills confidence that the more flexible test is actually working properly. Imagine that buggy target code always returned 0, and buggy test code accidentally cleared all bits so it was asking for pop count of zero and wrongly showing us Green. The dirt simple test would catch such a travesty.

2. Any suggestions are welcome!

I see what you're trying to do with the 0b1 thing that you left-shift, and I'm somewhat sympathetic. But it's kind of weird. It's not like you're going for parallel structure with other nearby binary constants. A 1 is going to mean the same thing in hex, decimal, octal, binary. Recommend you just use 1 in the source.

Now, if we had e.g. x & 255 masking and x | 256 bit setting, I would definitely insist on expressing those as 0xff and 0x100, in order to prevent bugs via better readability. But 1? Meh.

I feel that verifying count_set_bits(0) == 0 is an important enough edge case that it's worth coding manually, rather than hoping the PRNG managed to hit it.

And then maybe walk a single bit up from 1 to some power of two, verifying the count is always one.

Scrolling down the unittest page, I see subTest

Hmmm, interesting. I don't recall that I've ever seen subTest used in production code that I've reviewed. Thank you for bringing it to my attention. Sure, go for it, use it in good health.

Imagine there was some population count bug. Maybe it fails just for multiples of twelve. Or maybe it always returns 0. In the first case, seeing that one-twelfth of tests failed would be very informative, and might reduce debugging time. In the second case, we'd be spammed with a ton of failures. Use what you feel would best support your development efforts. I'd be fine with either, since in both cases I will work toward turning the tests completely Green, and will definitely augment the automated test observations with single-step observations from within a debugger.

---

CLI

TestCountSetBits().test_suite()

I understand why you wrote this, it is a perfectly sensible way to execute the test runner. Especially when protected by a guard:

if __name__ == "__main__":
    TestCountSetBits().test_suite()

But I stopped appending such boilerplate to my tests long ago, preferring to habitually run

$ python -m unittest tests/*_test.py

Or I let $ make test do that for me. Or I ask pytest to run it, while still inheriting from TestCase. Typically that's because I want to make a $ pytest --cov code coverage measurement.

---

repeatable tests

I admire that you want to test "lots of inputs", more adventurous inputs than the boring ones you might have dreamt up and manually typed in. But please understand that Random Tests Are Bad. That is, if you get a failed Red test you won't necessarily be able to learn much from it. It's not a sure thing you can reproduce the bug.

So if you do go with random, definitely seed() the PRNG, maybe with 0, or with 42, so that subsequent runs will test the same inputs.

Or use a PRNG once to produce input.csv, and feed that same file to all subsequent test runs.

---

torture test

Consider using the amazing hypothesis testing library, if you're keen to find the edge cases.

It's not always a fit. It works best when you can offer an "oracle" for correctness, such as reversible {serialize, deserialize}, {encrypt, decrypt}, or {compress, decompress} pairs. It's very good at exploring the space of unusual inputs, and then having found a failing input it will boil it down to the simplest such failing input that it can find.

---

This codebase achieves its design goals.

I would be willing to delegate or accept maintenance tasks on it.