Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Assigned: Unassigned » Wim Leers
Crell’s picture

My comment from the previous thread:

+++ b/core/modules/filter/lib/Drupal/filter/Tests/FilterAPITest.php
@@ -0,0 +1,111 @@
+  /**
+   * Tests the ability to apply only a subset of filters.
+   */
+  function testCheckMarkup() {
+    $text = "Text with <marquee>evil content and</marquee> a URL: http://drupal.org!";
+    $expected_filtered_text = "Text with evil content and a URL: <a href=\"http://drupal.org\">http://drupal.org</a>!";
+    $expected_filter_text_without_html_generators = "Text with evil content and a URL: http://drupal.org!";
+
+    $this->assertIdentical(
+      check_markup($text, 'filtered_html', '', FALSE, array()),
+      $expected_filtered_text,
+      'Expected filter result.'
+    );
+    $this->assertIdentical(
+      check_markup($text, 'filtered_html', '', FALSE, array(FILTER_TYPE_MARKUP_LANGUAGE)),
+      $expected_filter_text_without_html_generators,
+      'Expected filter result when skipping FILTER_TYPE_MARKUP_LANGUAGE filters.'
+    );
+    // Related to @see FilterSecurityTest.php/testSkipSecurityFilters(), but
+    // this check focuses on the ability to filter multiple filter types at once.
+    // Drupal core only ships with these two types of filters, so this is the
+    // most extensive test possible.
+    $this->assertIdentical(
+      check_markup($text, 'filtered_html', '', FALSE, array(FILTER_TYPE_HTML_RESTRICTOR, FILTER_TYPE_MARKUP_LANGUAGE)),
+      $expected_filter_text_without_html_generators,
+      'Expected filter result when skipping FILTER_TYPE_MARKUP_LANGUAGE filters, even when trying to disable filters of the FILTER_TYPE_HTML_RESTRICTOR type.'
+    );
+  }
+

This should be two separate tests. Merging two tests into a single test method is a moderately large testing faux pas.

Crell’s picture

To clarify per Wim's request, this method is testing both $expected_filtered_text and the "without html generators" situations. That's wrong. There should be one test that sets up and runs the first assertIdentical(), and a second test that sets up and runs the second assertIdentical(). Yes, there will be some overlap between those methods. That's OK. In unit tests, repeated code between methods is OK (or can be factored to a utility method); the focus is on making each test method as focused and single-purpose as possible, so that if something fails you get good fine-grained results.

And of course these should all be unit tests, not web tests, but that may not be possible until the code is refactored to be OO post-December.

Wim Leers’s picture

Gotcha. I've always scoped unit tests differently, the same way as Qt's tests do it: one test method per method/function that you want to test, and that test method will then cover all possible cases (e.g. if the method accepts an int, then it tries -1, 0, 1, 1000, and MAX_INT-1 or so). Either way works for me.

IIRC I tried to make it a UnitTestCase subclass, but that failed, IIRC because check_markup() is not available for unit tests.

Wim Leers’s picture

Status: Active » Needs review
FileSize
2.63 KB

1) Making this a unit test is impossible AFAICT. Unit tests imply no DB access; check_markup() relies on filter_formats() which access the DB. It is possible to work around that (e.g. by filling its static cache), but I doubt that's acceptable in core.
2) Implemented what you asked for in #3. Also fixed a typo.
3) Could you please point me to docs that state what you said in #3 is indeed required for Drupal core tests? If not, we should create that documentation, or otherwise others might do it the same way I did it.

alansaviolobo queued 5: 1816162-5.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 5: 1816162-5.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.