Todo

* Update documentation in settings.php to warn that enabling twig_debug mode will break tests.

Original issue description

I don't think is is necessarily a bug as such, but automated tests that perform assertions directly against rendered HTML will fail when Twig debug mode is enabled. This is because the debug mode wraps the output of each Twig template with HTML comments containing debug information, so while the element is still intact and correct, performing a direct comparison against known good HTML will fail.

For example, this small test fragment fails when debug mode is on, even though the output is displayed correctly to the user

$img_tag = theme('image_style', $variables);
$this->assertEqual($img_tag, '<img class="image-style-test" src="' . $url . '" width="120" height="60" alt="" />');

In an ideal world, tests shouldn't be doing assertions against HTML strings IMO, but since that's not a fight I want to get into I suggest adding a note to the documentation in default.settings.php that warns of potential false failures when debug mode is enabled.

CommentFileSizeAuthor
#14 drupal-twig_debug_markup_tests_doc_note-1971860-14.patch770 bytesarh1
#4 drupal-twig_debug_markup_tests_doc_note-1971860-4.patch779 bytesarh1

Comments

star-szr’s picture

star-szr
he/him
Primary language English
commented

From #1898458-22: theme_test.module - Convert PHPTemplate templates to Twig:

Might just be a matter of defaulting twig_debug to be off for all tests except when testing twig_debug itself :)

Need to think about this some more but it might work.

fabianx’s picture

fabianx commented
Issue tags: +Twig

Tagging

fabianx’s picture

fabianx commented
Issue summary: View changes

fix typo

fabianx’s picture

fabianx commented
Issue tags: +Documentation, +Novice

Tagging with novice and documentation.

arh1’s picture

arh1 commented
StatusFileSize
new drupal-twig_debug_markup_tests_doc_note-1971860-4.patch779 bytes

Here's a patch to add documentation about this issue.

arh1’s picture

arh1 commented
Status: Active » Needs review

Status: Needs review » Needs work
Issue tags: -Documentation, -Novice, -Twig

The last submitted patch, drupal-twig_debug_markup_tests_doc_note-1971860-4.patch, failed testing.

mariafromatoz’s picture

mariafromatoz commented
Status: Needs work » Needs review
Issue tags: +Documentation, +Novice, +Twig

#4: drupal-twig_debug_markup_tests_doc_note-1971860-4.patch queued for re-testing.

fabianx’s picture

fabianx commented
Status: Needs review » Reviewed & tested by the community

Looks good to me!

star-szr’s picture

star-szr
he/him
Primary language English
commented
Title: Twig debug mode breaks tests » Document that Twig debug mode breaks tests
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Component: theme system » documentation
Assigned: Unassigned » jhodgdon
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: jhodgdon » Unassigned
Status: Reviewed & tested by the community » Needs work

Could we perhaps make the wording of this a bit better? It currently says:

Note that this debugging markup will cause any automated tests made directly against rendered HTML to fail. When performing automated tests, 'twig_debug' should be set to FALSE.

It seems a bit odd to me... I don't think that we "make tests against" anything (we make test assertions about stuff, and we compare values in tests), and I don't think that we "perform" tests (we run them).

alexander_danilenko’s picture

alexander_danilenko commented

Looks good. I am not english native speaker but it is good to understand

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

As a native speaker of English, I found it very awkward.... How about:

Note that this debugging markup will cause automated tests that directly check rendered HTML to fail. When running automated tests, 'twig_debug' should be set to FALSE.

arh1’s picture

arh1 commented
Status: Needs work » Needs review
StatusFileSize
new drupal-twig_debug_markup_tests_doc_note-1971860-14.patch770 bytes

That wording sounds fine to me. Updated patch attached.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: Unassigned » jhodgdon
Status: Needs review » Reviewed & tested by the community

OK, thanks! I'll get that committed shortly.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: jhodgdon » Unassigned
Status: Reviewed & tested by the community » Fixed

Thanks again! Committed to 8.x.

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.