Challenge

  1. Given
    • the drupalRenderPage() helper class method from the patch in http://drupal.org/node/1279226#comment-5949082
    • the knowledge that a DrupalWebTestCase installs an entirely new Drupal site for every single test* method in a test case
  2. When looking at CommonJavaScriptTestCase and realizing that none of the gazillion test methods needs a completely fresh Drupal site.
  3. Then
    • refactor the test methods in CommonJavaScriptTestCase so there is only one test method (but retaining the test method comments as inline comments)
    • merge the new CommonJavaScriptDefaultsTestCase test case into the former (if it exists already)
    • get the patch committed to Drupal 8
    • backport the patch to Drupal 7.
CommentFileSizeAuthor
#2 1558706_1_js_tests.patch11.28 KBcosmicdreams

Comments

cosmicdreams’s picture

cosmicdreams commented
Assigned: Unassigned » cosmicdreams

Awesome description sun. and it sounds fun. I'll try to do this one tonight.

cosmicdreams’s picture

cosmicdreams commented
Status: Active » Needs review
StatusFileSize
new 1558706_1_js_tests.patch11.28 KB

half-way done. Seeing how the tests respond

Status: Needs review » Needs work

The last submitted patch, 1558706_1_js_tests.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ b/core/modules/system/tests/common.test
@@ -1135,23 +1134,20 @@ class CommonJavaScriptTestCase extends WebTestBase {
   function tearDown() {
     // Restore configured value for JavaScript preprocessing.
-    $config = config('system.performance');
-    $config->set('preprocess_js', $this->preprocess_js);
-    $config->save();
+    config('system.performance')
+      ->set('preprocess_js', $this->preprocess_js)
+      ->save();

This restore of config can be entirely removed.

+++ b/core/modules/system/tests/common.test
@@ -1214,79 +1198,46 @@ class CommonJavaScriptTestCase extends WebTestBase {
+    // Test adding a JavaScript file with a different group.
...
+    // Test adding a JavaScript file with a different weight.

Each new test needs to start with a reset, so previous drupal_add_js() calls do not interfere with new ones.

It would be a good idea to add a custom resetJS() method to this test, which performs the reset, so it can possibly be easily adjusted in one place later.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Closed (won't fix)

I'm sorry to do this, but in the meantime, #1411074: Add a flag to set up test environment only once per test class; introduce setUpBeforeClass() and tearDownAfterClass() [PHPUnit] is the actual and real solution, which makes this issue obsolete and kinda approaches the problem from the opposite direction.

CommonJavaScriptTestCase was already fixed in there.

Regardless of that, thanks for jumping on this issue and trying to resolve it! :)