Allow swagger-php 4.x #436

caugner · 2022-01-20T15:22:44Z

Fixes #432.

caugner · 2022-01-20T15:34:40Z

@DarkaOnLine To run tests both with 3.x and 4.x, maybe it would make sense to convert the existing matrix dimension stability into composer_flags with two values '' and '--prefer-lowest', like here: https://github.com/psalm/psalm-plugin-laravel/blob/64e95fc4bf011225a1635e04de61831b1e473aa1/.github/workflows/test.yml#L16

edit Added this in a8c7128.

DarkaOnLine

Swagger-UI also has a v4. Can we bump the version of it? Template also changed, can we updated ours: https://github.com/swagger-api/swagger-ui/blob/master/dist/index.html

.github/workflows/test-config.yml

caugner · 2022-01-21T09:35:30Z

Swagger-UI also has a v4. Can we bump the version of it?

Just to be sure: In contrast to zircote/swagger-php, this would be a real bump from ^3.0 to ^4.0, right? I'd suggest to do this in a separate PR, because these dependencies don't depend on each other.

caugner · 2022-01-21T10:05:17Z

Looks like L5-Swagger is affected by the removal of some deprecated elements. 🤔

OpenApi\Analysis::processors() was deprecated in swagger-php 3.3.0 (zircote/swagger-php@139cea2), but the replacement (Generator::getProcessors()) was introduced in swagger-php 3.2.0 (zircote/swagger-php@c5f0819), so to make L5-Swagger compatible with swagger-php 4.x, we need to require swagger-php ^3.2.

This allows us to migrate object-oriented Generator.

caugner · 2022-01-21T10:40:53Z

As for the codeclimate, my PR fixes and adds 6 issues each, I'm open to any suggestions to fix those.

I tried to run the tests locally, but I'm getting mkdir(): Permission denied errors:

> vendor/bin/phpunit
PHPUnit 9.5.12 by Sebastian Bergmann and contributors.

Warning:       XDEBUG_MODE=coverage or xdebug.mode=coverage has to be set

EEEEEEEEEEEEEEEEEEEEEEEEEEEEEE                                    30 / 30 (100%)

Time: 00:00.528, Memory: 20.00 MB

There were 30 errors:

1) Tests\ConfigFactoryTest::ifThrowsExceptionIfDocumentationConfigNotFound
ErrorException: mkdir(): Permission denied

/home/claas/git/github.com/L5-Swagger/tests/TestCase.php:208
/home/claas/git/github.com/L5-Swagger/tests/TestCase.php:36

After running these commands from the CI config, I was able to run the tests:

L5-Swagger/.github/workflows/test-config.yml

Lines 49 to 56 in aab46bf

    
                   sudo chown -R $USER:$USER . 
        
                   sudo chmod -R g+rw . 
        
                   mkdir -p tests/storage/logs/test-reports 
        
                   mkdir -p vendor/orchestra/testbench-core/laravel/vendor/swagger-api 
        
                   mkdir -p vendor/orchestra/testbench-core/laravel/vendor/swagger-api/swagger-ui 
        
                   mkdir -p vendor/orchestra/testbench-core/laravel/vendor/swagger-api/swagger-ui/dist 
        
                   sudo chown -R $USER:$USER vendor/orchestra/testbench-core/laravel/vendor/swagger-api 
        
                   chmod -R 777 vendor/orchestra/testbench-core/laravel/vendor/swagger-api

Maybe this should be documented in the README, automatised via a composer hook, or just happen automatically as part of the TestCase. 🤔

caugner · 2022-01-21T10:43:39Z

Now, I'm getting the following failures (with --prefer-stable):

> vendor/bin/phpunit
PHPUnit 9.5.12 by Sebastian Bergmann and contributors.

Warning:       XDEBUG_MODE=coverage or xdebug.mode=coverage has to be set

...EE..EEEEFFE...............E                                    30 / 30 (100%)

Time: 00:01.440, Memory: 28.00 MB

There were 8 errors:

1) Tests\ConsoleTest::canGenerate with data set "default" ('l5-swagger:generate')
ErrorException: Required @OA\Info() not found

/home/claas/git/github.com/L5-Swagger/vendor/zircote/swagger-php/src/Loggers/DefaultLogger.php:27
/home/claas/git/github.com/L5-Swagger/vendor/psr/log/src/LoggerTrait.php:86
/home/claas/git/github.com/L5-Swagger/vendor/zircote/swagger-php/src/Annotations/AbstractAnnotation.php:455
/home/claas/git/github.com/L5-Swagger/vendor/zircote/swagger-php/src/Annotations/OpenApi.php:145
/home/claas/git/github.com/L5-Swagger/vendor/zircote/swagger-php/src/Analysis.php:438
/home/claas/git/github.com/L5-Swagger/vendor/zircote/swagger-php/src/Generator.php:353
/home/claas/git/github.com/L5-Swagger/src/Generator.php:178
/home/claas/git/github.com/L5-Swagger/src/Generator.php:122
/home/claas/git/github.com/L5-Swagger/src/Console/GenerateDocsCommand.php:72
/home/claas/git/github.com/L5-Swagger/src/Console/GenerateDocsCommand.php:58
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:36
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Container/Util.php:40
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:93
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:37
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Container/Container.php:653
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Console/Command.php:136
/home/claas/git/github.com/L5-Swagger/vendor/symfony/console/Command/Command.php:298
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Console/Command.php:121
/home/claas/git/github.com/L5-Swagger/vendor/symfony/console/Application.php:1005
/home/claas/git/github.com/L5-Swagger/vendor/symfony/console/Application.php:299
/home/claas/git/github.com/L5-Swagger/vendor/symfony/console/Application.php:171
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Console/Application.php:94
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Console/Application.php:186
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php:263
/home/claas/git/github.com/L5-Swagger/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php:261
/home/claas/git/github.com/L5-Swagger/tests/ConsoleTest.php:22

