Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search screen and return x/ypos #112

Closed
Harm10 opened this issue Sep 17, 2023 · 10 comments · Fixed by #114
Closed

Search screen and return x/ypos #112

Harm10 opened this issue Sep 17, 2023 · 10 comments · Fixed by #114

Comments

@Harm10
Copy link
Contributor

Harm10 commented Sep 17, 2023

I would like to have a command that searches for a string on the screen and returns its x/ypos if it finds it.
Perhaps also specify a beginning position to search specific parts of the screen.

@robinmatz
Copy link
Collaborator

Hey @Harm10 ,

I opened a PR with a first throw of resolving your request. This is how the acceptance tests for this look like.

Test Find String
    ${position}    Find String    Welcome
    Should Be Equal    ${{ [(1, 10)] }}    ${position}

Test Find String Case-Insensitive
    ${position}    Find String    Welcome    ignore_case=True
    Should Be Equal    ${{ [(1, 10), (9, 5)] }}    ${position}

Test Find String Without Result
    ${position}    Find String    ${STRING_NON_EXISTENT}
    Should Be Equal    ${{ [] }}    ${position}

Is the return format what you would expect? As you can see, the keyword returns a list of ypos, xpos tuples or an empty list if the string was not found.

In alignment with other keywords, there is an optial ignore_case parameter in order to do the search case-insensitively.

There are two things that I am not sure about.

  • The first one is the option to only return occurrences after a certain beginning position. Would this benefit your use case or would it do without?
  • The second one is the return format. As you can see in my other PR, for Get current cursor position #110 I thought about providing a mode parameter where the user can choose to return a dict instead of tuples. Would it make sense to also provide this parameter here? Or maybe do not support different modes at all also for Get current cursor position #110?

samuelpcabral added a commit that referenced this issue Oct 20, 2023
Draft: #112 Implement find string keyword
@Harm10
Copy link
Contributor Author

Harm10 commented Oct 23, 2023

@robinmatz Sorry I missed your questions. I get so many mails from GitHub that I easily miss things apparently.

Let me first react on the dict/tuples. Me personally I wouldn't mind. Being a software developer too I would advocate to give users as many options as possible. The one likes this format and the other something else.

About your other question: I would really like being able to specify a beginning position this would make it possible first to find a certain string and after that another string that should start somewhere on the screen after the position of the first string (does this make sense to you?)

I would also advocate having an option of limiting the amount of positions found. Suppose I want to find the first underscore on the screen that has lots of underscores on there I really do not want to find them all!

@robinmatz
Copy link
Collaborator

Hmm, ok. I opted to keep the option to let the user specify the return format. The keyword can return both a list of tuples or a list of dictionaries.

I think I will create another PR to also include a parameter to specify a beginning index, something like

Get String Positions    search_string    only_after={"ypos":10, "xpos": 25}

To keep it consistent, I think this only_after parameter should also be able to accept either a tuple (10, 25) or a dictionary as shown above. What do you thinkl?

@robinmatz robinmatz reopened this Oct 25, 2023
@Harm10
Copy link
Contributor Author

Harm10 commented Oct 25, 2023

I think your suggestion for only_after is a good one!

I do not have a use case for this at the moment but how about also adding only_before?

And to make it even "wilder": search string after/before another string? Or is this getting too complicated?

@robinmatz
Copy link
Collaborator

I was thinking about this request for a while. I think it would be better to split this only_after to a new keyword, because it would look more readable when using positional arguments.
Compare, e.g.,

Get String Positions     My search string     {"ypos": 5, "xpos": 10}

to

Get String Positions Only After    5    10    My search string

I mean, it still is not exactly what you would call "intuitive", but is already a lot better than the first variant.

About the only after / only before another string, I will have to think about it a bit

@robinmatz
Copy link
Collaborator

robinmatz commented Nov 3, 2023

Also, for consistency reasons, if you think about the existing Page Should Contain Match or Page Should Match Regex,
there could or maybe should be equivalent keywords like
Get Glob Positions and Get Regex Positions. But if we combine these also with Only After / Only Before / Only After String/Glob/Regex variants, we have an explosion of new keywords...

I know that for User Keywords, Robotframework, allows embedded arguments in keyword names, e.g.

*** Test Cases ***
Some Test Name
    Log My Text To Console
    Log Another Text To Console

*** Keywords ***
Log ${text} To Console
    Log To Console    ${text}

Potentially, assuming there is an equivalent for this for python libraries, it could prevent this explosion of keywords.

@robinmatz
Copy link
Collaborator

Also, for consistency reasons, if you think about the existing Page Should Contain Match or Page Should Match Regex, there could or maybe should be equivalent keywords like Get Glob Positions and Get Regex Positions. But if we combine these also with Only After / Only Before / Only After String/Glob/Regex variants, we have an explosion of new keywords...

I know that for User Keywords, Robotframework, allows embedded arguments in keyword names, e.g.

*** Test Cases ***
Some Test Name
    Log My Text To Console
    Log Another Text To Console

*** Keywords ***
Log ${text} To Console
    Log To Console    ${text}

Potentially, assuming there is an equivalent for this for python libraries, it could prevent this explosion of keywords.

Seems like it is possible in newer versions of robotframework (https://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html#embedding-arguments-into-keyword-names)

from robot.api.deco import keyword


@keyword('Add ${quantity:\d+} copies of ${item} to cart')
def add_copies_to_cart(quantity, item):
    # ...

would translate to Robotframework code

Add 10 Copies Of Chocolate To Cart

Now, how to translate this to our use case?

@Keyword("Get ${type} Positions"} --> allows for Get String Positions, Get Glob Positions, Get Regex Positions

@Keyword("Get ${type} Positions ${where}") --> Allows for Get String Positions Only After, Get String Positions Only Before, Get Glob Positions Only After, Get Glob Positions Only Before, etc.

@Harm10
Copy link
Contributor Author

Harm10 commented Nov 4, 2023

Is there a side-effect of having a lot of keywords (e.g. loading the package?)? In my view having a direct keyword for getting what you want is more intuitive than using a specific option of a keyword. Having said that if you need to use more than 1 keyword after each other also makes your test less readable and looking complicated.
So I guess it is a trade-off but where exactly?

@robinmatz
Copy link
Collaborator

I have tried out the option of using an embedded option, but it seems that this is not available, because mixing embedded arguments and positional arguments is not possible for python library keywords. It might become available in later versions of robotframework, but I do not think this is worth waiting for, and it would also imply that this library would only be available with the newest versions of robotframework.

So, we are basically left with creating many keywords. But I guess this would be ok, too.

@Harm10
Copy link
Contributor Author

Harm10 commented Nov 4, 2023

The more keywords the better :-) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants