Uh oh!
There was an error while loading. Please reload this page.
add default prefix and allow prefixes with spaces - #335
Conversation
ombre42
commented
Oct 1, 2014
An alternate solution I have already mentioned is to not raise an error if the prefix is not found in our list, but fail over to the default strategy using the entire value of |
There was a problem hiding this comment.
This looks strange. Doesn't assertRaises catch the ValueError so except ValueError block should never be executed? That block seems to test the actual error message. If you want to accomplish that, you can either use the standard assertRaisesRegexp (new in Python 2.7), Robot's assert_raises_with_msg, or just execute the code failing code normally and actually catch the error with except ValueError.
There was a problem hiding this comment.
Good catch! That was copypasta on my part - the unit test was broken before my changes. It looked funny to me too, but my mind was elsewhere. Decided to use the utility method due to shortcomings in 2.6 you mentioned. More readable than standard catch. Thanks!
pekkaklarck
commented
Oct 1, 2014
I think adding |
ombre42
commented
Oct 3, 2014
@pekkaklarck Thanks for your input. I'm glad you think its a good idea - I was a bit on the fence as this is rarely needed and easily worked around. |
pekkaklarck
commented
Oct 3, 2014
Looks good to me. |
ombre42
commented
Oct 4, 2014
@pekkaklarck Thanks again. Manual merge is a little painful. Going to try and avoid that in the future. |
emanlove
commented
Oct 4, 2014
Sorry for not chiming in sooner. My thoughts go to the question of trying not to have too many similar locators. This might not be an issue but I would prefer not to see a lot of find exact match and find partial match for links, images, etc. As I mentioned in the post of SmartClient/SmartGWT locators possible having a locator plug and play code... Really just need some time to focus on this and get my thoughts in order. |
ombre42
commented
Oct 5, 2014
@emanlove you have a point. The purpose of this project is to make testing with Selenium easier. The reason I found adding partial link acceptable was because it can improve the readability of a test and is easier than the alternative. |
emanlove
commented
Oct 7, 2014
No, go ahead and leave it in there. Someone saw a need, your code made a good fix, and a problem was solved. Keep it in. |
This is a change I would like peer-reviewed. If the default strategy changes are rejected, I can submit a separate request to just allow prefixes with spaces.
Allows you to use the default strategy (match key attributes) when a '=' is in the locator.
Locator
page?a=bwill still cause error, but can be changed todefault=page?a=b.Locator prefixes will now be space-insensitive so the following will work when the previously did not:
partiallink=textx path = //divFixes#193