Skip to content

Commit

Permalink
phpunit: Clean cache directory before running tests
Browse files Browse the repository at this point in the history
We've had a number of cases where we had confusing test failures
locally due to a bad cache entry from another commit we were working
on. Clean the cache before each test run to avoid this issue.

Since cleaning the cache directory is a bit more slow than simply
checking its existence, I've also gone ahead and switched the logic
from setUp (before each test) to setUpBeforeClass (only once).
For that to work, the variables have to be static instead.

While at it, address a few minor outdated style choices that were
inconsistent with other Wikimedia code:
* Rename the two variables and our test classes to camelcase.
* Remove "phpunit_" prefix.
* Rename our base class to end in "TestCase" and mention Less.

Change-Id: I980ca88619425848b40899267a217c7f017f9559
  • Loading branch information
Krinkle committed Jun 25, 2024
1 parent a731b9b commit 58c499c
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 28 deletions.
8 changes: 4 additions & 4 deletions test/phpunit/FixturesTest.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

class phpunit_FixturesTest extends phpunit_bootstrap {
class FixturesTest extends LessTestCase {
private const KNOWN_FAILURE = [
'lessjs-2.5.3' => [
// We moved this to Less.php parens.less test case because
Expand Down Expand Up @@ -95,7 +95,7 @@ public function testFixture( $cssFile, $lessFile, $options, $ifSetSkipTestMessag

// Check with cache
$optionsWithCache = $options + [
'cache_dir' => $this->cache_dir,
'cache_dir' => self::$cacheDir,
'functions' => [
'_color' => [ __CLASS__, 'FnColor' ],
'add' => [ __CLASS__, 'FnAdd' ],
Expand All @@ -105,7 +105,7 @@ public function testFixture( $cssFile, $lessFile, $options, $ifSetSkipTestMessag
$files = [ $lessFile => '' ];
try {
$cacheFile = Less_Cache::Regen( $files, $optionsWithCache );
$css = file_get_contents( $this->cache_dir . '/' . $cacheFile );
$css = file_get_contents( self::$cacheDir . '/' . $cacheFile );
} catch ( Less_Exception_Parser $e ) {
$css = $e->getMessage();
}
Expand All @@ -115,7 +115,7 @@ public function testFixture( $cssFile, $lessFile, $options, $ifSetSkipTestMessag
// Check using the cached data
try {
$cacheFile = Less_Cache::Get( $files, $optionsWithCache );
$css = file_get_contents( $this->cache_dir . '/' . $cacheFile );
$css = file_get_contents( self::$cacheDir . '/' . $cacheFile );
} catch ( Less_Exception_Parser $e ) {
$css = $e->getMessage();
}
Expand Down
2 changes: 1 addition & 1 deletion test/phpunit/FunctionTest.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

class phpunit_FunctionTest extends phpunit_bootstrap {
class FunctionTest extends LessTestCase {

public function testFunction() {
$lessFile = __DIR__ . '/data/f1.less';
Expand Down
8 changes: 4 additions & 4 deletions test/phpunit/MapTest.php
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<?php

class phpunit_MapTest extends phpunit_bootstrap {
class MapTest extends LessTestCase {

public function testMap() {
$lessFile = $this->fixtures_dir . '/bootstrap3-sourcemap/less/bootstrap.less';
$expectedFile = $this->fixtures_dir . '/bootstrap3-sourcemap/expected/bootstrap.map';
$mapDestination = $this->cache_dir . '/bootstrap.map';
$lessFile = self::$fixturesDir . '/bootstrap3-sourcemap/less/bootstrap.less';
$expectedFile = self::$fixturesDir . '/bootstrap3-sourcemap/expected/bootstrap.map';
$mapDestination = self::$cacheDir . '/bootstrap.map';

$options['sourceMap'] = true;
$options['sourceMapWriteTo'] = $mapDestination;
Expand Down
4 changes: 2 additions & 2 deletions test/phpunit/ParserTest.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

class phpunit_ParserTest extends phpunit_bootstrap {
class ParserTest extends LessTestCase {
public function testGetVariablesUncompiled() {
$lessCode = '
// Rule > Quoted
Expand Down Expand Up @@ -89,7 +89,7 @@ public function testGetVariables() {

public function testGetParsedFiles() {
$parser = new Less_Parser();
$baseDir = Less_Parser::WinPath( realpath( $this->fixtures_dir . '/less.php/less' ) );
$baseDir = Less_Parser::WinPath( realpath( self::$fixturesDir . '/less.php/less' ) );
$parser->parseFile( $baseDir . '/imports.less' );
$parser->getCss();

Expand Down
43 changes: 26 additions & 17 deletions test/phpunit/bootstrap.php
Original file line number Diff line number Diff line change
@@ -1,31 +1,40 @@
<?php

class phpunit_bootstrap extends PHPUnit\Framework\TestCase {
public $fixtures_dir;
public $cache_dir;
class LessTestCase extends PHPUnit\Framework\TestCase {
protected static $fixturesDir;
protected static $cacheDir;

public static function getFixtureDir() {
$rootDir = dirname( dirname( __DIR__ ) );
return $rootDir . '/test/Fixtures';
}

public function setUp(): void {
public static function setUpBeforeClass(): void {
$rootDir = dirname( dirname( __DIR__ ) );
require_once $rootDir . '/lib/Less/Autoloader.php';
Less_Autoloader::register();

$this->fixtures_dir = self::getFixtureDir();
$this->cache_dir = $rootDir . '/test/phpunit/_cache/';
$this->checkCacheDirectory();
$rootDir = dirname( dirname( __DIR__ ) );
self::$fixturesDir = $rootDir . '/test/Fixtures';

self::$cacheDir = $rootDir . '/test/phpunit/_cache/';
self::checkCacheDirectory();
// Cleaning the cache dir is only needed once per class
self::cleanCacheDirectory();
}

private function checkCacheDirectory() {
if ( !file_exists( $this->cache_dir ) && !mkdir( $this->cache_dir ) ) {
$this->fail( "Could not be create cache dir at " . $this->cache_dir );
private static function checkCacheDirectory() {
if ( !file_exists( self::$cacheDir ) && !mkdir( self::$cacheDir ) ) {
self::fail( "Could not create cache dir at " . self::$cacheDir );
}

if ( !is_writable( self::$cacheDir ) ) {
self::fail( "Cache dir not writable at " . self::$cacheDir );
}
}

if ( !is_writable( $this->cache_dir ) ) {
$this->fail( "Cache dir not writable at " . $this->cache_dir );
private static function cleanCacheDirectory() {
// Clean directory of cache from previous test runs
// on different code versions (incl e.g. any bugs they might contain).
foreach ( scandir( self::$cacheDir ) as $entry ) {
if ( $entry !== '.' && $entry !== '..' ) {
unlink( self::$cacheDir . '/' . $entry );
}
}
}
}

0 comments on commit 58c499c

Please sign in to comment.