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

Internal: More reliable internal testing helper implementation #2057

Merged
merged 2 commits into from
Dec 6, 2024

Conversation

caendesilva
Copy link
Member

Fixes an issue where the old implementation does not work reliably on macOS when running the full suite.


Explanation of Changes:

  1. Using XPath: XPath is a powerful language for querying XML documents (and HTML, which is XML-like). It's much more efficient and reliable for complex queries than manual traversal.

  2. DOMXPath Object: We create a DOMXPath object using the existing $this->document object, which is already a parsed DOM. This avoids re-parsing the HTML.

  3. Robust Class Matching: The XPath expression //*[contains(concat(' ', normalize-space(@class), ' '), ' $class ')] is a standard and robust way to find elements with a specific class, even if they have multiple classes. It handles leading/trailing spaces correctly.

  4. Iterating DOMNodeList: The query() method returns a DOMNodeList. We iterate over this list and add the matching elements to our collection.

  5. Type Safety: The added if ($node instanceof DOMElement) check ensures type safety and prevents unexpected behavior if non-element nodes are encountered in the DOMNodeList.

  6. parseNodeRecursive: We reuse your existing parseNodeRecursive method to convert the DOMElement objects into TestableHtmlElement objects for consistency.

Why this is better:

  • Reliability: XPath is standardized and less prone to platform-specific quirks.
  • Efficiency: XPath is generally faster than manual DOM traversal.
  • Maintainability: The code is simpler and easier to understand.
  • Robustness: The class matching logic is more robust.

@github-actions github-actions bot added the internal This pull request does not affect package code label Dec 6, 2024
@caendesilva caendesilva enabled auto-merge December 6, 2024 12:30
@caendesilva caendesilva merged commit 88ef637 into master Dec 6, 2024
21 checks passed
@caendesilva caendesilva deleted the more-reliable-internal-testing-helper branch December 6, 2024 12:31
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (e040ace) to head (673573c).
Report is 13 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##              master     #2057   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity      1766      1766           
===========================================
  Files            182       182           
  Lines           4758      4758           
===========================================
  Hits            4758      4758           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal This pull request does not affect package code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant