Problem/Motivation

BrowserTestBase::getTextContent() wrongly returns the raw content. See the attached "test only" patch.

Proposed resolution

Fix it.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#10 interdiff-2773389-8-10.txt822 bytesmartin107
#10 2773389-10.patch1.33 KBmartin107
#8 interdiff-2-8.txt847 bytesmartin107
#8 2773389-8.patch1.33 KBmartin107
#2 2773389-2.patch1.35 KBclaudiu.cristea
gettextcontent-test-only.patch851 bytesclaudiu.cristea
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea at Webikon commented
Title: BrowserTestBase::getgetTextContent() wrongly returns the raw content » BrowserTestBase::getTextContent() wrongly returns the raw content
FileSize
2773389-2.patch1.35 KB

Patch.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea at Webikon commented
Related issues: +#2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea at Webikon commented
Issue summary: View changes
Issue tags: +Quick fix

The last submitted patch, gettextcontent-test-only.patch, failed testing.

Eric_A’s picture

Eric_A CreditAttribution: Eric_A commented
Version: 8.2.x-dev » 8.1.x-dev

This code is in the stable branch.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Needs review » Needs work

Only one minor thing:

+++ b/core/modules/simpletest/tests/src/Functional/BrowserTestBaseTest.php
@@ -33,6 +33,11 @@ public function testGoTo() {
+    $text = $this->getTextContent();
+    $this->assertTrue(strpos($text, 'Test page text.') !== FALSE);
+    $this->assertTrue(strpos($text, '</html>') === FALSE);

use ->assertContains() and ->assertNotContains() instead.

Otherwise makes sense!

martin107’s picture

martin107 CreditAttribution: martin107 commented
Status: Needs work » Needs review
FileSize
2773389-8.patch1.33 KB
interdiff-2-8.txt847 bytes

I think the idea behind this issue is good.

BUT From https://www.drupal.org/core/release-cycle-overview

Week of August 3, 2016. 8.2.0-beta1 released and 8.3.x-dev opened.

I suspect this is not a 8.1.x-dev issue anymore.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea at Webikon commented
Status: Needs review » Needs work 
+++ b/core/modules/simpletest/tests/src/Functional/BrowserTestBaseTest.php
@@ -33,6 +33,11 @@ public function testGoTo() {
+    $this->assertNotContains('Test page text.', $text);
+    $this->assertContains('</html>', $text);

I think you inverted them :)

martin107’s picture

martin107 CreditAttribution: martin107 commented
Status: Needs work » Needs review
FileSize
2773389-10.patch1.33 KB
interdiff-2773389-8-10.txt822 bytes

Oh crap ... I did ... up is down, down is up :(...

thanks

The last submitted patch, 8: 2773389-8.patch, failed testing.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Version: 8.1.x-dev » 8.3.x-dev
Status: Needs review » Reviewed & tested by the community

Looks good! This should go into 8.3.x first, then it is up to the committers to decide whether to cherry pick to the other branches or not.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Given that this is a pretty fundamental bug I think we should have this in all branches.

Committed and pushed fdb71536e1f592f52757e14dc7353dbbaaf249f1 to 8.3.x and ae332ad to 8.2.x and 4697702 to 8.1.x. Thanks!

  • alexpott committed fdb7153 on 8.3.x 
    Issue #2773389 by martin107, claudiu.cristea, klausi: BrowserTestBase::...

  • alexpott committed ae332ad on 8.2.x 
    Issue #2773389 by martin107, claudiu.cristea, klausi: BrowserTestBase::...

  • alexpott committed 4697702 on 8.1.x 
    Issue #2773389 by martin107, claudiu.cristea, klausi: BrowserTestBase::...

Status: Fixed » Closed (fixed)

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