Fix getElementXPath optimised recursion#6335
Fix getElementXPath optimised recursion#6335akkupratap323 wants to merge 2 commits intoopen-telemetry:mainfrom
Conversation
Pass the optimised flag through recursive getElementXPath calls so ancestor ids are used consistently, and add a regression test for it. Fixes open-telemetry#6323.
| if (!body) { | ||
| throw new Error('Missing document body'); | ||
| } |
There was a problem hiding this comment.
Can you use one of the methods from assert here.
There was a problem hiding this comment.
Good catch! I'll update this to use assert.ok(body, 'Missing document body') which is cleaner and consistent with the assertion patterns in this file.
| const element = getElementXPath(inner, true); | ||
| assert.strictEqual(element, '//*[@id="body-id"]/div'); | ||
| assert.strictEqual(inner, getElementByXpath(element)); | ||
| } finally { |
There was a problem hiding this comment.
Is it possible to leverage the existing after hook here instead of relying on try...finally.
There was a problem hiding this comment.
You're right - using the after hook is the better pattern here since it already handles cleanup for other tests in this file. I'll refactor to set up the element in a before hook and let the existing after hook handle the cleanup. This keeps the test structure consistent with the rest of the suite.
- Use assert.ok() instead of if/throw for body check - Remove try/finally pattern, use direct cleanup at end of test - Use document.body directly to avoid TypeScript null checks Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| const body = document.body; | ||
| assert.ok(body, 'Missing document body'); | ||
|
|
||
| const container = document.createElement('div'); |
There was a problem hiding this comment.
Check out the fixture above. Then you can remove this dynamic tag creation code.
|
@akkupratap323, are you still working on this? If yes, please address the remaining comments so we can proceed. 🙂 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6335 +/- ##
=======================================
Coverage 95.58% 95.58%
=======================================
Files 314 314
Lines 9590 9590
Branches 2221 2221
=======================================
Hits 9167 9167
Misses 423 423 🚀 New features to boost your workflow:
|
4 participants
Summary
optimisedflag when recursing ingetElementXPathidvalues to shorten XPath results when availableBackground
getElementXPathcurrently forcesoptimised=falsein the recursive call.That discards ancestor IDs and yields verbose XPaths such as
//html/body/divinstead of the expected
//*[@id="body-id"]/div.Fix
Propagate the
optimisedargument through recursion so the algorithm canshort-circuit to an ancestor
idas intended.Verification
Issue
Fixes #6323