-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat: update selenium atoms based on selenium 4.11.0 #268
Conversation
@jlipps Do you remember where this atoms change come from? This repo's
https://github.com/SeleniumHQ/selenium/blob/selenium-4.6.0/rb/BUILD.bazel Python has only find_elements, get_attribute, isdisplayed.
|
@KazuCocoa yeah maybe it was 3.141.59, since i think maybe that was what was in master when i did this? i don't remember. are you saying that many atoms have been removed in favour of direct remote debugger calls? |
I may not properly understand this usage well yet. I initially thought old(?) Selenium clients also used the same atoms, so this remote-debugger referred to them from the selenium project, although the latest selenium client (I only checked Ruby and python tho), had a few atoms only. Others were just endpoint calls defined in W3C. So... I just wondered if we only needed the same a few atoms only, not others nowadays (we may also need to update some this repo's implementation though, I guess). Sorry, this is not so concrete question now. |
Hmm no worries. Yeah maybe we don't need all the same atoms as before. But we should probably support the same set of behaviours. Maybe more of the clients just send their own JS snippets for many of these commands now, so we can get away with fewer atoms and just Execute Script? |
426730f
to
d711c8a
Compare
@mykola-mokhnach @KazuCocoa this PR has new atoms from selenium 4.11. it also has one that i built 'manually' but hope to add as a PR to selenium. all the remote debugger e2e tests pass, and the same tests pass in the xcuitest driver web set that passed before (not all of them passed initially for me on my machine, at least on ios 16.2, idk why). all that to say, this is now ready for review! hopefully an atoms update will fix some of the issues that our users have been reporting. |
Probably can we remove https://github.com/appium/appium-remote-debugger/blob/master/atoms-notes.md for now..? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't check everything, but your comment might indicate this will work (at least some points). Hopefully this will improve Safari remote debugger behavior
btw, i have update the title with |
OK, changes made based on feedback. My plan would be to publish this as a new major rev beta, and then create an xcuitest PR to see how it works |
OK I have published 10.0.0-beta.0. I did not merge this PR yet since autorelease is turned on. |
4532808
to
4dbc5b6
Compare
BREAKING CHANGE
WIP of bringing the newest selenium atoms over. things don't work and it's not exactly clear why. debugging is required.