This issue has novice tasks. If you are an experienced core developer and have multiple commit mentions, please review novices' work on these tasks rather than doing them yourself. Feedback from experienced contributors is valued.

Problem/Motivation

The FormBuilderTest is testing both the FormBuilder class and the OptGroup class. Ideally unit tests should cover only one class.

Proposed resolution

Create a new unit test for OptGroup

Remaining tasks

User interface changes

None

API changes

None

CommentFileSizeAuthor
#3 interdiff-1-3.txt375 bytesmartin107
#3 optGroup-2259301-3.patch3 KBmartin107
#1 optGroup-2259301-1.patch3.03 KBmartin107
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

martin107’s picture

martin107 CreditAttribution: martin107 commented
FileSize
optGroup-2259301-1.patch3.03 KB

Thanks @alexpott - a useful learning experience.

FormBuilderTest and OptGroupTest pass on my machine..

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Active » Needs review 
+++ b/core/tests/Drupal/Tests/Core/Form/OptGroupTest.php
@@ -0,0 +1,61 @@
+
+}
\ No newline at end of file

Needs a need line at the EOF

martin107’s picture

martin107 CreditAttribution: martin107 commented
FileSize
optGroup-2259301-3.patch3 KB
interdiff-1-3.txt375 bytes

Fixed.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Needs review » Reviewed & tested by the community

Ran test locally

phpunit --filter OptGroup --tap
TAP version 13
ok 1 - Drupal\Tests\Core\Form\OptGroupTest::testFlattenOptions with data set #0 (array('foo'))
ok 2 - Drupal\Tests\Core\Form\OptGroupTest::testFlattenOptions with data set #1 (array(array('foo')))
ok 3 - Drupal\Tests\Core\Form\OptGroupTest::testFlattenOptions with data set #2 (array(stdClass))
ok 4 - Drupal\Tests\Core\Form\OptGroupTest::testFlattenOptions with data set #3 (array(stdClass))
1..4

Looks good to go. Thanks.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • Commit 445abea on 8.x by webchick: 
    Issue #2259301 by martin107 | alexpott: Move OptGroup tests out of...

Status: Fixed » Closed (fixed)

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