Comments

dagmar created an issue. See original summary.

dagmar’s picture

Issue summary: View changes
dagmar’s picture

Status: Active » Needs review
FileSize
3.47 KB

I had some issues with the following commented lines:

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

  • 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.
Jo Fitzgerald’s picture

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

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

Status: Needs work » Needs review
FileSize
6.63 KB
3.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

Status: Needs review » Postponed
michielnugter’s picture

Issue tags: +phpunit initiative
dagmar’s picture

Status: Postponed » Needs work
dagmar’s picture

Title: Convert web tests of dblog » Convert dblog web tests to BrowserTestBase
Status: Needs work » Needs review
FileSize
4.43 KB
1.17 KB
dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Nice, we have a minimal conversion now!

dagmar’s picture

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

naveenvalecha’s picture

#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

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

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

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

dagmar’s picture

Status: Needs work » Needs review
FileSize
5.11 KB
971 bytes

Removed asText.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Great!

catch’s picture

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

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.