Updated: Comment #3

Problem/Motivation

\Drupal\Core\ModuleHandler::parseDependency() has no unit tests.

Proposed resolution

Provide unit tests.

Remaining tasks

  1. Write initial patch
  2. Fix newline as requested in #2.2:
    • Apply the patch
    • Open core/tests/Drupal/Tests/Core/Extension/ModuleHandlerUnitTest
    • Find the spot that is referenced in #2.2
    • Add an empty newline between the two brackets.
    • That's it!

    #2.3 does not have to fixed but could be, for extra credit. :-) In that case simply document $dependency and $expected in the usual way. Documentation suggestion: $dependency "A dependency string to be parsed by ModuleHandler::parseDependency()." $expected "The parsed dependency array ModuleHandler::parseDependency" is expected to return."

    3. Original report by @tstoeckler

    Note: Either I'm on crack or there's no "module system" component, which is why I'm using "base system".

    See title.

CommentFileSizeAuthor
#1 2090939-module-handler-parse-dependency-unit-test.patch4 KBtstoeckler

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Active » Needs review
StatusFileSize
new 2090939-module-handler-parse-dependency-unit-test.patch4 KB

Here we go.

dawehner’s picture

dawehner
Primary language German
commented 
  1. +++ b/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerUnitTest.php
@@ -29,14 +29,104 @@ public static function getInfo() {
+   *
+   * @dataProvider providerTestParseDependency
+   */
+  public function testParseDependency($dependency, $expected) {
...
+  /**
+   * Data provider for testParseDependency().
+   */
+  public function providerTestParseDependency() {
+    return array(

    We could also add documentation for all that stuff here but yeah ... this are "just" tests. 

  2. +++ b/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerUnitTest.php
@@ -29,14 +29,104 @@ public static function getInfo() {
+  }
 }

    Just in case you have time: add a new empty line on there.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs review » Needs work
Issue tags: +Novice

You mean for the $dependency $expected stuff? I would personally find that silly and also I'm not aware of any standards so far, but OTOH it wouldn't bother me much. Have to re-roll for 2.2 anyway.

Marking Novice to maybe pick up on the sprint on friday. Will update the issue summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Issue summary: View changes

Updated issue summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Issue summary: View changes

Updated issue summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Assigned: tstoeckler » Unassigned
Issue summary: View changes

Status: Needs work » Needs review

dawehner queued 1: 2090939-module-handler-parse-dependency-unit-test.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2090939-module-handler-parse-dependency-unit-test.patch, failed testing.

Abhishek Verma’s picture

Abhishek Verma commented
Status: Needs work » Needs review

We research on this issues.
We found that file name has been changed from
a/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerUnitTest.php TO
a/core/tests/Drupal/Tests/Core/Extension/ModuleHandlerTest.php
Therefore issues can be closed.

zealfire’s picture

zealfire commented
Status: Needs review » Needs work

@Abhishek i too had the question about existence of ModuleHandlerUnitTest.php so i asked it on irc and came to know that it has been changed to ModuleHandlerTest.php, so we need to write test for the same.You can further clarify your doubts if any on irc.
Thanks.

dnmurray’s picture

dnmurray commented

I was going to re-roll this patch, but it looks like ModuleHandlerTest.php already has the changes (or similar functionality), so this can probably be closed.

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +rc eligible

Just tagging it as rc eligible, as its test changes only

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
Status: Needs work » Closed (works as designed)

The test is already there in ModuleHandlerTest.php

  /**
   * @dataProvider dependencyProvider
   * @covers ::parseDependency
   */
  public function testDependencyParsing($dependency, $expected) {
    $version = ModuleHandler::parseDependency($dependency);
    $this->assertEquals($expected, $version);
  }

  /**
   * Provider for testing dependency parsing.
   */
  public function dependencyProvider() {
    return array(
      array('system', array('name' => 'system')),
      array('taxonomy', array('name' => 'taxonomy')),
      array('views', array('name' => 'views')),
      array('views_ui(8.x-1.0)', array('name' => 'views_ui', 'original_version' => ' (8.x-1.0)', 'versions' => array(array('op' => '=', 'version' => '1.0')))),
      // Not supported?.
      // array('views_ui(8.x-1.1-beta)', array('name' => 'views_ui', 'original_version' => ' (8.x-1.1-beta)', 'versions' => array(array('op' => '=', 'version' => '1.1-beta')))),
      array('views_ui(8.x-1.1-alpha12)', array('name' => 'views_ui', 'original_version' => ' (8.x-1.1-alpha12)', 'versions' => array(array('op' => '=', 'version' => '1.1-alpha12')))),
      array('views_ui(8.x-1.1-beta8)', array('name' => 'views_ui', 'original_version' => ' (8.x-1.1-beta8)', 'versions' => array(array('op' => '=', 'version' => '1.1-beta8')))),
      array('views_ui(8.x-1.1-rc11)', array('name' => 'views_ui', 'original_version' => ' (8.x-1.1-rc11)', 'versions' => array(array('op' => '=', 'version' => '1.1-rc11')))),
      array('views_ui(8.x-1.12)', array('name' => 'views_ui', 'original_version' => ' (8.x-1.12)', 'versions' => array(array('op' => '=', 'version' => '1.12')))),
      array('views_ui(8.x-1.x)', array('name' => 'views_ui', 'original_version' => ' (8.x-1.x)', 'versions' => array(array('op' => '<', 'version' => '2.x'), array('op' => '>=', 'version' => '1.x')))),
      array('views_ui( <= 8.x-1.x)', array('name' => 'views_ui', 'original_version' => ' ( <= 8.x-1.x)', 'versions' => array(array('op' => '<=', 'version' => '2.x')))),
      array('views_ui(<= 8.x-1.x)', array('name' => 'views_ui', 'original_version' => ' (<= 8.x-1.x)', 'versions' => array(array('op' => '<=', 'version' => '2.x')))),
      array('views_ui( <=8.x-1.x)', array('name' => 'views_ui', 'original_version' => ' ( <=8.x-1.x)', 'versions' => array(array('op' => '<=', 'version' => '2.x')))),
      array('views_ui(>8.x-1.x)', array('name' => 'views_ui', 'original_version' => ' (>8.x-1.x)', 'versions' => array(array('op' => '>', 'version' => '2.x')))),
      array('drupal:views_ui(>8.x-1.x)', array('project' => 'drupal', 'name' => 'views_ui', 'original_version' => ' (>8.x-1.x)', 'versions' => array(array('op' => '>', 'version' => '2.x')))),
    );
  }