[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