rtCamp / login-with-google

Minimal plugin which allows WordPress user to login with google.
https://wordpress.org/plugins/login-with-google/
GNU General Public License v2.0
57 stars 17 forks source link

Fix: PHPCS Errors for PHP files #143

Closed hbhalodia closed 1 year ago

hbhalodia commented 1 year ago

Issue - #142

thelovekesh commented 1 year ago

@hbhalodia PHPUnit tests seem to be failing as there is test scripts specified in the composer scripts[].

Below diff should fix the issue.

diff --git a/.github/workflows/phpunit_on_pull_request.yml b/.github/workflows/phpunit_on_pull_request.yml
index fcca561..e2670d3 100644
--- a/.github/workflows/phpunit_on_pull_request.yml
+++ b/.github/workflows/phpunit_on_pull_request.yml
@@ -48,9 +48,13 @@ jobs:
         if: steps.check_files.outputs.files_exists == 'true'
         run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"

+      - name: Update composer packages
+        if: steps.check_files.outputs.files_exists == 'true'
+        run: composer update
+
       - name: Run PHPUnit
         if: steps.check_files.outputs.files_exists == 'true'
-        run: composer update && composer tests:unit
+        run: vendor/bin/phpunit tests/php/Unit/ --verbose

       - name: Archive code coverage results
         uses: actions/upload-artifact@v2
hbhalodia commented 1 year ago

Hi @thelovekesh, Seems like some of the tests aer giving errors due to changes in path for the assets.

1) RtCamp\GoogleLogin\Tests\Unit\Modules\AssetsTest::testRegisterLoginStyles 2) RtCamp\GoogleLogin\Tests\Unit\Modules\AssetsTest::testEnqueueLoginStyleWithStyleNotRegistered,

as we have not added build files folder and seems like in enquing this scripts and styles we need to change a code bit there to achieve what we need and then have to check the test.

Other error is use of wp_json_encode, not able to identify that function and adding as undefined because wp_json_encode is a WordPress function and we are calling in PHP environment. Should I update the rule to exclude this phpcs error or should we create some wrapper function to the main testee class and should call wp_json_encode function there? Let me know your thoughts on this.

Thanks.

thelovekesh commented 1 year ago

@hbhalodia I think we can ignore the tests here for now since changes are going to be merged on a develop(refactor/v2) branch.