Skip to content

Fix GH-11160: Few tests failed building with new libxml 2.11.0 #11162

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

Closed
wants to merge 1 commit into from

Conversation

nielsdos
Copy link
Member

It's possible to categorise the failures into 2 categories:

  • Changed error message. In this case we either duplicate the test and modify the error message. Or if the change in error message is small, we use the EXPECTF matchers to make the test compatible with both old and new versions of libxml2.
  • Missing warnings. This is caused by a change in libxml2 where the parser started using SAX APIs internally [1]. In this case the error_type passed to php_libxml_internal_error_handler() changed from PHP_LIBXML_ERROR to PHP_LIBXML_CTX_WARNING because it internally started to use the SAX handlers instead of the generic handlers. However, for the SAX handlers the current input stack is empty, so nothing is actually printed. I fixed this by falling back to a regular warning without a filename & line number reference, which mimicks the old behaviour. Furthermore, this change now also shows an additional warning in a test which was previously hidden.

[1] https://gitlab.gnome.org/GNOME/libxml2/-/commit/9a82b94a94bd310db426edd453b0f38c6c8f69f5

It's possible to categorise the failures into 2 categories:
  - Changed error message. In this case we either duplicate the test and
    modify the error message. Or if the change in error message is
    small, we use the EXPECTF matchers to make the test compatible with both
    old and new versions of libxml2.
  - Missing warnings. This is caused by a change in libxml2 where the
    parser started using SAX APIs internally [1]. In this case the
    error_type passed to php_libxml_internal_error_handler() changed from
    PHP_LIBXML_ERROR to PHP_LIBXML_CTX_WARNING because it internally
    started to use the SAX handlers instead of the generic handlers.
    However, for the SAX handlers the current input stack is empty, so
    nothing is actually printed. I fixed this by falling back to a
    regular warning without a filename & line number reference, which
    mimicks the old behaviour. Furthermore, this change now also shows
    an additional warning in a test which was previously hidden.

[1] https://gitlab.gnome.org/GNOME/libxml2/-/commit/9a82b94a94bd310db426edd453b0f38c6c8f69f5
@nielsdos nielsdos linked an issue Apr 29, 2023 that may be closed by this pull request
@andypost
Copy link
Contributor

Thank you! it fixes all tests on 7 arches

@andypost
Copy link
Contributor

btw using patch with PHP-8.0

BORKED TEST SUMMARY
---------------------------------------------------------------------
duplicated SKIPIF section [/builds/alpine/aports/testing/php8/src/php-8.0.28/ext/dom/tests/DOMDocument_loadXML_error2_pre2_11.phpt]
duplicated SKIPIF section [/builds/alpine/aports/testing/php8/src/php-8.0.28/ext/dom/tests/DOMDocument_load_error2_pre2_11.phpt]

@nielsdos
Copy link
Member Author

btw using patch with PHP-8.0

BORKED TEST SUMMARY
---------------------------------------------------------------------
duplicated SKIPIF section [/builds/alpine/aports/testing/php8/src/php-8.0.28/ext/dom/tests/DOMDocument_loadXML_error2_pre2_11.phpt]
duplicated SKIPIF section [/builds/alpine/aports/testing/php8/src/php-8.0.28/ext/dom/tests/DOMDocument_load_error2_pre2_11.phpt]

In 8.0 the way to check for extension availability was different than in 8.1+. This means that on 8.0 the tests already have a SKIPIF section, which is what the test runner complains about. If you want to fix it on 8.0 then you need to place the if-check I added below the <?php include('skipif.inc'); ?> line.

@andypost
Copy link
Contributor

Used your suggestion and now both pass https://gitlab.alpinelinux.org/alpine/aports/-/pipelines/162300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Few tests failed building with new libxml 2.11.0
3 participants