?
avatar csthomas
csthomas
16 Aug 2016

Steps to reproduce the issue

  1. Create a test env for php, php-memcached and hhvm and apply PR #11565.

  2. sudo service memcached restart

  3. View libraries/vendor/phpunit/phpunit/phpunit. It should start like:
    #!/usr/bin/env php

  4. Run libraries/vendor/phpunit/phpunit/phpunit tests/unit/suites/libraries/joomla/cache/storage/JCacheStorageMemcachedTest

    PHPUnit 4.8.24 by Sebastian Bergmann and contributors.
    Warning: The Xdebug extension is not loaded
    No code coverage will be generated.

    .......

    Time: 5.08 seconds, Memory: 8.00MB

    OK (7 tests, 17 assertions)

  5. Edit libraries/vendor/phpunit/phpunit/phpunit and replace first like from php to hhvm.
    Should be: #!/usr/bin/env hhvm

  6. Run libraries/vendor/phpunit/phpunit/phpunit tests/unit/suites/libraries/joomla/cache/storage/JCacheStorageMemcacheTest

    PHPUnit 4.8.24 by Sebastian Bergmann and contributors.
    Warning: The Xdebug extension is not loaded
    No code coverage will be generated.

    Fatal error: [] operator not supported for strings in libraries/joomla/cache/storage/memcache.php on line 194

  7. Run sudo service memcached restart

  8. (The same as point 6) Run libraries/vendor/phpunit/phpunit/phpunit tests/unit/suites/libraries/joomla/cache/storage/JCacheStorageMemcacheTest

    PHPUnit 4.8.24 by Sebastian Bergmann and contributors.
    Warning: The Xdebug extension is not loaded
    No code coverage will be generated.

    .......

    Time: 9.17 seconds, Memory: 28.64MB

    OK (7 tests, 17 assertions)

Expected result

Memcache Test for HHVM should pass on PR #11565

Actual result

but did not pass because it was run after memcached storage did test which left empty array in memcached server.

I suggest to restart memcached server before each memcache/memcached tests.

Summary

Memcache storage test fails if memcached server has value from memcached tests.

https://travis-ci.org/joomla/joomla-cms/jobs/152511380

avatar csthomas csthomas - open - 16 Aug 2016
avatar photodude
photodude - comment - 17 Aug 2016

I agree, it doesn't make a lot of sense that we are not resetting/restarting caching servers prior to each Unit test being run.

The suggested fix might be something to add to my PR relating to memory management of the unit tests. #10685

avatar csthomas
csthomas - comment - 17 Aug 2016

I thought it would solve the problem
but on second thought I would not force users to reset server after change cache handler between memcache and memcached.

Then this issue may be incorrect.
I have plan to add new change to memcache storage and strict check if $index is array.

avatar photodude
photodude - comment - 17 Aug 2016

What you proposed was forcing the unit test to reset the cache handler between unit tests.

Since unit tests should be independent and isolated, resetting the cache handler between the tests makes sense as it maintains the isolation between tests.

(On a side note: on the user end, either a strict check for array and handling that case, or the array_push() I suggested ensures the end users don't run into a fatal error related to MEMCACHE::get returning a string.)

avatar csthomas
csthomas - comment - 17 Aug 2016

If you want to do reset cache then please create (IMHO) a new PR for that.

I not enough familiar with unit tests.

avatar brianteeman brianteeman - change - 17 Aug 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 18 Aug 2016
Category Unit Tests
avatar csthomas csthomas - edited - 18 Aug 2016
avatar csthomas
csthomas - comment - 18 Aug 2016

I close that because PR #11565 fix that problem.

avatar csthomas csthomas - change - 18 Aug 2016
Status New Closed
Closed_Date 0000-00-00 00:00:00 2016-08-18 23:58:21
Closed_By csthomas
avatar csthomas csthomas - close - 18 Aug 2016

Add a Comment

Login with GitHub to post a comment