Convert system functional tests to phpunit

Scope:
./System/AccessDeniedTest.php
./System/AdminTest.php
./System/CronRunTest.php
./System/DefaultMobileMetaTagsTest.php
./System/ErrorHandlerTest.php - see #2984072: System: Convert ErrorHandlerTest to phpunit
./System/FloodTest.php
./System/FrontPageTest.php
./System/HtaccessTest.php
./System/PageNotFoundTest.php
./System/PageTitleTest.php
./System/ResponseGeneratorTest.php - see #2927766: Update ResponseGeneratorTest to use the BrowserTestBase base class instead of the deprecated RESTTestBase
./System/ShutdownFunctionsTest.php
./System/SiteMaintenanceTest.php
./System/SystemConfigFormTestBase.php - see #2941494: Deprecate SystemConfigFormTestBase and create kernel test version
./System/ThemeTest.php
./System/TokenReplaceWebTest.php
./System/UncaughtExceptionTest.php - see #2863262: Bootstrap: Convert system functional tests to phpunit

CommentFileSizeAuthor
#12 interdiff-2982150-9-12.txt1.12 KBApacheEx
#12 2982150-12.patch25.23 KBApacheEx
#9 interdiff-2982150-7-9.txt405 bytesApacheEx
#9 2982150-9.patch25.08 KBApacheEx
#7 interdiff-2982150-4-7.txt1.61 KBApacheEx
#7 2982150-7.patch25.04 KBApacheEx
#4 2982150-4.patch24.14 KBApacheEx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Lendude created an issue. See original summary.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
Issue tags: +phpunit initiative
Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
Parent issue: » #2862508: Convert system functional tests to phpunit
ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Issue summary: View changes
Status: Active » Needs review
FileSize
2982150-4.patch24.14 KB

First try without ErrorHandlerTest and UncaughtExceptionTest which have some methods from simpletest module.

  • deleteAssert
  • assertErrorLogged
  • assertNoErrorsLogged

all these methods (more/less) are working with assertions in simpletest table - need to think how to deal with it, what's about followup?

p.s. I'm trying to do as minimal as possible changes to make tests passed.

Status: Needs review » Needs work

The last submitted patch, 4: 2982150-4.patch, failed testing. View results

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
Issue summary: View changes

@ApacheEx thanks!

UncaughtExceptionTest is part of another conversion #2863262: Bootstrap: Convert system functional tests to phpunit, so +1 to leaving that out here (it's also very hard :)

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Status: Needs work » Needs review
FileSize
2982150-7.patch25.04 KB
interdiff-2982150-4-7.txt1.61 KB

Here is the second try :)
Still without ErrorHandlerTest. But let's check how it's going with other tests.

Status: Needs review » Needs work

The last submitted patch, 7: 2982150-7.patch, failed testing. View results

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Status: Needs work » Needs review
FileSize
2982150-9.patch25.08 KB
interdiff-2982150-7-9.txt405 bytes

now it should pass (yn)

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +DevDaysLisbon

@ApacheEx very nice minimal conversion. Thank you! I went through it and everything looks good to me.

+++ b/core/modules/system/tests/src/Kernel/System/FloodTest.php
@@ -1,30 +1,22 @@
-class FloodTest extends WebTestBase {
+class FloodTest extends KernelTestBase {

awesome! great conversion to a KernelTest, gets rid of the $request injection nicely.

So this just leaves ErrorHandlerTest, which probably needs a follow up because that isn't straight forward, and UncaughtExceptionTest which has a follow up.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/system/tests/src/Functional/System/DefaultMobileMetaTagsTest.php
@@ -40,6 +41,8 @@ public function testDefaultMetaTagsExist() {
+    $this->resetAll();

Let's add an install hook for the module that does this:

function system_module_test_install() {
  Drupal\Core\Cache\Cache::invalidateTags(['rendered']);
}

This is related to #2783791: Module install doesn't invalidate render cache

ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Internetdevels, Drupal Ukraine Community commented
Status: Needs work » Needs review
FileSize
2982150-12.patch25.23 KB
interdiff-2982150-9-12.txt1.12 KB

Thanks for review and feedback, all make sense for me.
Here is updated patch.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
Status: Needs review » Reviewed & tested by the community

@ApacheEx, thanks for that! Feedback has been addressed, back to RTBC.

alexpott’s picture

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

Committed 013d70a and pushed to 8.6.x. Thanks!

  • alexpott committed 013d70a on 8.6.x 
    Issue #2982150 by ApacheEx, Lendude, alexpott: System: Convert system...
ApacheEx’s picture

ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commented
Issue summary: View changes

Thanks a lot. I've added a followup for ErrorHandlerTest.

Status: Fixed » Closed (fixed)

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