Problem/Motivation

Parent ticket: #2616404: Create SimpleTest for basic creating, importing, exporting, etc.

Need test coverage for assignment plugins.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nedjo created an issue. See original summary.

nedjo’s picture

Status: Active » Needs review
FileSize
8.73 KB

Here are draft tests for the base, core, and dependency assignment plugins.

dawehner’s picture

In general the tests are looking great! Small tests, and limits in its scope.

  1. +++ b/tests/src/Kernel/FeaturesAssignTest.php
    @@ -0,0 +1,225 @@
    +  /**
    +   * @var \Drupal\features\FeaturesManagerInterface
    +   */
    +  protected $featuresManager;
    ...
    +  /**
    +   * @var \Drupal\features\FeaturesAssignerInterface
    +   */
    +  protected $assigner;
    ...
    +    $this->assigner = \Drupal::service('features_assigner');
    

    Given that features knows which code exists I'd have typehinted against concrete classes, which makes it a bit easier to follow.

  2. +++ b/tests/src/Kernel/FeaturesAssignTest.php
    @@ -0,0 +1,225 @@
    +    $this->addConfigurationItem('node.type.article', [], [
    

    yeah, for adding helper methods here.

  3. +++ b/tests/src/Kernel/FeaturesAssignTest.php
    @@ -0,0 +1,225 @@
    +    $this->assertEquals($expected_package_names, array_keys($packages), 'Expected packages not created.');  ¶
    ...
    +    $this->assertEquals($expected_config_items, $packages['article']['config'], 'Expected configuration items not present in article package.');  ¶
    

    There are a little bit of whitespace ... I'd also just not use messages anymore, its kinda pointless IMHO quite often.

  4. +++ b/tests/src/Kernel/FeaturesAssignTest.php
    @@ -0,0 +1,225 @@
    +    $this->assertTrue(in_array('field.storage.node.body', $packages['core']['config'], 'Expected configuration item not present in core package.'));  ¶
    +    $this->assertFalse(in_array('field.field.node.article.body', $packages['core']['config'], 'Unexpected configuration item present in core package.'));  ¶
    

    There is assertContains if you want to use that. Its a bit better to read at the end.

dawehner’s picture

Status: Needs review » Fixed

Fixed those nitpicks and committed to 8.x-3.x

Thank you!

  • dawehner committed 3c3adde on 8.x-3.x authored by nedjo
    Issue #2619650 by nedjo: Kernel test coverage for assignment plugins
    

Status: Fixed » Needs work

The last submitted patch, 2: assignment-tests-2619650-2.patch, failed testing.

nedjo’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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