From 53a85f2a69d875aa670e4432b3db7e3a0629cd6e Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 16 Apr 2020 11:31:28 +0300 Subject: [PATCH 01/10] Refactor test_pickle_image to use pytest.mark.parametrize --- Tests/test_pickle.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/Tests/test_pickle.py b/Tests/test_pickle.py index ba4868988..b95d8baca 100644 --- a/Tests/test_pickle.py +++ b/Tests/test_pickle.py @@ -1,11 +1,14 @@ import pickle +import pytest from PIL import Image -def helper_pickle_file(tmp_path, pickle, protocol=0, mode=None): +def helper_pickle_file( + tmp_path, pickle, protocol=0, test_file="Tests/images/hopper.jpg", mode=None +): # Arrange - with Image.open("Tests/images/hopper.jpg") as im: + with Image.open(test_file) as im: filename = str(tmp_path / "temp.pkl") if mode: im = im.convert(mode) @@ -35,11 +38,14 @@ def helper_pickle_string( assert im == loaded_im -def test_pickle_image(tmp_path): +@pytest.mark.parametrize( + "test_file", ["Tests/images/hopper.jpg"], +) +def test_pickle_image(test_file, tmp_path): # Act / Assert for protocol in range(0, pickle.HIGHEST_PROTOCOL + 1): - helper_pickle_string(pickle, protocol) - helper_pickle_file(tmp_path, pickle, protocol) + helper_pickle_string(pickle, protocol, test_file) + helper_pickle_file(tmp_path, pickle, protocol, test_file) def test_pickle_p_mode(): From d62b9098dd80b7792f0d5478d4e67ac06154606d Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 16 Apr 2020 11:44:28 +0300 Subject: [PATCH 02/10] Refactor other tests into test_pickle_image --- Tests/test_pickle.py | 47 ++++++++++++++------------------------------ 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/Tests/test_pickle.py b/Tests/test_pickle.py index b95d8baca..fe4e75646 100644 --- a/Tests/test_pickle.py +++ b/Tests/test_pickle.py @@ -39,45 +39,28 @@ def helper_pickle_string( @pytest.mark.parametrize( - "test_file", ["Tests/images/hopper.jpg"], + ("test_file", "test_mode"), + [ + ("Tests/images/hopper.jpg", None), + ("Tests/images/hopper.jpg", "L"), + ("Tests/images/hopper.jpg", "PA"), + ("Tests/images/test-card.png", None), + ("Tests/images/zero_bb.png", None), + ("Tests/images/zero_bb_scale2.png", None), + ("Tests/images/non_zero_bb.png", None), + ("Tests/images/non_zero_bb_scale2.png", None), + ("Tests/images/p_trns_single.png", None), + ("Tests/images/pil123p.png", None), + ("Tests/images/itxt_chunks.png", None), + ], ) -def test_pickle_image(test_file, tmp_path): +def test_pickle_image(tmp_path, test_file, test_mode): # Act / Assert for protocol in range(0, pickle.HIGHEST_PROTOCOL + 1): helper_pickle_string(pickle, protocol, test_file) helper_pickle_file(tmp_path, pickle, protocol, test_file) -def test_pickle_p_mode(): - # Act / Assert - for test_file in [ - "Tests/images/test-card.png", - "Tests/images/zero_bb.png", - "Tests/images/zero_bb_scale2.png", - "Tests/images/non_zero_bb.png", - "Tests/images/non_zero_bb_scale2.png", - "Tests/images/p_trns_single.png", - "Tests/images/pil123p.png", - "Tests/images/itxt_chunks.png", - ]: - for protocol in range(0, pickle.HIGHEST_PROTOCOL + 1): - helper_pickle_string(pickle, protocol=protocol, test_file=test_file) - - -def test_pickle_pa_mode(tmp_path): - # Act / Assert - for protocol in range(0, pickle.HIGHEST_PROTOCOL + 1): - helper_pickle_string(pickle, protocol, mode="PA") - helper_pickle_file(tmp_path, pickle, protocol, mode="PA") - - -def test_pickle_l_mode(tmp_path): - # Act / Assert - for protocol in range(0, pickle.HIGHEST_PROTOCOL + 1): - helper_pickle_string(pickle, protocol, mode="L") - helper_pickle_file(tmp_path, pickle, protocol, mode="L") - - def test_pickle_la_mode_with_palette(tmp_path): # Arrange filename = str(tmp_path / "temp.pkl") From cdf3c981037578c727c70a42693d418cd5347ec4 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 16 Apr 2020 11:45:12 +0300 Subject: [PATCH 03/10] Add failing test for pickling webp --- Tests/test_pickle.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/test_pickle.py b/Tests/test_pickle.py index fe4e75646..c5d40b068 100644 --- a/Tests/test_pickle.py +++ b/Tests/test_pickle.py @@ -44,6 +44,7 @@ def helper_pickle_string( ("Tests/images/hopper.jpg", None), ("Tests/images/hopper.jpg", "L"), ("Tests/images/hopper.jpg", "PA"), + ("Tests/images/hopper.webp", None), ("Tests/images/test-card.png", None), ("Tests/images/zero_bb.png", None), ("Tests/images/zero_bb_scale2.png", None), From 2e6ab7c669603ba13bff3b28de56b2e02107ff93 Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 16 Apr 2020 11:46:34 +0300 Subject: [PATCH 04/10] Fix pickling webp --- src/PIL/WebPImagePlugin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/PIL/WebPImagePlugin.py b/src/PIL/WebPImagePlugin.py index eda685508..a24ff682c 100644 --- a/src/PIL/WebPImagePlugin.py +++ b/src/PIL/WebPImagePlugin.py @@ -38,6 +38,8 @@ class WebPImageFile(ImageFile.ImageFile): format = "WEBP" format_description = "WebP image" + __loaded = -1 + __logical_frame = -1 def _open(self): if not _webp.HAVE_WEBPANIM: From 5529aba441936167df6164a948b799f1c4c8d16a Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 16 Apr 2020 12:56:12 +0300 Subject: [PATCH 05/10] Skip webp test when webp not available --- Tests/test_pickle.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Tests/test_pickle.py b/Tests/test_pickle.py index c5d40b068..02212c7d8 100644 --- a/Tests/test_pickle.py +++ b/Tests/test_pickle.py @@ -3,6 +3,8 @@ import pickle import pytest from PIL import Image +from .helper import skip_unless_feature + def helper_pickle_file( tmp_path, pickle, protocol=0, test_file="Tests/images/hopper.jpg", mode=None @@ -44,7 +46,9 @@ def helper_pickle_string( ("Tests/images/hopper.jpg", None), ("Tests/images/hopper.jpg", "L"), ("Tests/images/hopper.jpg", "PA"), - ("Tests/images/hopper.webp", None), + pytest.param( + "Tests/images/hopper.webp", None, marks=skip_unless_feature("webp") + ), ("Tests/images/test-card.png", None), ("Tests/images/zero_bb.png", None), ("Tests/images/zero_bb_scale2.png", None), From 913e79f0102971f5d9f9bd1fcffbd4045933a24f Mon Sep 17 00:00:00 2001 From: Hugo Date: Thu, 16 Apr 2020 15:52:10 +0300 Subject: [PATCH 06/10] Test the modes --- Tests/test_pickle.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/test_pickle.py b/Tests/test_pickle.py index 02212c7d8..aa06e57ae 100644 --- a/Tests/test_pickle.py +++ b/Tests/test_pickle.py @@ -62,8 +62,8 @@ def helper_pickle_string( def test_pickle_image(tmp_path, test_file, test_mode): # Act / Assert for protocol in range(0, pickle.HIGHEST_PROTOCOL + 1): - helper_pickle_string(pickle, protocol, test_file) - helper_pickle_file(tmp_path, pickle, protocol, test_file) + helper_pickle_string(pickle, protocol, test_file, test_mode) + helper_pickle_file(tmp_path, pickle, protocol, test_file, test_mode) def test_pickle_la_mode_with_palette(tmp_path): From f589f8689fd5906b9c120b9f6fee1b61f3ee621b Mon Sep 17 00:00:00 2001 From: Hugo van Kemenade Date: Fri, 17 Apr 2020 11:20:38 +0300 Subject: [PATCH 07/10] Initialise __logical_frame = 0 so tell() == 0 when unpickled Co-Authored-By: Andrew Murray <3112309+radarhere@users.noreply.github.com> --- src/PIL/WebPImagePlugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PIL/WebPImagePlugin.py b/src/PIL/WebPImagePlugin.py index a24ff682c..000e02b43 100644 --- a/src/PIL/WebPImagePlugin.py +++ b/src/PIL/WebPImagePlugin.py @@ -39,7 +39,7 @@ class WebPImageFile(ImageFile.ImageFile): format = "WEBP" format_description = "WebP image" __loaded = -1 - __logical_frame = -1 + __logical_frame = 0 def _open(self): if not _webp.HAVE_WEBPANIM: From 94751da23e3876a395f2b44033c76842a5cb6158 Mon Sep 17 00:00:00 2001 From: Hugo Date: Fri, 17 Apr 2020 11:21:57 +0300 Subject: [PATCH 08/10] Initialise __physical_frame = 0 and add pickle roundtrip tell test --- Tests/test_pickle.py | 11 +++++++++++ src/PIL/WebPImagePlugin.py | 1 + 2 files changed, 12 insertions(+) diff --git a/Tests/test_pickle.py b/Tests/test_pickle.py index aa06e57ae..a6652562c 100644 --- a/Tests/test_pickle.py +++ b/Tests/test_pickle.py @@ -82,3 +82,14 @@ def test_pickle_la_mode_with_palette(tmp_path): im.mode = "PA" assert im == loaded_im + + +def test_pickle_tell(): + # Arrange + image = Image.open("Tests/images/hopper.webp") + + # Act: roundtrip + unpickled_image = pickle.loads(pickle.dumps(image)) + + # Assert + assert unpickled_image.tell() == 0 diff --git a/src/PIL/WebPImagePlugin.py b/src/PIL/WebPImagePlugin.py index 000e02b43..011e10716 100644 --- a/src/PIL/WebPImagePlugin.py +++ b/src/PIL/WebPImagePlugin.py @@ -40,6 +40,7 @@ class WebPImageFile(ImageFile.ImageFile): format_description = "WebP image" __loaded = -1 __logical_frame = 0 + __physical_frame = 0 def _open(self): if not _webp.HAVE_WEBPANIM: From 986c55ff646852573b95986b628e5ab133a2ed4a Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Fri, 17 Apr 2020 19:15:05 +1000 Subject: [PATCH 09/10] Initialise __loaded = 0, removed initialisation of __physical_frame --- src/PIL/WebPImagePlugin.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/PIL/WebPImagePlugin.py b/src/PIL/WebPImagePlugin.py index 011e10716..77a02e4b9 100644 --- a/src/PIL/WebPImagePlugin.py +++ b/src/PIL/WebPImagePlugin.py @@ -38,9 +38,8 @@ class WebPImageFile(ImageFile.ImageFile): format = "WEBP" format_description = "WebP image" - __loaded = -1 + __loaded = 0 __logical_frame = 0 - __physical_frame = 0 def _open(self): if not _webp.HAVE_WEBPANIM: From e52b8cefe16ce247696da943790dda005d8a85e6 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Fri, 17 Apr 2020 20:08:10 +1000 Subject: [PATCH 10/10] Skip test if webp is not available --- Tests/test_pickle.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/test_pickle.py b/Tests/test_pickle.py index a6652562c..4bf6d586a 100644 --- a/Tests/test_pickle.py +++ b/Tests/test_pickle.py @@ -84,6 +84,7 @@ def test_pickle_la_mode_with_palette(tmp_path): assert im == loaded_im +@skip_unless_feature("webp") def test_pickle_tell(): # Arrange image = Image.open("Tests/images/hopper.webp")