[PATCH v4 8/8] oleacc: Add tests for default edit accessible object.

[Piotr Caban]

Hi Connor,

On 9/24/21 10:53 PM, Connor McAdams wrote:
> +typedef struct
> +{
> +    INT child_id;
> +
> +    const WCHAR *expected_name;
> +    HRESULT expected_name_hr;
> +
> +    const WCHAR *expected_value;
> +    HRESULT expected_value_hr;
> +
> +    const WCHAR *expected_description;
> +    HRESULT expected_description_hr;
> +
> +    const WCHAR *expected_help;
> +    HRESULT expected_help_hr;
> +
> +    const WCHAR *expected_kbd_shortcut;
> +    HRESULT expected_kbd_shortcut_hr;
> +
> +    const WCHAR *expected_default_action;
> +    HRESULT expected_default_action_hr;
Do we really need all these *_hr fields? Looking on the tests the 
functions are always returning non-NULL pointer+S_OK and NULL+S_FALSE.

> +    LONG left_offset, top_offset, width_offset, height_offset;
I bet you're adding it for future use but currently all of these fields 
are always 0. It's better to add them when they are actually used. I'm 
not sure if hard-coding exact values will work here.

> +    /*
> +     * Tests beyond this point are only applicable to full accessible objects
> +     * and not simple elements.
> +     */
> +    if (vals->child_id != CHILDID_SELF)
> +        return;
The code is not used at this point. It's better to add this check while 
adding tests with different child_id.

> +static void test_default_edit_accessible_object(void)
> +{
> +    HWND hwnd, label0, label1, btn0, btn1;
> +    IAccessible *acc;
> +    HWND edit[4];
> +    HRESULT hr;
> +    VARIANT v;
> +    BSTR str;
> +
> +    /* Create a window that looks like this:
> +     * +----------------------------------------+
> +     * |   ___________________________________  |
> +     * |  |___________________________________| |
> +     * |                (edit[0])               |
> +     * |          ____________________________  |
> +     * | Label0: |____________________________| |
> +     * |(label0)       (edit[1])  (password)    |
> +     * |          ______   _________________    |
> +     * | Label1: |button| |multi-line edit  |   |
> +     * |(label1)  (btn0)  |_________________|   |
> +     * |                     (edit[2])          |
> +     * |   __________________________________   |
> +     * |  |                                  |  |
> +     * |  |    edit with embedded button     |  |
> +     * |  |__________________________________|  |
> +     * |    (edit[3])             (btn1)        |
> +     * +----------------------------------------+
> +     */
We're generally trying to avoid adding comments when code is 
self-explanatory. As far as I understand the positions are not important 
anyway so maybe it would be even better to show that in tests.

Thanks,
Piotr