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.

Sign up for GitHub

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

Jump to bottom

Broken code coverage collection #3337

Closed
swissspidy opened this issue Sep 24, 2019 · 1 comment · Fixed by #3341
Closed

Broken code coverage collection #3337

swissspidy opened this issue Sep 24, 2019 · 1 comment · Fixed by #3341
Labels
Bug Something isn't working Infrastructure Changes impacting testing infrastructure or build tooling
Milestone
v1.3

Comments

@swissspidy
Copy link
Collaborator

The code coverage analysis job on Travis that runs on develop currently fails due to Maximum function nesting level of '256' reached errors.

Example: https://travis-ci.org/ampproject/amp-wp/jobs/588932199

The failings tests are as follows:

  • AMP_Tag_And_Attribute_Sanitizer_Test::test_body_sanitizer with data set deeply_nested_elements_250
  • AMP_Tag_And_Attribute_Sanitizer_Test::test_html_sanitizer with data set #228 ('<html amp><head><meta charset.../html>', null, array(), null)

Looks like these were introduced in #3243.

Since the first test is specifically for deeply nested elements, we might need to increase the maximum function nesting level by quite a bit to accomodate for this during code coverage analysis.

This could be done by adding a custom php.ini config file with the content xdebug.max_nesting_level=9999 (or similar) and using phpenv config-add path/to/custom/php.ini (docs) to load it. I guess we could even create that config on the fly just for this one job.

Related:
@swissspidy swissspidy added Bug Something isn't working Infrastructure Changes impacting testing infrastructure or build tooling labels Sep 24, 2019
@swissspidy swissspidy mentioned this issue Sep 24, 2019
Merged
@westonruter
Copy link
Member

We could also just do a quick fix to just reduce the DOM nesting level to something like 200 which should bring the total stack trace below 256. Having a document with elements nested 200 levels seems just about as (un)common as nesting 250 levels.

@westonruter westonruter mentioned this issue Sep 24, 2019
Merged
@westonruter westonruter added this to the v1.3 milestone Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Something isn't working Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Milestone
v1.3
Development

Successfully merging a pull request may close this issue.

Limit deeply-nesting test to 200 to fix Xdebug error ampproject/amp-wp
2 participants
@westonruter @swissspidy