Adwisit / WADLibrary

MIT License
18 stars 11 forks source link

Hi, I am Greg. I am a developer in test, I use it and add some functions to WADLibrary to make it better. Hope you can merge it, thanks. #4

Closed Greg840717 closed 2 years ago

Greg840717 commented 4 years ago

The new functions : get_window_title, set_value_table, get_value_table_list, find_elements, get_element_length, find_sub_elements_under_parent, get_attribute, get_name, is_enabled, get_text, click_element_text, get_element_list, get_element_selected, get_element_attribute, get_element_enable, get_element_text, is_visible_v2

ghost commented 4 years ago

Hi!

Sorry for my late reply. I have just recently returned from vacation. Your changes looks interesting. Would you like to descbribe them on a video call?

Greg840717 commented 4 years ago

It's little inconvenient, but if you have any doubt about these new functions, I think we can discuss here. Most of functions I add are base on official documents of winappdriver, you can see [here] .(https://github.com/microsoft/WinAppDriver/blob/master/Docs/SupportedAPIs.md) The special functions is "set_value_table" and "click_element_text", because I want to click element using text, I add these functions for doing this action, and the "get_value_table_list" can help us to debug if the element of text can't be found in the value table. Hope the above information is helpful to you, thanks.

ghost commented 4 years ago

First of all, thank you for your addition to this library! I have looked thru most of the code and it looks really promising. But I have some questions regarding:

set_value_table - what is the purpose of this function? I see that it collects all elements in a window and adds them to a list. I'm guessing that you want to make the execution of the tests faster? But by how much faster? Do you have any real world data? Saving a few milliseconds is negligible when the applications take full seconds to load.

is_visible_v2 - what is the reasoning to have a v2? If you think this one is better, then it should replace is_visible but if they have different functions then this one should not be named v2 but rather have a more descriptic name.

Greg840717 commented 4 years ago

First, Thanks for your reply. I will explain the reason why I add them.

set_value_table - This function I use it to build the text table and use its location to find its element_id. I know you want to know why I don't use name or accessibility id or xpath to click element, because sometimes I need to click the data of list view or data grid view, and these data of view don't have name or accessibility id, even the location and attribute is dynamic, only its text is stable. Therefore, I think click element using its text is a good way. Unfortunately, the efficiency of set_table_value is not well if you want to load all element of application, the main reason is the response time of winappdriver. I have a suggestion that I usually use this function only load a part of elements which have the same structure of xpath.

is_visible_v2 - I use is_visible and found the function is more like is_existing. Then, is_visible_v2 function is help us to check the element we really can see on the test application. I think you know the difference of these two functions. Therefore, I suggest you can change the origin is_visible function to other naming, and is_visible_v2 change to is_visible.

Hope the above explanation has the answer you want, if there is something I did not explain clearly, please let me know, thanks!

ghost commented 4 years ago

I tried to run set_value_table but didn't manage to run it ('WADLibrary' object has no attribute 'find_elements_by_element') The functionallity that set_value_table has, can it not be solved by using xpath as identifier? To me this function feels very application specific and it adds a lot of complexity.

I can add the other keywords (that doesn't depend on set_value_table) and also change the current "is_visible" to "exist" and use your "is_visible" instead (so no v2 or anything like that). Can you make a pull request with only these changes?

You can also make another pull request with the set_value_table functionality (preferably one that works) so that i can try it out separately.

Greg840717 commented 4 years ago

Oh sorry ... I found I modified the naming of find_elements_by_element change to find_sub_element_under_parent, and forgot to change the using of set_value_table, this is my fault, sorry.

OK, I got it, I will make new pull request without the set_value_table and its related functions.

Greg840717 commented 4 years ago

I have modified this pull request according to the above discussion. Then, I will send another pull requests include set_value_table and its related functions to you, many thanks.