Parametrize test_lib_pack.py

[Yay295]

No description provided.

[radarhere]

Calling one test from another seems odd, and a potential source of confusion?

[Yay295]

I could make test_unpack not a test function, and have all three test functions call that, but I don't see how that would be any clearer.

[radarhere]

I understand there's a lot of repetition in this file, but I don't think that this version is easier to understand.

If others think it is easier to understand, that's fine, but those are my thoughts on the matter.

[Yay295]

It's much easier to understand when a test fails.

[radarhere]

While I recognise I'm not as fond of parametrisation as others, I think over 200 and 500 lines of code to construct the parameters is a bit much.

If the concern is about clarity when a test fails, maybe just adding a message to the assertion would be a smaller change with a similar effect? Something like this, but with the details about which case failed as the message.

Pillow/Tests/test_file_webp_metadata.py

Line 100 in b55d74b

assert webp_icc_profile == expected_icc_profile, "Webp ICC didn't match"

[Yay295]

The main benefit I see for parametrization is that each assert can fail on its own, instead of the test stopping at the first failed assert.