Sounds like zircote/swagger-php#1005, but I would have to look into this further.

caugner · 2022-01-21T10:51:59Z

As for the Required @OA\Info() not found errors: The tests are not entirely clear to me, but I guess they run on the annotations from the tests/storage/annotations/OpenApi directory. And at least the Anotations.php file does have an @OA\Info(), so maybe it's a bug in 77445da:

L5-Swagger/tests/storage/annotations/OpenApi/Anotations.php

Lines 3 to 16 in aab46bf

    
           /** 
        
            * @OA\Info( 
        
            *      version="1.0.0", 
        
            *      title="L5 OpenApi", 
        
            *      description="L5 Swagger OpenApi description", 
        
            *      @OA\Contact( 
        
            *          email="darius@matulionis.lt" 
        
            *      ), 
        
            *     @OA\License( 
        
            *         name="Apache 2.0", 
        
            *         url="http://www.apache.org/licenses/LICENSE-2.0.html" 
        
            *     ) 
        
            * ) 
        
            */

caugner · 2022-01-21T10:52:40Z

@DarkaOnLine Maybe you can make some sense of this?

coveralls · 2022-02-01T06:24:08Z

Coverage increased (+0.4%) to 97.386% when pulling 5fa6274 on caugner:allow-swagger-php-4.x into dcdea59 on DarkaOnLine:master.

DarkaOnLine · 2022-02-01T07:33:22Z

@caugner I did some changes to your branch. Can you please review and if all good, we could merge this ;)

caugner

LGTM

caugner · 2022-02-02T16:16:59Z

@DarkaOnLine Excellent job, IMO this is ready for merge.

DarkaOnLine · 2022-02-03T07:43:14Z

@caugner thanks for contributing, appreciate your help 🙏

The new release - 8.2.0 is live now 🎉

build: allow swagger-php 4.x

adb1fea

ci: test both stable + lowest

a8c7128

DarkaOnLine reviewed Jan 21, 2022

View reviewed changes

.github/workflows/test-config.yml Outdated Show resolved Hide resolved

ci: use composer update

e429073

caugner requested a review from DarkaOnLine January 21, 2022 09:35

ci: don't require swagger-php

14eadf0

caugner added 2 commits January 21, 2022 11:32

build: require swagger-php >= 3.2

dde98f7

This allows us to migrate object-oriented Generator.

refactor: migrate to object-oriented Generator

77445da

caugner mentioned this pull request Jan 21, 2022

[CI] Test stable and lowest #437

Closed

caugner marked this pull request as draft January 21, 2022 10:56

DarkaOnLine added 5 commits January 24, 2022 09:03

Fix annotations for tests

acbf3ab

Merge remote-tracking branch 'origin/master' into allow-swagger-php-4.x

5baa03a

Add --prefer-stable

995ab2a

Update tests config

8d8de4e

Update tests config

48294bc

Update tests config

f8b7d6d

DarkaOnLine marked this pull request as ready for review February 1, 2022 06:42

DarkaOnLine added 5 commits February 1, 2022 09:05

Move annotations to separate files

7d427cc

Move annotations to separate files

c6fe80d

Move annotations to separate files

2e59e65

Tests

88652ca

CS fix

944a5b9

CS fix

5fa6274

caugner commented Feb 2, 2022

View reviewed changes

DarkaOnLine approved these changes Feb 3, 2022

View reviewed changes

DarkaOnLine merged commit f7f4451 into DarkaOnLine:master Feb 3, 2022

caugner deleted the allow-swagger-php-4.x branch February 3, 2022 10:09

xaviapa mentioned this pull request Feb 8, 2022

Required @OA\Info() not found #443

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow swagger-php 4.x #436

Allow swagger-php 4.x #436

caugner commented Jan 20, 2022

caugner commented Jan 20, 2022 •

edited

Loading

DarkaOnLine left a comment •

edited

Loading

caugner commented Jan 21, 2022

caugner commented Jan 21, 2022

caugner commented Jan 21, 2022

caugner commented Jan 21, 2022

caugner commented Jan 21, 2022

caugner commented Jan 21, 2022

coveralls commented Feb 1, 2022 •

edited

Loading

DarkaOnLine commented Feb 1, 2022

caugner left a comment

caugner commented Feb 2, 2022

DarkaOnLine commented Feb 3, 2022

Allow swagger-php 4.x #436

Allow swagger-php 4.x #436

Conversation

caugner commented Jan 20, 2022

caugner commented Jan 20, 2022 • edited Loading

DarkaOnLine left a comment • edited Loading

Choose a reason for hiding this comment

caugner commented Jan 21, 2022

caugner commented Jan 21, 2022

caugner commented Jan 21, 2022

caugner commented Jan 21, 2022

caugner commented Jan 21, 2022

caugner commented Jan 21, 2022

coveralls commented Feb 1, 2022 • edited Loading

DarkaOnLine commented Feb 1, 2022

caugner left a comment

Choose a reason for hiding this comment

caugner commented Feb 2, 2022

DarkaOnLine commented Feb 3, 2022

caugner commented Jan 20, 2022 •

edited

Loading

DarkaOnLine left a comment •

edited

Loading

coveralls commented Feb 1, 2022 •

edited

Loading