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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

idebr created an issue. See original summary.

idebr’s picture

Status: Active » Needs review
FileSize
2.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
FileSize
2.25 KB
3.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
FileSize
618 bytes
2.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.