Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixed issue with including including PHPUnit's Autoload script. #1174

Merged
merged 6 commits into from

3 participants

@psparrow

This pull request makes two small changes to CakeTestSuiteDispatcher::loadTestFramework().

First, it resolves any issues with vendor paths ending with a DS character by stripping the character from the end of the vendor path and only adding it where necessary.

Second, it now returns TRUE or FALSE instead of the return value from PHPUnit/Autoload.php. PHPUnit/Autoload.php returns NULL when included successfully, which causes the result to be treated as a false value in CakeTestSuiteDispatcher::_checkPHPUnit().

...e/Test/Case/TestSuite/CakeTestSuiteDispatcherTest.php
@@ -0,0 +1,28 @@
+<?php
+
+class CakeTestSuiteDispatcherTest extends CakeTestCase {
+
+ protected function clearPaths() {
+ App::build(array('Vendor' => array('junk')), App::RESET);
+ ini_set('include_path', 'junk');
@markstory Owner

This will destroy an application include_paths.

What if we restore them in the tearDown() of the test? I wanted to find a way to test the failure of including PHPUnit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@psparrow psparrow commented on the diff
...e/Test/Case/TestSuite/CakeTestSuiteDispatcherTest.php
@@ -0,0 +1,38 @@
+<?php
+
+class CakeTestSuiteDispatcherTest extends CakeTestCase {
+
+ public function setUp() {
+ $this->vendors = App::path('vendors');
+ $this->includePath = ini_get('include_path');
+ }
+
+ public function tearDown() {

Added code to restore the include_path and vendor paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...e/Test/Case/TestSuite/CakeTestSuiteDispatcherTest.php
((16 lines not shown))
+ App::build(array('Vendor' => array('junk')), App::RESET);
+ ini_set('include_path', 'junk');
+ }
+
+ public function testLoadTestFramework() {
+ $dispatcher = new CakeTestSuiteDispatcher();
+
+ $this->assertTrue($dispatcher->loadTestFramework());
+
+ $this->clearPaths();
+
+ $exception = null;
+
+ try {
+ $dispatcher->loadTestFramework();
+ } catch(Exception $ex) {
@markstory Owner

Should be } catch (.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@psparrow psparrow commented on the diff
...e/Test/Case/TestSuite/CakeTestSuiteDispatcherTest.php
((16 lines not shown))
+ App::build(array('Vendor' => array('junk')), App::RESET);
+ ini_set('include_path', 'junk');
+ }
+
+ public function testLoadTestFramework() {
+ $dispatcher = new CakeTestSuiteDispatcher();
+
+ $this->assertTrue($dispatcher->loadTestFramework());
+
+ $this->clearPaths();
+
+ $exception = null;
+
+ try {
+ $dispatcher->loadTestFramework();
+ } catch (Exception $ex) {

I fixed the code formatting. Anything else?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@markstory markstory merged commit 76ea080 into cakephp:master
@ceeram
Collaborator

The newly added file does not follow cakephp coding conventions, phpcs fails, spaces were used for indenting. CakePHP uses tabs for indenting.

@ceeram
Collaborator

Fixed coding standards: 89100f9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 13, 2013
  1. @psparrow

    Fixed PHPUnit include bug.

    psparrow authored
    Fixed vendor include path bug.
  2. @psparrow
  3. @psparrow
  4. @psparrow

    Merge branch 'master' of github.com:psparrow/cakephp

    psparrow authored
    Conflicts:
    	lib/Cake/Test/Case/TestSuite/CakeTestSuiteDispatcherTest.php
  5. @psparrow

    Fixed code formatting.

    psparrow authored
  6. @psparrow
This page is out of date. Refresh to see the latest.
View
38 lib/Cake/Test/Case/TestSuite/CakeTestSuiteDispatcherTest.php
@@ -0,0 +1,38 @@
+<?php
+
+class CakeTestSuiteDispatcherTest extends CakeTestCase {
+
+ public function setUp() {
+ $this->vendors = App::path('vendors');
+ $this->includePath = ini_get('include_path');
+ }
+
+ public function tearDown() {

Added code to restore the include_path and vendor paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ App::build(array('Vendor' => $this->vendors), App::RESET);
+ ini_set('include_path', $this->includePath);
+ }
+
+ protected function clearPaths() {
+ App::build(array('Vendor' => array('junk')), App::RESET);
+ ini_set('include_path', 'junk');
+ }
+
+ public function testLoadTestFramework() {
+ $dispatcher = new CakeTestSuiteDispatcher();
+
+ $this->assertTrue($dispatcher->loadTestFramework());
+
+ $this->clearPaths();
+
+ $exception = null;
+
+ try {
+ $dispatcher->loadTestFramework();
+ } catch (Exception $ex) {

I fixed the code formatting. Anything else?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $exception = $ex;
+ }
+
+ $this->assertEquals(get_class($exception), "PHPUnit_Framework_Error_Warning");
+ }
+
+}
View
5 lib/Cake/TestSuite/CakeTestSuiteDispatcher.php
@@ -138,13 +138,14 @@ protected function _checkPHPUnit() {
*/
public function loadTestFramework() {
foreach (App::path('vendors') as $vendor) {
- if (is_dir($vendor . 'PHPUnit')) {
+ $vendor = rtrim($vendor, DS);
+ if (is_dir($vendor . DS . 'PHPUnit')) {
ini_set('include_path', $vendor . PATH_SEPARATOR . ini_get('include_path'));
break;
}
}
- return include 'PHPUnit' . DS . 'Autoload.php';
+ return (include('PHPUnit' . DS . 'Autoload.php')) !== false;
}
/**
Something went wrong with that request. Please try again.