Re: [PATCH v2 resend 1/6] comctl32/tests: Expand ownerdata listview tests.

[Zhiyi Zhang]

On 5/26/22 04:00, Angelo Haller wrote:
> From: Angelo Haller
>
> Add more test cases to ownderdata listviews.
> Check LVN_ITEMCHANGED IDs.
> Signed-off-by: Angelo Haller
>
> dlls/comctl32/tests/listview.c | 59 +++++++++++++++++++++++++++++-----
>  1 file changed, 51 insertions(+), 8 deletions(-)
>
> diff --git a/dlls/comctl32/tests/listview.c b/dlls/comctl32/tests/listview.c
> index 6ac7f53137d..78b3e3ae069 100644
> --- a/dlls/comctl32/tests/listview.c
> +++ b/dlls/comctl32/tests/listview.c
> @@ -255,11 +255,33 @@ static const struct message ownerdata_deselect_all_parent_seq[] = {
>      { 0 }
>  };
>
> -static const struct message ownerdata_multiselect_odstatechanged_seq[] = {
> - { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED },
>
> +static const struct message ownerdata_multiselect_select_0_to_1_seq[] = {
> - { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED },
> { WM_NOTIFY, sent|id, 0, 0, LVN_ODSTATECHANGED },
> - { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED },
> - { WM_NOTIFY, sent|id, 0, 0, LVN_ITEMCHANGED },
> - { WM_NOTIFY, sent|id|wparam, 0, 0, LVN_ITEMCHANGED },
> - { WM_NOTIFY, sent|id|wparam, 1, 0, LVN_ITEMCHANGED },
> - { 0 }
>
> +};
> -
>
> +static const struct message ownerdata_multiselect_select_0_to_2_seq[] = {
> - { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED },
> - { WM_NOTIFY, sent|id, 0, 0, LVN_ODSTATECHANGED },
> - { WM_NOTIFY, sent|id|wparam, 1, 0, LVN_ITEMCHANGED },
> - { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED },
> - { 0 }
>
> +};
> -
>
> +static const struct message ownerdata_multiselect_deselect_all_select_3_seq[] = {
> - { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED },
> - { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED },
> - { WM_NOTIFY, sent|id|wparam, 3, 0, LVN_ITEMCHANGED },
> - { 0 }
>
> +};
> -
>
> +static const struct message ownerdata_multiselect_deselect_3_select_2_seq[] = {
> - { WM_NOTIFY, sent|id|wparam, -1, 0, LVN_ITEMCHANGED },
> - { WM_NOTIFY, sent|id|wparam, 3, 0, LVN_ITEMCHANGED },
> - { WM_NOTIFY, sent|id|wparam, 2, 0, LVN_ITEMCHANGED },
> { 0 }
>
> };
>
> @@ -3575,8 +3597,8 @@ static void test_ownerdata_multiselect(void)
>      expect(0, res);
>  ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX,
> -
>            ownerdata_multiselect_odstatechanged_seq,
> -
>            "ownerdata select multiple notification", TRUE);
> -
>            ownerdata_multiselect_select_0_to_1_seq,
> -
>            "ownerdata multiselect: select multiple via SHIFT", TRUE);
>
> res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0);
>    expect(0, res);
>
> @@ -3592,8 +3614,8 @@ static void test_ownerdata_multiselect(void)
>      expect(0, res);
>  ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX,
> -
>            ownerdata_multiselect_odstatechanged_seq,
> -
>            "ownerdata select multiple notification", TRUE);
> -
>            ownerdata_multiselect_select_0_to_2_seq,
> -
>            "ownerdata multiselect: select multiple via SHIFT+CONTROL", TRUE);
>
> res = SendMessageA(hwnd, WM_KEYUP, VK_DOWN, 0);
>    expect(0, res);
>
> @@ -3604,6 +3626,27 @@ static void test_ownerdata_multiselect(void)
>      res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0);
>      expect(3, res);
> - flush_sequences(sequences, NUM_MSG_SEQUENCES);
> -
> - res = SendMessageA(hwnd, WM_KEYDOWN, VK_DOWN, 0);
> - expect(0, res);
> -
> - ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX,
> -
>            ownerdata_multiselect_deselect_all_select_3_seq,
> -
>            "ownerdata multiselect: deselect all, select item 3", TRUE);
> -
> - res = SendMessageA(hwnd, LVM_GETSELECTEDCOUNT, 0, 0);
> - expect(1, res);

Hi Angelo,

Thanks for looking into this. There are a few things can be improved for this series.

Please complete the keyboard sequence by sending WM_KEYUP for VK_DOWN and
WM_KEYUP for VK_UP.

Also please add tests for pressing VK_UP while holding Shift after setting pressing VK_DOWN while holding Shift.
Same for holding both Shift and Control And add checks for NMLVODSTATECHANGE member values. For example,
I don't think uOldState should always be 0.
> -
> - flush_sequences(sequences, NUM_MSG_SEQUENCES);
> -
> - res = SendMessageA(hwnd, WM_KEYDOWN, VK_UP, 0);
> - expect(0, res);
> -
> - ok_sequence(sequences, PARENT_ODSTATECHANGED_SEQ_INDEX,
> -
>            ownerdata_multiselect_deselect_3_select_2_seq,
> -
>            "ownerdata multiselect: deselect item 3, select item 2", TRUE);
> - DestroyWindow(hwnd);
>
> }