See #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)

CommentFileSizeAuthor
#18 interdiff-2863372-11-18.txt971 bytesdagmar
#18 2863372-18.patch5.11 KBdagmar
#11 interdiff-2863372-7-11.txt1.17 KBdagmar
#11 2863372-11.patch4.43 KBdagmar
#7 interdiff-2863372-4-7.txt3.93 KBdagmar
#7 2863372-7.patch6.63 KBdagmar
#4 2863372-4.patch3.31 KBjofitz
#4 interdiff-3-4.txt1.48 KBjofitz
#3 2863372-3.patch3.47 KBdagmar

Comments

dagmar created an issue. See original summary.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Issue summary: View changes
dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Status: Active » Needs review
StatusFileSize
new 2863372-3.patch3.47 KB

I had some issues with the following commented lines:

  • $this->verifyBreadcrumbs();
  • $this->assertTrue($link, 'DBLog event was recorded: [delete user]');
jofitz’s picture

jofitz
he/him
commented
StatusFileSize
new interdiff-3-4.txt1.48 KB
new 2863372-4.patch3.31 KB
  • Resolved the issue with verifyBreadcrumbs().
  • $this->assertTrue($link, 'DBLog event was recorded: [delete user]'); is failing because $links is empty due to failing to find the expected link: 'Deleted user: %name %email.'. Presumably this is because the user is not being successfully deleted, but I can't work out why.
jofitz’s picture

jofitz
he/him
commented
Status: Needs review » Needs work

Expecting this patch to fail so setting back to Needs Work in advance - we need to investigate further into why $this->assertTrue($link, 'DBLog event was recorded: [delete user]'); is failing.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Looks like the test is failing because the user is never deleted. The batch to delete the user fails with a fatal error "no active batch". So it looks like our browser tests don't support batch API yet. Maybe we need something like #2855942: Create ::checkForMetaRefresh() on BrowserTestBase here?

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Status: Needs work » Needs review
StatusFileSize
new 2863372-7.patch6.63 KB
new interdiff-2863372-4-7.txt3.93 KB

Thanks @klausi. I couldn't found an example of how ::checkForMetaRefresh() should be used. Please let me know if this is the right approach.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Status: Needs review » Postponed

Postponed until we have batch api support #2855942: Create ::checkForMetaRefresh() on BrowserTestBase, #2862885: Batch: Convert system functional tests to phpunit

michielnugter’s picture

michielnugter commented
Issue tags: +phpunit initiative
dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Status: Postponed » Needs work

#2862885: Batch: Convert system functional tests to phpunit is in. Now we can continue with this.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Title: Convert web tests of dblog » Convert dblog web tests to BrowserTestBase
Status: Needs work » Needs review
StatusFileSize
new 2863372-11.patch4.43 KB
new interdiff-2863372-7-11.txt1.17 KB
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Nice, we have a minimal conversion now!

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented

DbLogTest::asText method is no longer required now. Do we have to delete it? Or is an API change?

naveenvalecha’s picture

naveenvalecha
He/Him
commented

#13
Tests are not the part of API, so no API issue See https://www.drupal.org/core/d8-bc-policy#tests Let's do it on this issue if it's not required :)

//Naveen

dawehner’s picture

dawehner
Primary language German
commented

DbLogTest::asText method is no longer required now. Do we have to delete it? Or is an API change?

As long its not on a base class, I think its fine to remove it.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

We should either deprecate it if it's on a base class, or remove if it's not, but the patch currently does neither.

dawehner’s picture

dawehner
Primary language German
commented

Far, I vote for removing given its not on the base class.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Status: Needs work » Needs review
StatusFileSize
new 2863372-18.patch5.11 KB
new interdiff-2863372-11-18.txt971 bytes

Removed asText.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Great!

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 798a30f and pushed to 8.4.x. Thanks!

  • catch committed 798a30f on 8.4.x 
    Issue #2863372 by dagmar, Jo Fitzgerald: Convert dblog web tests to...
naveenvalecha’s picture

naveenvalecha
He/Him
commented

DbLogResourceTest still needs conversion to phpunit functional test. Filed a follow-up for that. #2889882: Convert ResponseGeneratorTest, DbLogResourceTest, RestRegisterUserTest to BTB

//Naveen

Status: Fixed » Closed (fixed)

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