BUG Fix non-test class manifest including sapphiretest / functionaltest

silverstripe · Jun 22, 2017 · 76f9594 · 76f9594
1 parent 9379834
commit 76f9594
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 22 deletions.
diff --git a/src/Core/Manifest/ClassManifest.php b/src/Core/Manifest/ClassManifest.php
@@ -368,7 +368,9 @@ public function regenerate($includeTests)
             'name_regex'    => '/^[^_].*\\.php$/',
             'ignore_files'  => array('index.php', 'main.php', 'cli-script.php'),
             'ignore_tests'  => !$includeTests,
-            'file_callback' => array($this, 'handleFile'),
+            'file_callback' => function ($basename, $pathname) use ($includeTests) {
+                $this->handleFile($basename, $pathname, $includeTests);
+            },
         ));
         $finder->find($this->base);
 
@@ -388,7 +390,7 @@ public function regenerate($includeTests)
         }
     }
 
-    public function handleFile($basename, $pathname)
+    public function handleFile($basename, $pathname, $includeTests)
     {
         $classes    = null;
         $interfaces = null;
@@ -401,6 +403,7 @@ public function handleFile($basename, $pathname)
         $key = preg_replace('/[^a-zA-Z0-9_]/', '_', $basename) . '_' . md5_file($pathname);
 
         // Attempt to load from cache
+        $changed = false;
         if ($this->cache
             && ($data = $this->cache->get($key))
             && $this->validateItemCache($data)
@@ -409,6 +412,7 @@ public function handleFile($basename, $pathname)
             $interfaces = $data['interfaces'];
             $traits = $data['traits'];
         } else {
+            $changed = true;
             // Build from php file parser
             $fileContents = ClassContentRemover::remove_class_content($pathname);
             try {
@@ -422,23 +426,16 @@ public function handleFile($basename, $pathname)
             $classes = $this->getVisitor()->getClasses();
             $interfaces = $this->getVisitor()->getInterfaces();
             $traits = $this->getVisitor()->getTraits();
-
-            // Save back to cache if configured
-            if ($this->cache) {
-                $cache = array(
-                    'classes' => $classes,
-                    'interfaces' => $interfaces,
-                    'traits' => $traits,
-                );
-                $this->cache->set($key, $cache);
-            }
         }
 
         // Merge this data into the global list
         foreach ($classes as $className => $classInfo) {
-            $extends = isset($classInfo['extends']) ? $classInfo['extends'] : null;
-            $implements = isset($classInfo['interfaces']) ? $classInfo['interfaces'] : null;
-
+            $extends = !empty($classInfo['extends'])
+                ? array_map('strtolower', $classInfo['extends'])
+                : [];
+            $implements = !empty($classInfo['interfaces'])
+                ? array_map('strtolower', $classInfo['interfaces'])
+                : [];
             $lowercaseName = strtolower($className);
             if (array_key_exists($lowercaseName, $this->classes)) {
                 throw new Exception(sprintf(
@@ -449,12 +446,20 @@ public function handleFile($basename, $pathname)
                 ));
             }
 
+            // Skip if implements TestOnly, but doesn't include tests
+            if (!$includeTests
+                && $implements
+                && in_array(strtolower(TestOnly::class), $implements)
+            ) {
+                $changed = true;
+                unset($classes[$className]);
+                continue;
+            }
+
             $this->classes[$lowercaseName] = $pathname;
 
             if ($extends) {
                 foreach ($extends as $ancestor) {
-                    $ancestor = strtolower($ancestor);
-
                     if (!isset($this->children[$ancestor])) {
                         $this->children[$ancestor] = array($className);
                     } else {
@@ -467,8 +472,6 @@ public function handleFile($basename, $pathname)
 
             if ($implements) {
                 foreach ($implements as $interface) {
-                    $interface = strtolower($interface);
-
                     if (!isset($this->implementors[$interface])) {
                         $this->implementors[$interface] = array($className);
                     } else {
@@ -484,6 +487,16 @@ public function handleFile($basename, $pathname)
         foreach ($traits as $traitName => $traitInfo) {
             $this->traits[strtolower($traitName)] = $pathname;
         }
+
+        // Save back to cache if configured
+        if ($changed && $this->cache) {
+            $cache = array(
+                'classes' => $classes,
+                'interfaces' => $interfaces,
+                'traits' => $traits,
+            );
+            $this->cache->set($key, $cache);
+        }
     }
 
     /**

diff --git a/src/Core/Manifest/ClassManifestVisitor.php b/src/Core/Manifest/ClassManifestVisitor.php
@@ -30,7 +30,7 @@ public function beforeTraverse(array $nodes)
     public function enterNode(Node $node)
     {
         if ($node instanceof Node\Stmt\Class_) {
-            $extends = '';
+            $extends = [];
             $interfaces = [];
 
             if ($node->extends) {

diff --git a/src/Dev/FunctionalTest.php b/src/Dev/FunctionalTest.php
@@ -34,7 +34,7 @@
  *  }
  * </code>
  */
-class FunctionalTest extends SapphireTest
+class FunctionalTest extends SapphireTest implements TestOnly
 {
     /**
      * Set this to true on your sub-class to disable the use of themes in this test.

diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php
@@ -45,7 +45,7 @@
  * This class should not be used anywhere outside of unit tests, as phpunit may not be installed
  * in production sites.
  */
-class SapphireTest extends PHPUnit_Framework_TestCase
+class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
 {
     /**
      * Path to fixture data for this test run.