[PATCH] comctl32/tests: Add more cases for ImageList_Remove().

[Serge Gautherie]

Signed-off-by: Serge Gautherie <winehq-git_serge_180711(a)gautherie.fr>
---
 dlls/comctl32/tests/imagelist.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/dlls/comctl32/tests/imagelist.c b/dlls/comctl32/tests/imagelist.c
index ed9b1cc..3f34197 100644
--- a/dlls/comctl32/tests/imagelist.c
+++ b/dlls/comctl32/tests/imagelist.c
@@ -404,10 +404,22 @@ static void test_add_remove(void)
     HICON hicon2 ;
     HICON hicon3 ;
 
+    /* removing from NULL handle should fail */
+    ok(!pImageList_Remove(NULL, 1), "Removed nonexistent item 1 from NULL handle\n");
+    ok(!pImageList_Remove(NULL, 0), "Removed nonexistent item 0 from NULL handle\n");
+    ok(!pImageList_Remove(NULL, -1), "Removed all items from NULL handle\n");
+    ok(!pImageList_Remove(NULL, -2), "Removed nonexistent item -2 from NULL handle\n");
+
     /* create an imagelist to play with */
     himl = pImageList_Create(84, 84, ILC_COLOR16, 0, 3);
     ok(himl!=0,"failed to create imagelist\n");
 
+    /* remove from empty list */
+    ok(!pImageList_Remove(himl, 1), "Removed nonexistent item 1 from empty list\n");
+    ok(!pImageList_Remove(himl, 0), "Removed nonexistent item 0 from empty list\n");
+    ok(pImageList_Remove(himl, -1), "Failed to remove all items from empty list\n");
+    ok(!pImageList_Remove(himl, -2), "Removed nonexistent item -2 from empty list\n");
+
     /* load the icons to add to the image list */
     hicon1 = CreateIcon(hinst, 32, 32, 1, 1, icon_bits, icon_bits);
     ok(hicon1 != 0, "no hicon1\n");
@@ -416,30 +428,33 @@ static void test_add_remove(void)
     hicon3 = CreateIcon(hinst, 32, 32, 1, 1, icon_bits, icon_bits);
     ok(hicon3 != 0, "no hicon3\n");
 
-    /* remove when nothing exists */
-    ok(!pImageList_Remove(himl, 0), "Removed nonexistent icon.\n");
-    /* removing everything from an empty imagelist should succeed */
-    ok(pImageList_Remove(himl, -1), "Removed nonexistent icon\n");
-
     /* add three */
     ok(0 == pImageList_ReplaceIcon(himl, -1, hicon1), "Failed to add icon1.\n");
     ok(1 == pImageList_ReplaceIcon(himl, -1, hicon2), "Failed to add icon2.\n");
     ok(2 == pImageList_ReplaceIcon(himl, -1, hicon3), "Failed to add icon3.\n");
 
     /* remove an index out of range */
-    ok(!pImageList_Remove(himl, 4711), "removed nonexistent icon\n");
+    ok(!pImageList_Remove(himl, 3), "Removed nonexistent item 3\n");
+    ok(!pImageList_Remove(himl, -2), "Removed nonexistent item -2\n");
 
     /* remove three */
-    ok(pImageList_Remove(himl, 0), "Can't remove 0\n");
-    ok(pImageList_Remove(himl, 0), "Can't remove 0\n");
-    ok(pImageList_Remove(himl, 0), "Can't remove 0\n");
+    ok(pImageList_Remove(himl, 2), "Can't remove item 2\n");
+    ok(pImageList_Remove(himl, 0), "Can't remove item 0 (1st)\n");
+    ok(pImageList_Remove(himl, 0), "Can't remove item 0 (2nd)\n");
 
     /* remove one extra */
     ok(!pImageList_Remove(himl, 0), "Removed nonexistent icon.\n");
+    ok(pImageList_Remove(himl, -1), "Failed to remove all items from emptied list\n");
 
     /* destroy it */
     ok(pImageList_Destroy(himl), "Failed to destroy imagelist.\n");
 
+    /* removing from an invalid handle should fail */
+    ok(!pImageList_Remove(himl, 1), "Removed nonexistent item 1 from bad handle\n");
+    ok(!pImageList_Remove(himl, 0), "Removed nonexistent item 0 from bad handle\n");
+    ok(!pImageList_Remove(himl, -1), "Removed all items from bad handle\n");
+    ok(!pImageList_Remove(himl, -2), "Removed nonexistent item -2 from bad handle\n");
+
     ok(-1 == pImageList_ReplaceIcon((HIMAGELIST)0xdeadbeef, -1, hicon1), "Don't crash on bad handle\n");
 
     ok(DestroyIcon(hicon1), "Failed to destroy icon 1.\n");
-- 
2.10.0.windows.1

[Serge Gautherie]

Hello Alexandre,

I am looking for a reviewer for my patches...

[PATCH] wininet: Add spaces to 3 ERR() and fix 1 'escape' copypasta.
https://source.winehq.org/patches/data/199448

[PATCH] comctl32/tests: Add more cases for ImageList_Remove().
https://source.winehq.org/patches/data/199500

Thanks.

--

[Nikolay Sivov]

On Fri, Feb 12, 2021 at 9:03 PM Serge Gautherie <
winehq-git_serge_180711(a)gautherie.fr> wrote:
> Hello Alexandre,
>
> I am looking for a reviewer for my patches...
>
> [PATCH] wininet: Add spaces to 3 ERR() and fix 1 'escape' copypasta.
> https://source.winehq.org/patches/data/199448
>
> [PATCH] comctl32/tests: Add more cases for ImageList_Remove().
> https://source.winehq.org/patches/data/199500

For the second patch, I'm not sure why you had to remove some existing
tests. Also there is no need to make failure messages that explicit:

"Removed nonexistent item 1 from NULL handle\n"

Obviously nothing was removed, because there is no list to remove from.

+    ok(!pImageList_Remove(himl, 3), "Removed nonexistent item 3\n");
+    ok(!pImageList_Remove(himl, -2), "Removed nonexistent item -2\n");

Same here, differences in return value does not mean item that does
not exist was removed. Item -2 can't exist in a first place.
> Thanks.
>
> --