sminnee /
silverstripe-framework
Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like TestRunner often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use TestRunner, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 22 | class TestRunner extends Controller { |
||
| 23 | |||
| 24 | /** @ignore */ |
||
| 25 | private static $default_reporter; |
||
| 26 | |||
| 27 | private static $url_handlers = array( |
||
|
Comprehensibility
introduced
by
|
|||
| 28 | '' => 'browse', |
||
| 29 | 'coverage/module/$ModuleName' => 'coverageModule', |
||
| 30 | 'coverage/suite/$SuiteName!' => 'coverageSuite', |
||
| 31 | 'coverage/$TestCase!' => 'coverageOnly', |
||
| 32 | 'coverage' => 'coverageAll', |
||
| 33 | 'cleanupdb' => 'cleanupdb', |
||
| 34 | 'module/$ModuleName' => 'module', |
||
| 35 | 'suite/$SuiteName!' => 'suite', |
||
| 36 | 'all' => 'all', |
||
| 37 | 'build' => 'build', |
||
| 38 | '$TestCase' => 'only' |
||
| 39 | ); |
||
| 40 | |||
| 41 | private static $allowed_actions = array( |
||
| 42 | 'index', |
||
| 43 | 'browse', |
||
| 44 | 'coverage', |
||
| 45 | 'coverageAll', |
||
| 46 | 'coverageModule', |
||
| 47 | 'coverageSuite', |
||
| 48 | 'coverageOnly', |
||
| 49 | 'cleanupdb', |
||
| 50 | 'module', |
||
| 51 | 'suite', |
||
| 52 | 'all', |
||
| 53 | 'build', |
||
| 54 | 'only' |
||
| 55 | ); |
||
| 56 | |||
| 57 | /** |
||
| 58 | * @var Array Blacklist certain directories for the coverage report. |
||
| 59 | * Filepaths are relative to the webroot, without leading slash. |
||
| 60 | * |
||
| 61 | * @see http://www.phpunit.de/manual/current/en/appendixes.configuration.html |
||
| 62 | * #appendixes.configuration.blacklist-whitelist |
||
| 63 | */ |
||
| 64 | static $coverage_filter_dirs = array( |
||
| 65 | '*/thirdparty', |
||
| 66 | '*/tests', |
||
| 67 | '*/lang', |
||
| 68 | ); |
||
| 69 | |||
| 70 | /** |
||
| 71 | * Override the default reporter with a custom configured subclass. |
||
| 72 | * |
||
| 73 | * @param string $reporter |
||
| 74 | */ |
||
| 75 | public static function set_reporter($reporter) { |
||
| 79 | |||
| 80 | /** |
||
| 81 | * Pushes a class and template manifest instance that include tests onto the |
||
| 82 | * top of the loader stacks. |
||
| 83 | */ |
||
| 84 | public static function use_test_manifest() { |
||
| 85 | $flush = false; |
||
| 86 | if(isset($_GET['flush']) && ($_GET['flush'] === '1' || $_GET['flush'] == 'all')) { |
||
| 87 | $flush = true; |
||
| 88 | } |
||
| 89 | |||
| 90 | $classManifest = new SS_ClassManifest( |
||
| 91 | BASE_PATH, true, $flush |
||
| 92 | ); |
||
| 93 | |||
| 94 | SS_ClassLoader::instance()->pushManifest($classManifest, false); |
||
| 95 | SapphireTest::set_test_class_manifest($classManifest); |
||
| 96 | |||
| 97 | SS_TemplateLoader::instance()->pushManifest(new SS_TemplateManifest( |
||
| 98 | BASE_PATH, project(), true, $flush |
||
| 99 | )); |
||
| 100 | |||
| 101 | Config::inst()->pushConfigStaticManifest(new SS_ConfigStaticManifest( |
||
| 102 | BASE_PATH, true, $flush |
||
| 103 | )); |
||
| 104 | |||
| 105 | // Invalidate classname spec since the test manifest will now pull out new subclasses for each internal class |
||
| 106 | // (e.g. Member will now have various subclasses of DataObjects that implement TestOnly) |
||
| 107 | DataObject::reset(); |
||
| 108 | } |
||
| 109 | |||
| 110 | public function init() { |
||
| 111 | parent::init(); |
||
| 112 | |||
| 113 | $canAccess = (Director::isDev() || Director::is_cli() || Permission::check("ADMIN")); |
||
| 114 | if(!$canAccess) return Security::permissionFailure($this); |
||
| 115 | |||
| 116 | if (!self::$default_reporter) self::set_reporter(Director::is_cli() ? 'CliDebugView' : 'DebugView'); |
||
| 117 | |||
| 118 | if(!PhpUnitWrapper::has_php_unit()) { |
||
| 119 | die("Please install PHPUnit using Composer"); |
||
| 120 | } |
||
| 121 | } |
||
| 122 | |||
| 123 | public function Link() { |
||
| 126 | |||
| 127 | /** |
||
| 128 | * Run test classes that should be run with every commit. |
||
| 129 | * Currently excludes PhpSyntaxTest |
||
| 130 | */ |
||
| 131 | View Code Duplication | public function all($request, $coverage = false) { |
|
| 132 | self::use_test_manifest(); |
||
| 133 | $tests = ClassInfo::subclassesFor('SapphireTest'); |
||
| 134 | array_shift($tests); |
||
| 135 | unset($tests['FunctionalTest']); |
||
| 136 | |||
| 137 | // Remove tests that don't need to be executed every time |
||
| 138 | unset($tests['PhpSyntaxTest']); |
||
| 139 | |||
| 140 | foreach($tests as $class => $v) { |
||
| 141 | $reflection = new ReflectionClass($class); |
||
| 142 | if(!$reflection->isInstantiable()) unset($tests[$class]); |
||
| 143 | } |
||
| 144 | |||
| 145 | $this->runTests($tests, $coverage); |
||
| 146 | } |
||
| 147 | |||
| 148 | /** |
||
| 149 | * Run test classes that should be run before build - i.e., everything possible. |
||
| 150 | */ |
||
| 151 | View Code Duplication | public function build() { |
|
| 152 | self::use_test_manifest(); |
||
| 153 | $tests = ClassInfo::subclassesFor('SapphireTest'); |
||
| 154 | array_shift($tests); |
||
| 155 | unset($tests['FunctionalTest']); |
||
| 156 | foreach($tests as $class => $v) { |
||
| 157 | $reflection = new ReflectionClass($class); |
||
| 158 | if(!$reflection->isInstantiable()) unset($tests[$class]); |
||
| 159 | } |
||
| 160 | |||
| 161 | $this->runTests($tests); |
||
| 162 | } |
||
| 163 | |||
| 164 | /** |
||
| 165 | * Browse all enabled test cases in the environment |
||
| 166 | */ |
||
| 167 | public function browse() { |
||
| 168 | self::use_test_manifest(); |
||
| 169 | self::$default_reporter->writeHeader(); |
||
| 170 | self::$default_reporter->writeInfo('Available Tests', false); |
||
| 171 | if(Director::is_cli()) { |
||
| 172 | $tests = ClassInfo::subclassesFor('SapphireTest'); |
||
| 173 | $relativeLink = Director::makeRelative($this->Link()); |
||
| 174 | echo "sake {$relativeLink}all: Run all " . count($tests) . " tests\n"; |
||
| 175 | echo "sake {$relativeLink}coverage: Runs all tests and make test coverage report\n"; |
||
| 176 | echo "sake {$relativeLink}module/<modulename>: Runs all tests in a module folder\n"; |
||
| 177 | foreach ($tests as $test) { |
||
| 178 | echo "sake {$relativeLink}$test: Run $test\n"; |
||
| 179 | } |
||
| 180 | } else { |
||
| 181 | echo '<div class="trace">'; |
||
| 182 | $tests = ClassInfo::subclassesFor('SapphireTest'); |
||
| 183 | asort($tests); |
||
| 184 | echo "<h3><a href=\"" . $this->Link() . "all\">Run all " . count($tests) . " tests</a></h3>"; |
||
| 185 | echo "<h3><a href=\"" . $this->Link() . "coverage\">Runs all tests and make test coverage report</a></h3>"; |
||
| 186 | echo "<hr />"; |
||
| 187 | foreach ($tests as $test) { |
||
| 188 | echo "<h3><a href=\"" . $this->Link() . "$test\">Run $test</a></h3>"; |
||
| 189 | } |
||
| 190 | echo '</div>'; |
||
| 191 | } |
||
| 192 | |||
| 193 | self::$default_reporter->writeFooter(); |
||
| 194 | } |
||
| 195 | |||
| 196 | /** |
||
| 197 | * Run a coverage test across all modules |
||
| 198 | */ |
||
| 199 | public function coverageAll($request) { |
||
| 203 | |||
| 204 | /** |
||
| 205 | * Run only a single coverage test class or a comma-separated list of tests |
||
| 206 | */ |
||
| 207 | public function coverageOnly($request) { |
||
| 210 | |||
| 211 | /** |
||
| 212 | * Run coverage tests for one or more "modules". |
||
| 213 | * A module is generally a toplevel folder, e.g. "mysite" or "framework". |
||
| 214 | */ |
||
| 215 | public function coverageModule($request) { |
||
| 218 | |||
| 219 | public function cleanupdb() { |
||
| 222 | |||
| 223 | /** |
||
| 224 | * Run only a single test class or a comma-separated list of tests |
||
| 225 | */ |
||
| 226 | public function only($request, $coverage = false) { |
||
| 227 | self::use_test_manifest(); |
||
| 228 | if($request->param('TestCase') == 'all') { |
||
| 229 | $this->all(); |
||
| 230 | } else { |
||
| 231 | $classNames = explode(',', $request->param('TestCase')); |
||
| 232 | foreach($classNames as $className) { |
||
| 233 | if(!class_exists($className) || !is_subclass_of($className, 'SapphireTest')) { |
||
| 234 | user_error("TestRunner::only(): Invalid TestCase '$className', cannot find matching class", |
||
| 235 | E_USER_ERROR); |
||
| 236 | } |
||
| 237 | } |
||
| 238 | |||
| 239 | $this->runTests($classNames, $coverage); |
||
| 240 | } |
||
| 241 | } |
||
| 242 | |||
| 243 | /** |
||
| 244 | * Run tests for one or more "modules". |
||
| 245 | * A module is generally a toplevel folder, e.g. "mysite" or "framework". |
||
| 246 | */ |
||
| 247 | public function module($request, $coverage = false) { |
||
| 248 | self::use_test_manifest(); |
||
| 249 | $classNames = array(); |
||
| 250 | $moduleNames = explode(',', $request->param('ModuleName')); |
||
| 251 | |||
| 252 | $ignored = array('functionaltest', 'phpsyntaxtest'); |
||
| 253 | |||
| 254 | foreach($moduleNames as $moduleName) { |
||
| 255 | $classNames = array_merge( |
||
| 256 | $classNames, |
||
| 257 | $this->getTestsInDirectory($moduleName, $ignored) |
||
| 258 | ); |
||
| 259 | } |
||
| 260 | |||
| 261 | $this->runTests($classNames, $coverage); |
||
| 262 | } |
||
| 263 | |||
| 264 | /** |
||
| 265 | * Find all test classes in a directory and return an array of them. |
||
| 266 | * @param string $directory To search in |
||
| 267 | * @param array $ignore Ignore these test classes if they are found. |
||
| 268 | * @return array |
||
| 269 | */ |
||
| 270 | protected function getTestsInDirectory($directory, $ignore = array()) { |
||
| 274 | |||
| 275 | /** |
||
| 276 | * Find all test classes in a file and return an array of them. |
||
| 277 | * @param string $file To search in |
||
| 278 | * @param array $ignore Ignore these test classes if they are found. |
||
| 279 | * @return array |
||
| 280 | */ |
||
| 281 | protected function getTestsInFile($file, $ignore = array()) { |
||
| 285 | |||
| 286 | /** |
||
| 287 | * @param array $classes to search in |
||
| 288 | * @param array $ignore Ignore these test classes if they are found. |
||
| 289 | */ |
||
| 290 | protected function filterTestClasses($classes, $ignore) { |
||
| 291 | $testClasses = array(); |
||
| 292 | if($classes) { |
||
| 293 | foreach($classes as $className) { |
||
| 294 | if( |
||
| 295 | class_exists($className) && |
||
| 296 | is_subclass_of($className, 'SapphireTest') && |
||
| 297 | !in_array($className, $ignore) |
||
| 298 | ) { |
||
| 299 | $testClasses[] = $className; |
||
| 300 | } |
||
| 301 | } |
||
| 302 | } |
||
| 303 | return $testClasses; |
||
| 304 | } |
||
| 305 | |||
| 306 | /** |
||
| 307 | * Run tests for a test suite defined in phpunit.xml |
||
| 308 | */ |
||
| 309 | public function suite($request, $coverage = false) { |
||
| 310 | self::use_test_manifest(); |
||
| 311 | $suite = $request->param('SuiteName'); |
||
| 312 | $xmlFile = BASE_PATH.'/phpunit.xml'; |
||
| 313 | if(!is_readable($xmlFile)) { |
||
| 314 | user_error("TestRunner::suite(): $xmlFile is not readable", E_USER_ERROR); |
||
| 315 | } |
||
| 316 | $xml = simplexml_load_file($xmlFile); |
||
| 317 | $suite = $xml->xpath("//phpunit/testsuite[@name='$suite']"); |
||
| 318 | if(empty($suite)) { |
||
| 319 | user_error("TestRunner::suite(): couldn't find the $suite testsuite in phpunit.xml"); |
||
| 320 | } |
||
| 321 | $suite = array_shift($suite); |
||
| 322 | $classNames = array(); |
||
| 323 | if(isset($suite->directory)) { |
||
| 324 | foreach($suite->directory as $directory) { |
||
| 325 | $classNames = array_merge($classNames, $this->getTestsInDirectory($directory)); |
||
| 326 | } |
||
| 327 | } |
||
| 328 | if(isset($suite->file)) { |
||
| 329 | foreach($suite->file as $file) { |
||
| 330 | $classNames = array_merge($classNames, $this->getTestsInFile($file)); |
||
| 331 | } |
||
| 332 | } |
||
| 333 | |||
| 334 | $this->runTests($classNames, $coverage); |
||
| 335 | } |
||
| 336 | |||
| 337 | /** |
||
| 338 | * Give us some sweet code coverage reports for a particular suite. |
||
| 339 | */ |
||
| 340 | public function coverageSuite($request) { |
||
| 343 | |||
| 344 | /** |
||
| 345 | * @param array $classList |
||
| 346 | * @param boolean $coverage |
||
| 347 | */ |
||
| 348 | public function runTests($classList, $coverage = false) { |
||
| 434 | |||
| 435 | public function setUp() { |
||
| 439 | |||
| 440 | public function tearDown() { |
||
| 443 | } |
||
| 444 |
Loading history...
