On Mon, Sep 27, 2021 at 04:10:39PM +0200, Piotr Caban wrote:
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.
Yeah, I added these for future simple element tests, but they may not work. I can remove them for now and test this later when we've got a case of this.
- /*
* 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
In general, do you think this style of test is too complicated? I did it in hopes of making future tests for different control objects cleaner/more readable, but if really long test functions are preferable, I can do it that way. My main issue with doing long function body tests is that after awhile I find it hard to keep track of what is being tested.
Thanks for the review!