Problem/Motivation

Spin off from #2809533: Convert AJAX part of \Drupal\system\Tests\Ajax\FrameworkTest to JavascriptTestBase

While converting tests it turned out that 'stylesheets-remove' wasn't removing css files at all. There is existing test coverage for this, but when this is updated to work properly, this breaks too.

The file was removing a non existing file but the test was using assertNoText which doesn't look at tag attributes so that was always going to pass, negative assertions--

Proposed resolution

Figure out why the BrowserTestBase tests break and either update those or fix the functionality

Fix the test

Remaining tasks

Figure out what the problem is.

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lendude created an issue. See original summary.

Lendude’s picture

Status: Active » Needs work
FileSize
937 bytes

This should pass, but doesn't.... this might be a problem in how the test is set up and not a bug, but we need to figure out which it is.

Lendude’s picture

Status: Needs work » Needs review
Lendude’s picture

this seems to work better, which indicates that its just the coverage that is broken.

The last submitted patch, 2: 3006625-2.patch, failed testing. View results

Lendude’s picture

Title: stylesheets-remove is broken or tests for it are broken » 'stylesheets-remove' test is broken
Issue summary: View changes
Issue tags: +phpunit initiative

updated IS with the findings

borisson_’s picture

+++ b/core/modules/system/tests/src/Functional/Theme/ThemeTest.php
@@ -94,7 +94,7 @@ public function testCSSOverride() {
+    $this->assertSession()->responseNotContains('js.module.css?', 'The theme\'s .info.yml file is able to override a module CSS file from being added to the page.');

Not sure why the question mark here is needed in the filename? Otherwise this is looking great.

Lendude’s picture

@borisson_ fair question :)

Stole that from \Drupal\system\Tests\Ajax\FrameworkTest (which led me to this test)

// We add a "?" to the assertion, because drupalSettings may include
// information about the file; we only really care about whether it appears
// in a LINK or STYLE tag, for which Drupal always adds a query string for
// cache control.

Added the comment and took out the assertion message which isn't a thing in that assertion

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

Awesome, thanks for making that clear.

  • lauriii committed 9c98f29 on 8.7.x
    Issue #3006625 by Lendude, borisson_: 'stylesheets-remove' test is...
lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed 9c98f29 and pushed to 8.7.x. Thanks!

Status: Fixed » Closed (fixed)

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

alexpott’s picture