Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extended Util/Common.php not found in unit tests. (3.0.1) #1564

Closed
louisl opened this issue Jul 18, 2017 · 12 comments
Closed

Extended Util/Common.php not found in unit tests. (3.0.1) #1564

louisl opened this issue Jul 18, 2017 · 12 comments

Comments

@louisl
Copy link

louisl commented Jul 18, 2017

I've extended Util/Common.php for some DRY arrays and functions, this all works fine when sniffing and fixing, but when a unit test file tests a sniff that 'use's the extended Common.php it's chucking out a class not found error.

Is this something the autoloading should do? or is it something I need to alter?

$ ./vendor/bin/phpunit --debug --filter CodeIgniter4 ./tests/AllTests.php
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.


Starting test 'CodeIgniter4\Tests\Arrays\ArrayDeclarationUnitTest::testSniff'.
.
Starting test 'CodeIgniter4\Tests\NamingConventions\ValidMethodNameUnitTest::testSniff'.
PHP Fatal error:  Class 'CodeIgniter4\Util\Common' not found in /Users/louislinehan/Documents/Projects/CodeIgniter4-Standard/CodeIgniter4/Sniffs/NamingConventions/ValidMethodNameSniff.php on line 98
@gsherwood
Copy link
Member

I think I'd need to be able to look at the code and try out a phpunit run to properly diagnose the problem here.

The autoloader may be able to find the file, but possibly not in a unit test run if it has no idea what custom namespace you are using. I'd have to debug it to see if that's possible. If not, you'd need to require the file yourself.

Is this standard available in a public repo for me to try out?

@jrfnl
Copy link
Contributor

jrfnl commented Jul 18, 2017

I've seen similar issues with PHPCS 3.x in combination with non-sniff files (abstract base sniff classes and such) and unit tests.

It seems the PHPCS 3.x autoloader handles these files fine when it's being run normally, but does not handle this at all when in the unit test environment.

Loosely related to #1469

As a temporary solution until this is fixed in PHPCS, you may want to try the below (which worked for me):

Adding a custom autoload file to the ruleset using <autoload>./../MyAutoLoad.php</autoload> with something along the lines of the below as content:

<?php
/*
 * Register an autoloader.
 *
 * This autoloader is not needed for running the sniffs, however, it *is* needed
 * for running the unit tests as the PHPCS native autoloader runs into trouble there.
 */
if (defined('PHP_CODESNIFFER_IN_TESTS')) {
    spl_autoload_register(function ($class) {
        // Only try & load our own classes.
        if (stripos($class, 'MyNameSpace') !== 0) {
            return;
        }

        $file = realpath(__DIR__) . DIRECTORY_SEPARATOR . strtr($class, '\\', DIRECTORY_SEPARATOR) . '.php';

        if (file_exists($file)) {
            include_once($file);
        }
    });
}

Edit: Just to be clear & to help you get the paths working - I have this file set up in the repository root. The standard itself is contained in a subdirectory, which also contains the ruleset.xml file.
The file is loaded in the unit test scenario via a PHPUnit bootstrap (after including the PHPCS native autoloader).

@louisl
Copy link
Author

louisl commented Jul 18, 2017

Thanks for the quick reply.

I've only just started the unit tests so there's not many there but please take a look at the develop branch here:

https://github.com/louisl/CodeIgniter4-Standard/tree/develop

@louisl
Copy link
Author

louisl commented Jul 18, 2017

@jrfnl Thank you also for the quick reply and info.

@gsherwood
Copy link
Member

I've only just started the unit tests so there's not many there but please take a look at the develop branch here:

Thanks for that. I can replicate the problem and will take a look into it.

