Contrib.social

[policy, no patch] Fix coding standards for PHPUnit tests

Open on Drupal.org →
Created on 29 April 2014, about 11 years ago
Updated 22 January 2023, over 2 years ago

Problem

  1. Our coding standards have been blindly taken over for PHPUnit tests, and are rigorously applied now.

  2. Excessive comments do not help in any way, they harm readability of otherwise self-documenting test code:

    /**
 * Tests PHP environment helper class.
 *
 * @group Drupal
 * @group Utility
 * @coversDefaultClass \Drupal\Component\Utility\Environment
 */
class EnvironmentTest extends UnitTestCase {

  /**
   * {@inheritdoc}
   */
  public static function getInfo() {
    return array(
      'name' => 'PHP environment utility helpers',
      'description' => '',
      'group' => 'Utility',
    );
  }

  /**
   * Tests \Drupal\Component\Utility\Environment::checkMemoryLimit().
   *
   * @param string $required
   *   The required memory argument for
   *   \Drupal\Component\Utility\Environment::checkMemoryLimit().
   * @param string $custom_memory_limit
   *   The custom memory limit argument for
   *   \Drupal\Component\Utility\Environment::checkMemoryLimit().
   * @param bool $expected
   *   The expected return value from
   *   \Drupal\Component\Utility\Environment::checkMemoryLimit().
   *
   * @dataProvider providerTestCheckMemoryLimit
   * @covers ::checkMemoryLimit
   */
  public function testCheckMemoryLimit($required, $custom_memory_limit, $expected) {
    $actual = Environment::checkMemoryLimit($required, $custom_memory_limit);
    $this->assertEquals($expected, $actual);
  }

  /**
   * Provides data for testCheckMemoryLimit().
   *
   * @return array
   *   An array of arrays, each containing the arguments for
   *   \Drupal\Component\Utility\Environment::checkMemoryLimit():
   *   required and memory_limit, and the expected return value.
   */
  public function providerTestCheckMemoryLimit() {
    $memory_limit = ini_get('memory_limit');
    $twice_avail_memory = ($memory_limit * 2) . 'MB';

    return array(
      // Minimal amount of memory should be available.
      array('30MB', NULL, TRUE),
      // Exceed a custom (unlimited) memory limit.
      array($twice_avail_memory, -1, TRUE),
      // Exceed a custom memory limit.
      array('30MB', '16MB', FALSE),
      // Available = required.
      array('30MB', '30MB', TRUE),
    );
  }

}

    ↓

    1. All of the extra phpDoc suggests that something special would be going, but this is just a regular test using a data provider.
    2. Insane amount of repetition of the namespace and class name of the class being tested.
    3. The connection between testCheckMemoryLimit() ↔ providerTestCheckMemoryLimit() is lost in a sea of useless comments.

  3. PHPUnit tests are most minimal units of test cases:

    1. Each unit test method maps to exactly one method on the test class:
      FooTest::testBar() → Foo::bar()

    2. Multiple unit test methods can map to the same test class method:
      FooTest::testBarInvalid() → Foo::bar()

    3. Data providers simplify argument permutation testing:
      FooTest::providerTestBar() → FooTest::testBar() → Foo::bar()

    4. PHPUnit's native @coversDefaultClass + @covers document the rest.

    None of this needs any additional documentation, because it's all regular PHPUnit test authoring practice.

  4. It is pointless and unhelpful to document anything else on top of self-descriptive + self-documenting unit test code.

Proposed solution

  1. Stop the coding standards nonsense for PHPUnit tests. Write self-descriptive + self-documenting unit tests instead.

    Identical test code as above, but readable this time:

    /**
 * Tests PHP environment helper class.
 *
 * @group Drupal
 * @group Utility
 * @coversDefaultClass \Drupal\Component\Utility\Environment
 */
class EnvironmentTest extends UnitTestCase {

  public static function getInfo() {
    return array(
      'name' => 'PHP environment utility helpers',
      'description' => '',
      'group' => 'Utility',
    );
  }

  /**
   * @covers ::checkMemoryLimit
   * @dataProvider providerTestCheckMemoryLimit
   */
  public function testCheckMemoryLimit($required, $custom_memory_limit, $expected) {
    $actual = Environment::checkMemoryLimit($required, $custom_memory_limit);
    $this->assertEquals($expected, $actual);
  }

  public function providerTestCheckMemoryLimit() {
    $memory_limit = ini_get('memory_limit');
    $twice_avail_memory = ($memory_limit * 2) . 'MB';

    return array(
      // Minimal amount of memory should be available.
      array('30MB', NULL, TRUE),
      // Exceed a custom (unlimited) memory limit.
      array($twice_avail_memory, -1, TRUE),
      // Exceed a custom memory limit.
      array('30MB', '16MB', FALSE),
      // Available = required.
      array('30MB', '30MB', TRUE),
    );
  }

}
📌 Task
Status

Active

Component

Coding Standards

Created by

🇩🇪Germany sun Karlsruhe

Live updates comments and jobs are added and updated live.
  • Coding standards

    It involves compliance with, or the content of coding standards. Requires broad community agreement.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024