Merge "ResourcesTest: Detect missing files in url(...) expressions"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 17 Sep 2015 19:46:11 +0000 (19:46 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 17 Sep 2015 19:46:11 +0000 (19:46 +0000)
includes/libs/CSSMin.php
includes/resourceloader/ResourceLoaderFileModule.php
tests/phpunit/structure/ResourcesTest.php

index 2c90431..2648921 100644 (file)
@@ -60,13 +60,15 @@ class CSSMin {
        /* Static Methods */
 
        /**
-        * Gets a list of local file paths which are referenced in a CSS style sheet
+        * Gets a list of local file paths which are referenced in a CSS style sheet.
         *
-        * This function will always return an empty array if the second parameter is not given or null
-        * for backwards-compatibility.
+        * If you wish non-existent files to be listed too, use getAllLocalFileReferences().
         *
-        * @param string $source CSS data to remap
-        * @param string $path File path where the source was read from (optional)
+        * For backwards-compatibility, if the second parameter is not given or null,
+        * this function will return an empty array instead of erroring out.
+        *
+        * @param string $source CSS stylesheet source to process
+        * @param string $path File path where the source was read from
         * @return array List of local file references
         */
        public static function getLocalFileReferences( $source, $path = null ) {
@@ -74,6 +76,25 @@ class CSSMin {
                        return array();
                }
 
+               $files = self::getAllLocalFileReferences( $source, $path );
+
+               // Skip non-existent files
+               $files = array_filter( $files, function ( $file ) {
+                       return file_exists( $file );
+               } );
+
+               return $files;
+       }
+
+       /**
+        * Gets a list of local file paths which are referenced in a CSS style sheet, including
+        * non-existent files.
+        *
+        * @param string $source CSS stylesheet source to process
+        * @param string $path File path where the source wa
+        * @return array List of local file references
+        */
+       public static function getAllLocalFileReferences( $source, $path ) {
                $path = rtrim( $path, '/' ) . '/';
                $files = array();
 
@@ -87,13 +108,7 @@ class CSSMin {
                                        break;
                                }
 
-                               $file = $path . $url;
-                               // Skip non-existent files
-                               if ( file_exists( $file ) ) {
-                                       break;
-                               }
-
-                               $files[] = $file;
+                               $files[] = $path . $url;
                        }
                }
                return $files;
index 4a4a828..112ebc0 100644 (file)
@@ -152,6 +152,12 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
         */
        protected $localFileRefs = array();
 
+       /**
+        * @var array Place where readStyleFile() tracks file dependencies for non-existent files.
+        * Used in tests to detect missing dependencies.
+        */
+       protected $missingLocalFileRefs = array();
+
        /* Methods */
 
        /**
@@ -917,10 +923,14 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                $localDir = dirname( $localPath );
                $remoteDir = dirname( $remotePath );
                // Get and register local file references
-               $this->localFileRefs = array_merge(
-                       $this->localFileRefs,
-                       CSSMin::getLocalFileReferences( $style, $localDir )
-               );
+               $localFileRefs = CSSMin::getAllLocalFileReferences( $style, $localDir );
+               foreach ( $localFileRefs as $file ) {
+                       if ( file_exists( $file ) ) {
+                               $this->localFileRefs[] = $file;
+                       } else {
+                               $this->missingLocalFileRefs[] = $file;
+                       }
+               }
                return CSSMin::remap(
                        $style, $localDir, $remoteDir, true
                );
index 2c92e30..69f4525 100644 (file)
@@ -276,6 +276,25 @@ class ResourcesTest extends MediaWikiTestCase {
                                        ( $file instanceof ResourceLoaderFilePath ? $file->getPath() : $file ),
                                );
                        }
+
+                       // To populate missingLocalFileRefs. Not sure how sane this is inside this test...
+                       $module->readStyleFiles(
+                               $module->getStyleFiles( $data['context'] ),
+                               $module->getFlip( $data['context'] ),
+                               $data['context']
+                       );
+
+                       $property = $reflectedModule->getProperty( 'missingLocalFileRefs' );
+                       $property->setAccessible( true );
+                       $missingLocalFileRefs = $property->getValue( $module );
+
+                       foreach ( $missingLocalFileRefs as $file ) {
+                               $cases[] = array(
+                                       $file,
+                                       $moduleName,
+                                       $file,
+                               );
+                       }
                }
 
                return $cases;