Problem/Motivation

The Field Group module includes automated tests, but \Drupal\Tests\field_group\FunctionalJavascript\FieldGroupUiTest is currently failing. See https://www.drupal.org/node/948488/qa

Proposed resolution

Fix the failing \Drupal\Tests\field_group\FunctionalJavascript\FieldGroupUiTest

Remaining tasks

  1. Write a patch
  2. Review
  3. Commit

User interface changes

None.

API changes

None.

Data model changes

None.

Comments

idebr created an issue. See original summary.

idebr’s picture

Status: Active » Needs review
StatusFileSize
new2.13 KB

Fixed the inner working of \Drupal\Tests\field_group\FunctionalJavascript\FieldGroupUiTest::testCreateAndEdit(), but it seems the last test failure is legit.

Status: Needs review » Needs work

The last submitted patch, 2: 3072732-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

idebr’s picture

Status: Needs work » Needs review
StatusFileSize
new2.25 KB
new3.18 KB

The label element is being unset from format_settings since #2824350: missing UI for description text for field groups, so I removed these assertions from the test.

Status: Needs review » Needs work

The last submitted patch, 4: 3072732-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

idebr’s picture

Status: Needs work » Needs review
StatusFileSize
new618 bytes
new2.89 KB

Let's go with the smallest possible change and leave the duplicate label schema for a follow-up.

andypost’s picture

Looks good except last removed line in test, where formatter extended

idebr’s picture

+++ b/tests/src/FunctionalJavascript/FieldGroupUiTest.php
@@ -100,10 +104,7 @@ class FieldGroupUiTest extends WebDriverTestBase {
-    $this->assertSame('Test 1 - Update', $display->getThirdPartySetting('field_group', 'group_test_1')['format_settings']['label']);
...
-    $this->assertSame('Test 2 - Update', $display->getThirdPartySetting('field_group', 'group_test_2')['format_settings']['label']);

#7 Are you referring to these lines? The label property was removed in #2824350: missing UI for description text for field groups since its data was added twice in the configuration. This is what I referred to in #4:

field_group_group_save()

if (isset($display)) {
    // Remove label from the format_settings.
    unset($group->format_settings['label']);

    $data = (array) $group;
    unset($data['group_name'], $data['entity_type'], $data['bundle'], $data['mode'], $data['form'], $data['context']);
    $display->setThirdPartySetting('field_group', $group->group_name, $data);
    $display->save();
  }
andypost’s picture

Status: Needs review » Reviewed & tested by the community

Yes, missed to check that. Let's unblock testing!

chris matthews’s picture

Priority: Normal » Major

@maintainers, would it be possible to commit @idebr's patch in #6 so that we can, as @idebr stated in #2909960: Plan for Field Group 8.x-3.0, "go through the issue queue and help with issue triage and add test coverage where necessary."?

  • swentel committed 0c56486 on 8.x-3.x authored by idebr
    Issue #3072732 by idebr: Automated tests are failing on 8.x-3.x
    
swentel’s picture

Committed and pushed, thanks!

swentel’s picture

Status: Reviewed & tested by the community » Fixed
idebr’s picture

Cheers! I filed a followup to enable strict config schema checking at #3085225: Enable strict config schema checking in \Drupal\Tests\field_group\Functional\ManageDisplayTest

Status: Fixed » Closed (fixed)

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