Re: [PATCH 1/2] comctl32: Added getter-setter tests for the tab control (second attempt)

[Vitaliy Margolen]

The_Hagop wrote:
> @@ -1,5 +1,6 @@
>  /* Unit test suite for tab control.
> -
> -
> - Copyright 2007 Hagop Hagopian
> - Copyright 2003 Vitaliy Margolen
> -
> - This library is free software; you can redistribute it and/or

These are sorted in  historical order.
> @@ -52,6 +59,8 @@
>
> static HFONT hFont = 0;
>
> +static HWND hTab;
> -
>
> static HWND
>  create_tabcontrol (DWORD style, DWORD mask)
>  {

You using exactly the same name as parameter to all of your functions.
This creates lots of confusion. Please remove this global variable and
pass it as a function parameter instead.
> - /* Creating a tooltip window*/
> - hwndTT = CreateWindowEx(WS_EX_TOPMOST,
> -
>    TOOLTIPS_CLASS,
> -
>    NULL,
> -
>    WS_POPUP | TTS_NOPREFIX | TTS_ALWAYSTIP,
> -
>    CW_USEDEFAULT,
> -
>    CW_USEDEFAULT,
> -
>    CW_USEDEFAULT,
> -
>    CW_USEDEFAULT,
> -
>    hTab,
> -
>    NULL,
> -
>    0,
> -
>    NULL
> -
>    );
> -
> - SetWindowPos(hwndTT,
> -
>    HWND_TOPMOST,
> -
>    0,
> -
>    0,
> -
>    0,
> -
>    0,
> -
>    SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE);

This looks just plain bad. Please put all parameters one after another.
There are absolutely no need to have each on it's own line.
> - ti.rect.left = rect.left;
> - ti.rect.top = rect.top;
> - ti.rect.right = rect.right;
> - ti.rect.bottom = rect.bottom;

You can assign structures to structures in C:
ti.rect = rect;
> - todo_wine{
> -
>    /* Testing CurFocus with negative value */
> -
>    SendMessage(hTab, TCM_SETCURFOCUS, -1, 0);
> -
>    focusIndex = SendMessage(hTab, TCM_GETCURFOCUS, 0, 0);
> -
>    expect(-1, focusIndex);
> - }

Here and all other places - the only part of the test that's failing is
"expect(,)". Add "todo_wine" at front of it only.

Also it would be interesting to test larger negative numbers, like -10.
This is because number of places use -1 to indicate that nothing is
selected. So in a sense you might be getting a correct number.
> - expect((int) extendedStyle, (int) prevExtendedStyle);
> - todo_wine{
> -
>    extendedStyle = SendMessage(hTab, TCM_GETEXTENDEDSTYLE, 0, 0);
> -
>    expect(TCS_EX_FLATSEPARATORS, (int) extendedStyle); 
> - }

Why are you casting DWORD to int?
> +static void test_getters_setters(INT nTabs)
> +{
> - RECT rTab;
> - INT nTabsRetrieved;
> - INT rowCount;
> -
> - hTab = createFilledTabControl(TCS_FIXEDWIDTH, TCIF_TEXT|TCIF_IMAGE, nTabs);
> - ok(hTab != NULL, "Failed to create tab control\n");
> -
> - SendMessage(hTab, TCM_SETMINTABWIDTH, 0, -1);
> -
> - /* Testing GetItemCount */
> - nTabsRetrieved = SendMessage(hTab, TCM_GETITEMCOUNT, 0, 0);
> - expect(nTabs, nTabsRetrieved);
> -
> - /* Testing GetRowCount */
> - rowCount = SendMessage(hTab, TCM_GETROWCOUNT, 0, 0);
> - expect(1, rowCount);
> -
> - /* Testing GetItemRect */
> - SendMessage(hTab, TCM_GETITEMRECT, 0 , (LPARAM) &rTab );
> - CheckSize(hTab, TAB_DEFAULT_WIDTH, -1 , "Default Width");
> -
> - test_getset_curFocus(hTab, nTabs);
> - test_getset_curSel(hTab, nTabs);
> -
> - test_getset_extendedStyle(hTab);
> - test_getset_unicodeFormat(hTab);
> - test_getset_item(hTab);
> - test_getset_tooltip(hTab);
> -
> - DestroyWindow(hTab);
>
> +}

All your small functions should go inside this function. There is no
need to create 100 small functions that do 1-3 tests.
> -
> - test_getters_setters(5);
>
> }

This part part of your patch appears to be malformed.

Vitlaiy