Repository navigation
Support Capybara's finder hints in find_css and find_xpath - #347
joeljunstrom wants to merge 1 commit into
Conversation
55406ce to
5ed6188
Compare
Capybara passes uses_visibility, texts, styles and position to finders that take them and reads the answers from Node#initial_cache. Without them it asked each candidate for its visibility and text in round trips of its own. One evaluate returns each node with its answers, which Ferrum's deep serialization brings back in a single call. Texts drop a node only when neither all its text nor its visible text holds them, case sensitively, as Capybara matches. Browser#find and #find_within still return nodes.
5ed6188 to
3fe08cc
Compare
navidemad
left a comment
There was a problem hiding this comment.
I compared this with the finder-hint work in capybara-lightpanda and checked the PR against its base using Capybara 3.40.0, Chrome 156, Ferrum main fb7d858, and released Ferrum 0.19.1.
The selective text-search improvement is substantial. I would address the two performance regressions below before merging. The separate lazy-cache comment is non-blocking, given the existing Selenium precedent.
The eight added finder-hint examples pass in a focused harness using their original test block and fixture. Targeted base/PR reproductions and measurements were independently rerun. The full upstream suite was not rerun locally. Timings are local measurements of a synthetic worst case, not general performance guarantees.
| let nodes = this.find(method, selector, within).filter(node => node.nodeType == Node.ELEMENT_NODE); | ||
|
|
||
| if (hints.texts.length > 0) { | ||
| nodes = nodes.filter(node => this.containsTexts(node, hints.texts, hints.visible)); |
There was a problem hiding this comment.
[P2] Bound eager hint work for large candidate sets
first can stop filtering after its first match, but this path computes text hints and visibility for every retained candidate. In a synthetic worst-case fixture with 1,000 nested divs containing needle at the leaf, page.first(:css, "div", text: "needle", wait: 0) had local three-run medians of approximately 13 ms on base and 174 ms on this branch with Ferrum main fb7d858, excluding DOM construction. The visibility ancestor walks alone perform quadratic work: instrumenting getComputedStyle during hint collection counted 5,250 / 20,500 / 81,000 calls for 100 / 200 / 400 nested candidates.
An experimental fallback above 512 candidates reduced the median to approximately 17 ms. Could we bound eager work and leave larger candidate sets to Capybara’s lazy filtering? The cutoff is experimental; a fallback would still need to exclude non-elements and preserve the node/cache representation.
There was a problem hiding this comment.
I'll look into it! Had issues running the local suite for some reason after rebasing so it was a bit of a shot in the dark.
| position: hints.fetch(:position, false) | ||
| } | ||
|
|
||
| evaluate("_cuprite.findWithHints(...arguments)", *[method, selector, hints, within].compact) |
There was a problem hiding this comment.
[P2] Avoid nested transport overhead on supported Ferrum 0.19.1
The gemspec allows Ferrum ~> 0.19.1, whose serializer recursively fetches object properties. Returning [node, {}] pairs adds two Runtime.getProperties requests per match even when no hints are requested. For 300 list items, page.all("li", visible: :all).size increases from 303 to 903 CDP commands; direct Browser#find has the same regression.
Ferrum main’s deep serializer avoids this (both variants use one command), but the Gemfile’s main-branch dependency does not protect released-gem users. Could the no-hints path transport a plain node array and construct empty-cache pairs in Ruby, or require a released Ferrum version containing deep serialization before shipping?
There was a problem hiding this comment.
Probably very reasonable to have this as ”on if available” kind of deal so checking the Ferrum capabilities sounds like a good idea
| return nodes.map(node => { | ||
| let cache = {}; | ||
| if (hints.visible) { | ||
| cache.visible = this.isVisible(node); |
There was a problem hiding this comment.
[P3, non-blocking] Make the changed lazy-result semantics explicit
This changes how not-yet-filtered candidates behave after the DOM changes. Starting with <div id="first">first</div><div id="second">second</div>:
result = page.all("div", minimum: 1)
page.execute_script("document.querySelector('#second').style.display = 'none'")
result.map { |node| node[:id] }Base returns ["first"] while this branch returns ["first", "second"]. Style and position filters behave similarly because later candidates retain their initial hints.
Capybara’s Selenium optimization already uses these caches for larger candidate sets, so this is a compatibility decision rather than an unsupported use of initial_cache. Could we explicitly test and document the intended snapshot behavior?
|
I fixed main branch, due to JS API changes. Can you rebase? |
Capybara passes
uses_visibility,texts,stylesandpositionto drivers whose finders accept them, and reads the answers fromNode#initial_cache. Cuprite's finders took only the selector, so Capybara asked every candidate for its visibility and text separately, about 8 CDP commands per node.The browser now answers the hints in the same call that finds the nodes. It returns each node with its answers as JSON, read from the array's handle, so nodes and answers can't come from different pages. Ferrum only resolves arrays that hold nothing but nodes, which is why the handle is read directly.
Text is filtered case-sensitively against all text or visible text, the strings Capybara compares, so nothing Capybara would match is dropped. Selenium's lowercased
textContentfilter misses a word split by a hidden element.On a list of 300 items, every third one hidden:
find("li", text: "Item 298")all("li", minimum: 1)assert_selector("li", text: "Item 2", count: 75)first("li", text: "Item 298", below: …)In a (well my) Rails app's suite the gain is smaller: about 10–20% fewer commands per test, with no measurable wall-time change, because most finds match few elements.
Browser#findand#find_withinstill return nodes.