ResourcesTest: Detect missing files in url(...) expressions
The way this is implemented is really dirty... but it found us a few pre-existing bugs already (T111518, T111519, T111771). I think it might be worth it. * CSSMin: Add new method getAllLocalFileReferences() which skips the file_exists() check. * ResourceLoaderModule: Make use of it to track missing files too. * ResourcesTest: Verify that the missing files are missing. Change-Id: I5a3cdeb7d53485f161ccf8133e76850cdf5b4579
This commit is contained in:
parent
3a3c3e73e7
commit
8f5cd11d82
3 changed files with 60 additions and 16 deletions
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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
|
||||
);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in a new issue