@gsherwood gsherwood added this to the 3.1.0 milestone Jul 18, 2017
gsherwood added a commit that referenced this issue Jul 18, 2017
…the same locations as during normal phpcs runs (ref #1564)
@gsherwood
Copy link
Member

I've pushed up a change to the way unit testing works to get the autoloader to work in the same way during unit tests runs as it does during a normal phpcs run. This means that installed paths will be added to the autoloader search paths like the main runner does, and ruleset.xml files will be parsed to find custom namespaces and to add the correct search path for them as well.

@louisl I've tried this change with your standard and it looks to be working ok.

@jrfnl It would be great if you could find some time to test this with the work you're doing on the WordPress standards, and PHPCompatibility if you are still working on those tests as well.

@jrfnl
Copy link
Contributor

jrfnl commented Jul 18, 2017

@gsherwood Just tested with the PHPCS 3.x compatible branch of PHPCompatibility and that removed the need for the above mentioned work-around. So yes, based on these initial tests, the change looks good to me. Thanks for that!

I'll test with WPCS later & will let you know the results for that as well.

@gsherwood
Copy link
Member

So yes, based on these initial tests, the change looks good to me.

Great news. Thanks.

@louisl
Copy link
Author

louisl commented Jul 18, 2017

@gsherwood Thanks very much! Confirmed working for me too.

@gsherwood
Copy link
Member

Confirmed working for me too.

Thanks for testing the fix for me.

@jrfnl
Copy link
Contributor

jrfnl commented Jul 23, 2017

@gsherwood Just letting you know: I've tested this with WPCS now as well and can confirm that this solution works for WPCS as well.

jrfnl added a commit to jrfnl/WordPress-Coding-Standards that referenced this issue Jul 23, 2017
…l as 3.x

The principle of how the unit tests will be run is now significantly different for PHPCS 2.x vs 3.x.

### PHPCS 3.x:
* To get the unit tests running, we need to make sure that our class aliases are in place, however to alias the PHPCS native classes, PHP needs to be able to find them, so we need to load the PHPCS native autoloader first.
* Additionally, the PHPCS native autoloader has a bug which means that non-sniff classes (abstracts, helper class) are not loaded correctly. This bug is fixed in PHPCS `master`, but is not contained (yet) in a released version. As a work-around, a WPCS autoloader is registered to make sure our classes can be loaded anyway. See: squizlabs/PHP_CodeSniffer/issues/1564

### PHPCS 2.x:
* The PHPCS 2.x unit test suite does not handle namespaced unit test classes. Or rather: it is not capable of translating the namespaced unit test classes to the namespaced sniff they belong to.
* To work around this, two small snippets of code need to be added to the PHPCS 2.x unit test classes for which we overload select methods.
* However, one of the methods we need to overload is declared as `final`, so for the `AbstractSniffUnitTest` class, we need a complete copy of the file and cannot just overload the relevant method.
* The changes made to the classes are fenced with inline comments annotating the change.
* The code style of these copied in classes has been left as is and these files will be excluded from the WPCS code style check.
* The WPCS version of the `AbstractSniffUnitTest` class is aliased to the PHPCS 3.x name via the PHPUnit bootstrap, so all the unit test classes can already use the PHPCS 3.x class name in their `use` statements.

### Bringing it all together.
This also means that the command to run the unit tests will be different in PHPCS 2.x vs 3.x:
```
For running the unit tests with PHPCS 3.x:
phpunit --bootstrap="./Test/phpcs3-bootstrap.php" --filter WordPress /path/to/PHP_CodeSniffer/tests/AllTests.php

For running the unit tests with PHPCS 2.x:
phpunit --bootstrap="./Test/phpcs2-bootstrap.php" --filter WordPress ./Test/AllTests.php
```
The contributing instructions will be updated to reflect this.

In travis, a `PHPCS_BRANCH` environment variable will be available, so there is also a generic `bootstrap.php` file which toggles between the version specific bootstraps based on the environment variable.
@gsherwood
Copy link
Member

Just letting you: I've tested this with WPCS now as well and can confirm that this solution works for WPCS as well.

Thanks for testing that for me.

jrfnl added a commit to WordPress/WordPress-Coding-Standards that referenced this issue Jul 24, 2017
…l as 3.x

The principle of how the unit tests will be run is now significantly different for PHPCS 2.x vs 3.x.

* To get the unit tests running, we need to make sure that our class aliases are in place, however to alias the PHPCS native classes, PHP needs to be able to find them, so we need to load the PHPCS native autoloader first.
* Additionally, the PHPCS native autoloader has a bug which means that non-sniff classes (abstracts, helper class) are not loaded correctly. This bug is fixed in PHPCS `master`, but is not contained (yet) in a released version. As a work-around, a WPCS autoloader is registered to make sure our classes can be loaded anyway. This work-around can be removed once the WPCS minimum required PHPCS 3.x version goes up to 3.1.0. See: squizlabs/PHP_CodeSniffer/issues/1564

* The PHPCS 2.x unit test suite does not handle namespaced unit test classes. Or rather: it is not capable of translating the namespaced unit test classes to the namespaced sniff they belong to.
* To work around this, two small snippets of code need to be added to the PHPCS 2.x unit test classes for which we overload select methods.
* However, one of the methods we need to overload is declared as `final`, so for the `AbstractSniffUnitTest` class, we need a complete copy of the file and cannot just overload the relevant method.
* The changes made to the classes are fenced with inline comments annotating the change.
* The code style of these copied in classes has been left as is and these files will be excluded from the WPCS code style check.
* The WPCS version of the `AbstractSniffUnitTest` class is aliased to the PHPCS 3.x name via the PHPUnit bootstrap, so all the unit test classes can already use the PHPCS 3.x class name in their `use` statements.

This also means that the command to run the unit tests will be different in PHPCS 2.x vs 3.x:
```
For running the unit tests with PHPCS 3.x:
phpunit --bootstrap="./Test/phpcs3-bootstrap.php" --filter WordPress /path/to/PHP_CodeSniffer/tests/AllTests.php

For running the unit tests with PHPCS 2.x:
phpunit --bootstrap="./Test/phpcs2-bootstrap.php" --filter WordPress ./Test/AllTests.php
```
The contributing instructions will be updated to reflect this.

In travis, a `PHPCS_BRANCH` environment variable will be available, so there is also a generic `bootstrap.php` file which toggles between the version specific bootstraps based on the environment variable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants