Problem/Motivation

The \Drupal\system\Tests\System\SystemConfigFormTestBase abstract class cannot simply be converted to phpunit since it is used by contrib and custom code. Instead, it can be copied over to both a kernel version (as originally intended, it works just fine as a kernel test), and a functional version for the FormObjectTest.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#7 2941494-07.patch5.01 KBjhedstrom
#7 interdiff-2941494-05-07.txt1.02 KBjhedstrom
#5 2941494-05.patch4.63 KBjhedstrom
#5 interdiff-2941494-03-05.txt710 bytesjhedstrom
#3 2941494-03.patch4.68 KBjhedstrom
#3 interdiff-2941494-02-03.txt4.28 KBjhedstrom
#2 2941494-02.patch4.15 KBjhedstrom
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhedstrom created an issue. See original summary.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented
Status: Active » Needs review
FileSize
2941494-02.patch4.15 KB
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented
Title: Deprecate SystemConfigFormTestBase and create kernel and functional test versions » Deprecate SystemConfigFormTestBase and create kernel test version
FileSize
interdiff-2941494-02-03.txt4.28 KB
2941494-03.patch4.68 KB

Actually, I think it makes more sense to split the web browser bits out of FormObjectTest, as they aren't utilizing the config test base at all. That way we only need to add a kernel test version instead of both.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Issue tags: +Needs change record

It is quite nice that you can use a kernel test here! I like the fact that this makes it quite easy to write tests for these forms.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented
Issue tags: -Needs change record
FileSize
interdiff-2941494-03-05.txt710 bytes
2941494-05.patch4.63 KB

I've added a change record. This patch fixes the @deprecated text which applied to the approach in #2 rather than the approach in #3.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at Dx Experts commented
Status: Needs review » Needs work
Issue tags: +phpunit initiative

@jhedstrom nice! kerneltest++

couple of things I still see:

  1. +++ b/core/modules/system/src/Tests/System/SystemConfigFormTestBase.php
@@ -10,6 +10,9 @@
+ * @deprecated in Drupal 8.6.x and will be removed before Drupal 9.0.0. Use
+ *   either \Drupal\KernelTests\ConfigFormTestBase instead.

    'or \Drupal\Tests\system\Kernel\Form\FormObjectTest'.
    Also needs a 'see' to the CR and a @trigger_error under the namespace (should be safe since it's not in use in core anymore now) 

  2. +++ b/core/tests/Drupal/KernelTests/ConfigFormTestBase.php
@@ -1,9 +1,8 @@
  * Full generic test suite for any form that data with the configuration system.

    This comment could use an update since it's not really an english sentence. I know it would still exist in SystemConfigFormTestBase but lets at least clean it up in the new class.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented
Status: Needs work » Needs review
FileSize
interdiff-2941494-05-07.txt1.02 KB
2941494-07.patch5.01 KB

This should address #6.

dawehner’s picture

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

Making it easier for people to write simple tests seems like a good idea.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Version: 8.6.x-dev » 8.5.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed 40d534a and pushed to 8.6.x. Thanks!

This can be backported to 8.5.x without the deprecation of the webtestbase class - to keep 8.5.x and 8.6.x testbases in sync.

diff --git a/core/modules/system/tests/src/Functional/Form/FormObjectTest.php b/core/modules/system/tests/src/Functional/Form/FormObjectTest.php
index dde04b59ff..e2c00f5d17 100644
--- a/core/modules/system/tests/src/Functional/Form/FormObjectTest.php
+++ b/core/modules/system/tests/src/Functional/Form/FormObjectTest.php
@@ -2,8 +2,6 @@
 
 namespace Drupal\Tests\system\Functional\Form;
 
-use Drupal\FunctionalTests\ConfigFormTestBase;
-use Drupal\form_test\FormTestObject;
 use Drupal\Tests\BrowserTestBase;
 
 /**

Fixed unused uses on commit.

  • alexpott committed 40d534a on 8.6.x 
    Issue #2941494 by jhedstrom, Lendude: Deprecate SystemConfigFormTestBase...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Patch (to be ported) » Fixed

Did the backport myself by just not including the changes to core/modules/system/src/Tests/System/SystemConfigFormTestBase.php

Committed a259c82 and pushed to 8.5.x. Thanks!

  • alexpott committed a259c82 on 8.5.x 
    Issue #2941494 by jhedstrom, Lendude, alexpott: Deprecate...

Status: Fixed » Closed (fixed)

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