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

Comments

martin107’s picture

martin107 commented
StatusFileSize
new 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 🇪🇺🌍
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 commented
StatusFileSize
new optGroup-2259301-3.patch3 KB
new interdiff-1-3.txt375 bytes

Fixed.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
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 🇨🇦
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.