Closed dugajean closed 5 years ago
I didn't notice that the build has failed. It's because of an include in tests/bootstrap.php
. I'm going to look into this and push a fix if possible.
Thanks for this great PR. :heart: I will check and validate your changes, ideas in the next view days. So that you have the chance to send a commit for the error check to fix them and the PR is more valid.
@dugajean again and again - thanks a lot, great PR, nice improvements and I like them and I will merge it fastly. But I will wait for an commit to fix the travis build.
@bueltge Thanks a lot! I will be fixing that and push it very soon. I'm glad it's proper.
@dugajean We have also an xml config file for PhpStorm for our php codex; maybe you need them? I will upload to the repository branch so that PhpStorm will use them. Makes easier to format via PhpStorm.
Added via e8bf9dd190c29d050631d1e7a6287ba398bd43be
@bueltge Yes, that would be useful! Thank you!
So for the builds, everything should be fine now normally as the tests are passing and phpcs
is returning only warnings, but the exit code isn't 0 and I finally figured out why:
I copied the Request
and ParameterBag
classes from the Inpsyde Google Tag Manager plugin and there are some phpcs
suppressions there. Oddly, the php_codesniffer
package doesn't take into consideration that the suppression means ignoring the error. I will adjust the .travis.yml
config and see how it goes. Hopefully the builds will pass after that.
That's a great step forward for the project. Maybe we find more time to go the next steps to leave the old plugin, the old code and get solid plugin with code there we will be maintain. Thanks again!
After composer update
I can't run locally the unittest.
/v/w/w/Adminimize (refactor) $ vendor/bin/phpunit
PHPUnit 6.3.1 by Sebastian Bergmann and contributors.
<p>Dummy Content for AdminBar Settings</p>
PHP Fatal error: Uncaught Error: Call to undefined function wp_kses() in /var/www/wp-plugins/Adminimize/src/Templates/AdminBar.php:5
Stack trace:
#0 /var/www/wp-plugins/Adminimize/vendor/phpunit/php-code-coverage/src/CodeCoverage.php(1122): include_once()
#1 /var/www/wp-plugins/Adminimize/vendor/phpunit/php-code-coverage/src/CodeCoverage.php(269): SebastianBergmann\CodeCoverage\CodeCoverage->initializeData()
#2 /var/www/wp-plugins/Adminimize/vendor/phpunit/phpunit/src/Framework/TestResult.php(659): SebastianBergmann\CodeCoverage\CodeCoverage->start(Object(Adminimize\Tests\Unit\AdminimizeTest))
#3 /var/www/wp-plugins/Adminimize/vendor/phpunit/phpunit/src/Framework/TestCase.php(883): PHPUnit\Framework\TestResult->run(Object(Adminimize\Tests\Unit\AdminimizeTest))
#4 /var/www/wp-plugins/Adminimize/vendor/phpunit/phpunit/src/Framework/TestSuite.php(744): PHPUnit\Framework\TestCase->run(Object(PHPUnit\Framework\TestResult))
#5 /var/www/wp-plugins/Adminimize/vendor/phpunit/phpunit/src/Framework/TestSuite.php(744): PHPUnit\Framework\ in /var/www/wp-plugins/Adminimize/src/Templates/AdminBar.php on line 5
Maybe you have a hint for me? Also the tabs are currently not in WP default style and ux:
That' the default
@bueltge For the error, I can't seem to be able to reproduce it. Maybe it's because we don't have the same environments? I have PHP 7.2.9 and WordPress 4.9.8. I have used a missing function in Templates/AdminBar.php
and the tests were passing anyway. Could you tell me what exactly you are using (describe your environment)?
I'll fix the tab design too. After I fix everything, I'll send another Pull Request.
After careful inspection I noticed that this error shows up when XDebug is on (it was off in my case). I'm going to look into a fix and push it.
Tabs have already been fixed. Pull Request pending...
So the issue happens because of the code coverage which happens when XDebug is running. I attempted mocking wp_kses
, which fixed the problem, but then another error came up saying that CodeCoverage::form()
doesn't exist (because of the $this->form()
call in AdminBar.php
).
This problem can be "fixed" by adding <directory>src/Templates</directory>
under the <exclude>
block in phpunit.xml.dist
. I'm not sure if that's an acceptable solution though.
I think we should not test via UnitTests the templates, so that we can exclude them. Thanks for this analyze and your help!
What's Included in this Pull Request
SettingsRepository
(partial implementation of WP_Cache inclusion)SettingsRepository
Tab
feature refactoringPlease note that this Pull Request is larger than it should be as it includes more than a specific feature. I deemed it as necessary to restructure the plugin's layout into something more intuitive and friendly. If something's out of order, feel free to let me know and I'll fix it.
Things not included in this Pull Request
Tab
have its own key in themw_adminimize
option or should there be only one large form including all settings which are split in tabs?)