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.

Sign up for GitHub

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

Jump to bottom

EZP-29396: Remove unnecessary test cache decorator #123

Merged
merged 7 commits into from
Jul 12, 2018
Merged

EZP-29396: Remove unnecessary test cache decorator #123

merged 7 commits into from
Jul 12, 2018

Conversation

kmadejski
Copy link
Member

Question Answer
JIRA issue EZP-29396
Bug yes
Target version 1.5 and master

This change is required regarding changes done in kernel in: ezsystems/ezpublish-kernel#2387.
Since test should be working properly with Redis, using of the eZ\Publish\Core\Persistence\Cache\Tests\Helpers\IntegrationTestCacheServiceDecorator is no longer necessary.

@kmadejski kmadejski requested review from andrerom and alongosz July 11, 2018 11:22
@kmadejski kmadejski mentioned this pull request Jul 11, 2018
Merged
7 tasks
alongosz
alongosz approved these changes Jul 11, 2018
View reviewed changes
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

@andrerom
Copy link
Contributor

andrerom commented Jul 11, 2018 •
edited
Loading

@kmadejski Seems we have issue from newer version og phpunit packages on PHP 7.1 and higher, could you check?

ref:

PHP Fatal error: Class 'PHPUnit\Framework\Constraint\Constraint' not found in /home/travis/build/ezsystems/ezplatform-solr-search-engine/vendor/ezsystems/ezpublish-kernel/eZ/Publish/API/Repository/Tests/PHPUnitConstraint/ValidationErrorOccurs.php on line 19

@kmadejski
Copy link
Member Author

@andrerom yes, actually, I'm investigating it right now.

@alongosz
Copy link
Member

@kmadejski we just need to upgrade PHPUnit to version ^7.0 because we use Kernel Setup Factory for integration tests, which uses new version of PHPUnit. Composer does not catch this, because it's require-dev package.

@kmadejski
Copy link
Member Author

kmadejski commented Jul 11, 2018 •
edited
Loading

@alongosz correct 🙂 (549170e). Let's see whether the test will fail again.

@kmadejski kmadejski force-pushed the ezp_29396 branch 2 times, most recently from f944905 to 7993d21 Compare July 12, 2018 10:57
andrerom
andrerom reviewed Jul 12, 2018
View reviewed changes
.travis.yml Outdated
- php: 5.6
env: TEST_CONFIG="phpunit-integration-legacy-solr.xml" SOLR_VERSION="6.6.0" CORES_SETUP="single" SOLR_CORES="collection1"

- php: 5.6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage for 7.1 on Solr 6 is missing now

maybe also just move COMPOSER_REQUIRE="ezsystems/ezpublish-kernel:~6.7.4@dev" to 7.0 job to avoid the shuffling of the list here also.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage for 7.1 on Solr 6 is missing now

Right, I'll add this, but this can be only against ezpublish-kernel master.

maybe also just move COMPOSER_REQUIRE="ezsystems/ezpublish-kernel:~6.7.4@dev" to 7.0

Unfortunately, we can't run test with PHP >= 7.0 and ezsystems/ezpublish-kernel:~6.7.4@dev because of the problem with typehints for return values in the new PHPUnit (ref: https://travis-ci.org/ezsystems/ezplatform-solr-search-engine/jobs/403021480).

Copy link
Contributor

@andrerom andrerom Jul 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But PHPUnit 7.x requires PHP 7.1 doesn't it? ref: https://phpunit.de/announcements/phpunit-7.html

Copy link
Member Author

@kmadejski kmadejski Jul 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you are right. Moved tests against kernel 6.7 to PHP 7.0.

@andrerom andrerom merged commit 55a02cf into 1.5 Jul 12, 2018
@andrerom andrerom deleted the ezp_29396 branch July 12, 2018 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrerom andrerom andrerom approved these changes

@alongosz alongosz alongosz approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kmadejski @andrerom @alongosz