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

Fix _find_by_data_locator #1924

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

markus-leben
Copy link

@markus-leben markus-leben commented Oct 7, 2024

Off by one in _find_by_data_locator which causes opaque failure on data values which contain a colon.

Example html:
<div data-automation-id="foo:bar">This might be trouble</div>

Example robot:
Scroll Element Into View data:automation-id:foo:bar

Error message:
Provided selector (automation-id:foo:bar) is malformed. Correct format: name:value.

I don't love the person putting colons in their automation ID, but we don't always get to choose the html we're testing.

Fixed off by one in _find_by_data_locator
@emanlove emanlove self-assigned this Oct 7, 2024
@emanlove emanlove added this to the Oct 2024 milestone Oct 7, 2024
@markus-leben
Copy link
Author

On second thought it might be a good idea to also change the error raised in the try or the message given in the except. The error that gets caught by that catch is a ValueError about unpacking with string's split() method:
ValueError: too many values to unpack (expected 2)

But the error message from the except in _find_by_data_locator catches that ValueError and replaces it with something that's in this case more vague, and implies that the reason the ValueError is raised is because of the condition in the try, rather than an unrelated ValueError.

I'm not sure what the ideal fix here would be though.

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

Successfully merging this pull request may close these issues.

2 participants