Skip to content

Reconfigure Travis-CI #685

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

Merged
merged 1 commit into from
Jun 29, 2020
Merged

Conversation

murtukov
Copy link
Contributor

@murtukov murtukov commented Jun 27, 2020

  • Drop Travis jobs on PHP versions below 7.4
  • Replace Xdebug driver with PCOV for coverage tests

Update:

  • Update PHP-CS-Fixer to v2.16.4

@mavimo
Copy link
Contributor

mavimo commented Jun 27, 2020

@murtukov any reason (related to PCOV) to drop the previous versions of PHP?

@murtukov
Copy link
Contributor Author

murtukov commented Jun 27, 2020

@mavimo No, there is no specific reason related to PCOV. It just aims the master branch, which is GraphQLBundle 1.0. It requires PHP >= 7.4 (https://github.com/overblog/GraphQLBundle/blob/master/docs/index.md#versions-requirements)

@mavimo
Copy link
Contributor

mavimo commented Jun 28, 2020

@murtukov the php-cs-fixer version we are using at the moment (2.15.3) fail on php7.4, can you bump the required version to v2.16.4 in our composer.json, this should fix the travis-ci failure, thx!

@murtukov
Copy link
Contributor Author

murtukov commented Jun 28, 2020

@mavimo ok, i changed the version but the travis jobs for php-cs-fixer and phpstan were still failing. I performed cs-fix and fixed the reported errors for static analysis.

  • PHP-CS-Fixer only removed redundant PHPDoc tags
  • I disabled the PHP-CS-Fixer's single_line_throw rule because we have a planty of throw messages that need to be broken into multiple lines. This rules was just messing the code up.
  • I also removed some ignored entries from static-analysis errors, because I fixed them in the code.

The another problem is, that the local code-quality and the one in travis are generating a little different report, that's why I pushed so many commits, because I coudln't check code-quality locally.

I don't know, how other branches were merged, if they all fail in coding quality, even 0.11.

@mavimo
Copy link
Contributor

mavimo commented Jun 28, 2020

@murtukov LGTM, I see that this branch need to be rebased on master, to you have time to fix it? PS: can you squash all "test" commits into single, please?

@mcg-web do you want to double check ok we can merge it once rebased?

@mcg-web
Copy link
Contributor

mcg-web commented Jun 28, 2020

@mavimo LGTM ! After squash and rebase we can merge this, thank you @murtukov !

@murtukov murtukov force-pushed the reconfigure_travis_ci branch 3 times, most recently from de190c9 to 2af2356 Compare June 28, 2020 20:26
@murtukov
Copy link
Contributor Author

@mavimo @mcg-web

I squashed the commits and rebased onto master. Any thoughts why it doesn't cover lib/generator now?

@mcg-web
Copy link
Contributor

mcg-web commented Jun 29, 2020

Everything seem good in the phpunit config file, this issue comes from pcov dir configuration itself

When pcov.directory is left unset, PCOV will attempt to find src, lib or, app in the current working directory, in that order; If none are found the current directory will be used, which may waste resources storing coverage information for the test suite.

I don't use PCOV so I can't help you on this one.

@murtukov
Copy link
Contributor Author

murtukov commented Jun 29, 2020

@mcg-web I am not sure about pcov.directory option. Here is why:

  • Before rebase and squash all checks passed.
  • I tried to do tests with pcov.directory option and it didn't work out for me.
  • I tried to perform coverage tests locally and it ignores the lib/generator both with Xdebug and PCOV in branches master, 0.13, 0.12.
  • Local coverage test works as expected in the branch 0.11, again with both Xdebug and PCOV.

I am really confused right now. I will change into different target branches of this PR to see if all checks pass.

Update: bad idea to chage the target branch of this PR. I will rather open another draft PR to test if coverage works in other branches.

Update: as I expected it covers the lib/generator folder in the 0.11. Seems like this is due to version of PHPUnit, if I downgrade PHPUnit to 7.5 it works as expected. I need further investigation, but have no enough time now. Somebody has to help me with that.

Update: Ok, I found the problem. Will provide the fix commit soon with description.

@murtukov murtukov changed the base branch from master to 0.11 June 29, 2020 13:42
@murtukov murtukov changed the base branch from 0.11 to master June 29, 2020 13:42
@murtukov murtukov mentioned this pull request Jun 29, 2020
- Drop PHP versions below 7.4
- Replace Xdebug with PCOV driver
- Exclude 'vendor' and 'tests' folders from coverage

Perform code-quality fixes:

- Update PHP-CS-Fixer to v2.16.4
- Remove redundant PHPDoc tags
- Disable 'single_line_throw' rule
- Fix invalid @param tags
- Remove unused use statement
- Remove redundant @param tag
- Remove unmatched error patterns
- Add @var tag with type-hints
@murtukov murtukov force-pushed the reconfigure_travis_ci branch from 37c83c6 to 8d0cd09 Compare June 29, 2020 18:17
@murtukov
Copy link
Contributor Author

murtukov commented Jun 29, 2020

@mcg-web You were right, the problem was in the pcov.directory option, but there were another nuances, that confused me.

Notes for the future:

  • When PCOV extension is installed, Xdebug as automatically disabled. And even if you try to perform coverage test with Xdebug, it will use pcov driver, until you explicitly disable it with pcov.enabled=0

  • I was putting the -d option in the wrong place in the command. They should stay before bin/phpunit

    Wrong:

    bin/phpunit --color=always -v --debug -d pcov.enabled=1 -d pcov.directory=. -d pcov.exclude="/(vendor|tests)/" --coverage-clover=build/logs/clover.xml
    

    Correct:

    php -d pcov.enabled=1 -d pcov.directory=. -d pcov.exclude="/(vendor|tests)/" bin/phpunit --color=always -v --debug  --coverage-clover=build/logs/clover.xml
    

    Also the -d option doesn't require a space in between, so the following is also correct:

    php -dpcov.enabled=1 -dpcov.directory=. -dpcov.exclude="/(vendor|tests)/" bin/phpunit --color=always -v --debug  --coverage-clover=build/logs/clover.xml
    
  • The versions of PHPUnit below 8 somehow change behavior of pcov. For example with PHPUnit 7.5 it automatically sets option pcov.directory=., at least it was in my case.

I squashed all commits again. There are no new critical changes, so I assume it doesn't require another review and merge this.

@murtukov murtukov merged commit 631815e into overblog:master Jun 29, 2020
This was referenced Jun 29, 2020
@mavimo
Copy link
Contributor

mavimo commented Jun 29, 2020

@murtukov LGTM!

PS: thx for all the effort you put on this PR! 💪

@murtukov murtukov deleted the reconfigure_travis_ci branch June 30, 2020 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants