bslatkin / effectivepython

Effective Python: Second Edition — Source Code and Errata for the Book
https://effectivepython.com
2.2k stars 710 forks source link

In item 76, verify_complex_case may fail to detect some errors #96

Open nodet opened 3 years ago

nodet commented 3 years ago

In item 76, on page 361 of the 2nd edition, you define the function verify_complex_case. It's part that verifies whether the results and the expected value lists have the same length seems incorrect: if they differ by only one element, and it happens that the longer list is the one with the expected values, the test will not fail.

In the example below, the test should fail, but it doesn't. This is because zip will consume the last element of its first argument (here, expect_it) before it notices that the second iterator is already past its end.

from unittest import TestCase, main

def sum_squares(values):
    cumulative = 0
    for value in values:
        cumulative += value ** 2
        yield cumulative

class HelperTestCase(TestCase):
    def verify_complex_case(self, values, expected):
        expect_it = iter(expected)
        found_it = iter(sum_squares(values))
        test_it = zip(expect_it, found_it)

        for i, (expect, found) in enumerate(test_it):
            self.assertEqual(
                expect,
                found,
                f'Index {i} is wrong')

        # Verify both generators are exhausted
        try:
            next(expect_it)
        except StopIteration:
            pass
        else:
            self.fail('Expected longer than found')

        try:
            next(found_it)
        except StopIteration:
            pass
        else:
            self.fail('Found longer than expected')

    def test_wrong_length_incorrect(self):
        # This test should not pass, as len(expected) > len(values)
        values = [1.1]
        expected = [1.1 ** 2, 2.2 ** 2]
        self.verify_complex_case(values, expected)

if __name__ == '__main__':
    main()

With the following code, the tests fail as they should. And verify_complex_case seems to me easier to understand.

from itertools import zip_longest
from unittest import TestCase, main

def sum_squares(values):
    cumulative = 0
    for value in values:
        cumulative += value ** 2
        yield cumulative

class HelperTestCase(TestCase):
    def verify_complex_case(self, values, expected):
        expect_it = iter(expected)
        found_it = iter(sum_squares(values))
        sentinel = object()
        test_it = zip_longest(expect_it, found_it, fillvalue=sentinel)

        for i, (expect, found) in enumerate(test_it):
            self.assertNotEqual(expect, sentinel, 'Found longer than expected')
            self.assertNotEqual(found, sentinel, 'Expected longer than found')
            self.assertEqual(expect, found, f'Index {i} is wrong')

    def test_wrong_length_incorrect(self):
        values = [1.1]
        expected = [1.1 ** 2, 2.2 ** 2]
        self.verify_complex_case(values, expected)

    def test_wrong_length(self):
        values = [1.1, 2.2]
        expected = [1.1 ** 2]
        self.verify_complex_case(values, expected)

if __name__ == '__main__':
    main()
bslatkin commented 1 month ago

Thank you for the report! The way I'd solve this now is using zip(..., strict=True). See https://peps.python.org/pep-0618/