Comments

dstol created an issue. See original summary.

dstol’s picture

Status: Active » Needs review
StatusFileSize
new9.67 KB
dstol’s picture

  1. +++ b/tests/src/Unit/Plugin/DMU/Analyzer/FlagHookTest.php
    @@ -41,7 +41,7 @@ END;
    -    $this->assertCount(1, $issues[0]->getViolations());
    +    //$this->assertCount(1, $issues[0]->getViolations());
    

    .

  2. +++ b/tests/src/Unit/Plugin/DMU/Analyzer/TestsTest.php
    @@ -22,7 +22,6 @@ END;
         $indexer->build();
    -
         $this->container
    

    .

fix before commit

phenaproxima’s picture

Status: Needs review » Needs work

Apart from these nitpicks, looks great! You, sir, are my hero.

+++ b/src/Plugin/DMU/Analyzer/Tests.php
+    $total += $target->getIndexer('class')->getQuery()->condition('parent', '\DrupalWebTestCase');

This should be a countQuery(), and it should also check for classes which extend DrupalUnitTestCase (OR condition).

+++ b/src/Plugin/DMU/Indexer/Constants.php
+          // 'file' => $node->getSourcePosition()->getFilename(),
+          'file' => 'foo.module',

Let's delete the commented-out line and add a TODO here explaining that file name resolution is temporarily unavailable due to changes in Pharborist.

+++ b/src/Plugin/DMU/Indexer/FunctionCalls.php
+  public function build() {
+
+    /** @var \Symfony\Component\Finder\SplFileInfo $file */

Nit: extra line of white space.

+++ b/src/Plugin/DMU/Indexer/FunctionCalls.php
+        ->each(function(FunctionCallNode $function_call) use ($path) {
+          $this->add($function_call);
+        });

This could just be ->each([$this, 'add']), like in addFile().

+++ b/tests/src/Unit/Plugin/DMU/Analyzer/FlagHookTest.php
-    $this->assertCount(1, $issues[0]->getViolations());
+    //$this->assertCount(1, $issues[0]->getViolations());

Can you add a TODO comment above the disabled line, explaining what breaks?

+++ b/tests/src/Unit/Plugin/DMU/Indexer/IndexerTestBase.php
+    $this->assertInstanceOf($this->info['class']['expectType'][0], $collection);

For clarity's sake, can you change the variable name from $collection to $node (since not all indexers will return a collection)?

dineshw’s picture

Bingo, The patch just works fine!!!
@dstol @phenaproxima : You both saves my day :)

@dstol Thanks a ton, saved my day!

@phenaproxima : I'm just wondering should I go ahead and apply changes suggested by you in comment #4 , let me know if you want me to test it!

phenaproxima’s picture

@dineshw: The changes in #4 are just nitpicks. @dstol's patch fixes the failures, so the stuff in #4 is just stuff I'd like him to add before I commit this patch. They're not necessary to get things done. :)

dineshw’s picture

Sounds great!

dstol’s picture

Status: Needs work » Needs review
StatusFileSize
new11.52 KB
new6.57 KB

  • dstol committed e463829 on 8.x-1.x
    Issue #2554197 by dstol: Getting it working again
    
dstol’s picture

Status: Needs review » Fixed
phenaproxima’s picture

Thank you!

Status: Fixed » Closed (fixed)

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

dineshw’s picture

Thanks!