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.

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

[FrameworkBundle] Skip redis cache pools test on failed connection #18939

Closed
wants to merge 2 commits into from

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Jun 1, 2016

Q A
Branch? 3.1
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Actually, running the FrameworkBundle tests suit leads to a failure that says:

  1. Symfony\Bundle\FrameworkBundle\Tests\Functional\CachePoolsTest::testRedisCachePools
    Symfony\Component\Cache\Exception\InvalidArgumentException: Redis connection failed: redis://localhost
    [...]
    [...]/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/CachePoolsTest.php:59
    [...]/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/CachePoolsTest.php:30

So I propose to catch this specific exception in case of redis is not (locally) supported, in order to be sure that tests stay green.

@nicolas-grekas
Copy link
Member

There is already a redis server detection, but it checks for 127.0.0.1, then it uses localhost, which might not be the same (e.g. if localhost is [::1]). I suggest to replace all redis://localhost strings in the tests with redis://127.0.0.1 instead.

@chalasr
Copy link
Member Author

chalasr commented Jun 2, 2016

@nicolas-grekas I tried to apply your suggestion, unfortunately it doesn't helped.

In fact the exception doesn't match with \PHPUnit_Framework_Error_Warning, even by removing the if (0 !== strpos('unable to...') { throw $e; } the test continues then fails at the first exception.

Strangely, this error occurs only from the testRedisCachePools, not in the testRedisCustomCachePools. I think I need to investigate what happen in the redis_config.yml.
The catch I added seems to solve it, but the problem may come from the default cache configuration of the framework. WDYT?

@nicolas-grekas
Copy link
Member

Ok, then 👍

@fabpot
Copy link
Member

fabpot commented Jun 3, 2016

Thank you @chalasr.

fabpot added a commit that referenced this pull request Jun 3, 2016
…connection (chalasr)

This PR was squashed before being merged into the 3.1 branch (closes #18939).

Discussion
----------

[FrameworkBundle] Skip redis cache pools test on failed connection

| Q             | A
| ------------- | ---
| Branch?       | 3.1
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Actually, running the FrameworkBundle tests suit leads to a failure that says:

> 1) Symfony\Bundle\FrameworkBundle\Tests\Functional\CachePoolsTest::testRedisCachePools
Symfony\Component\Cache\Exception\InvalidArgumentException: Redis connection failed: redis://localhost
[...]
[...]/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/CachePoolsTest.php:59
[...]/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/CachePoolsTest.php:30

So I propose to catch this specific exception in case of redis is not (locally) supported, in order to be sure that tests stay green.

Commits
-------

0d349dd [FrameworkBundle] Skip redis cache pools test on failed connection
@chalasr chalasr deleted the fix_redis_tests branch August 27, 2016 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants