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

[Tests] Added ClockMock to make time-sensitive tests more robust #134

Merged
merged 1 commit into from
Sep 14, 2020

Conversation

mnocon
Copy link
Member

@mnocon mnocon commented Sep 10, 2020

Question Answer
JIRA issue n/a
Type Improvement in tests
Target version 1.0, needs to be adapted in 2.1 and master (1)
BC breaks no
Doc needed no

Failing build: https://travis-ci.org/github/ezsystems/ezplatform-http-cache/jobs/725689032

1) eZ\Publish\Core\MVC\Symfony\Cache\Tests\LocalPurgeClientTest::testPurge

Expectation failed for method name is equal to 'purgeByRequest' when invoked 1 time(s)

Parameter 0 for invocation EzSystems\PlatformHttpCacheBundle\RequestAwarePurger::purgeByRequest(Symfony\Component\HttpFoundation\Request Object (...)) does not match expected value.

Failed asserting that two objects are equal.

--- Expected

+++ Actual

@@ @@

             'SCRIPT_NAME' => ''
             'SCRIPT_FILENAME' => ''
             'SERVER_PROTOCOL' => 'HTTP/1.1'
-            'REQUEST_TIME' => 1599682188
+            'REQUEST_TIME' => 1599682189
             'PATH_INFO' => ''
             'REQUEST_METHOD' => 'PURGE'
             'REQUEST_URI' => '/'

Request::create uses time() method under the hood to set the REQUEST_TIME header. By using the ClockMock (as described in https://symfony.com/doc/3.4/components/phpunit_bridge.html#time-sensitive-tests) we can make sure that the time call in Request class is mocked and we're not hit by this issue again

There are 2 deprecation warnings in current tests:

OK, but incomplete, skipped, or risky tests!
Tests: 3385, Assertions: 3881, Skipped: 162, Incomplete: 4.

Remaining direct deprecation notices (1)

  1x: The eZ\Publish\Core\SignalSlot\Signal\ContentService\RemoveTranslationSignal is deprecated since 6.13 and will be removed in 7.0. Use eZ\Publish\Core\SignalSlot\Signal\ContentService\DeleteTranslationSignal instead
    1x in RemoveTranslationSlotTest::getReceivedSignals from EzSystems\PlatformHttpCacheBundle\Tests\SignalSlot

Other deprecation notices (1)

  1x: The eZ\Publish\Core\Repository\Repository::getCurrentUser method is deprecated (since 6.6, to be removed. Use PermissionResolver::getCurrentUserReference() instead.  Get current user.  Loads the full user object if not already loaded, if you only need to know user id use {@see getCurrentUserReference()}).
    1x in RoleIdentifyTest::testSetIdentity from EzSystems\PlatformHttpCacheBundle\Tests\ContextProvider

I've disabled them as described in https://symfony.com/doc/3.4/components/phpunit_bridge.html#disabling-the-deprecation-helper, because I don't want to introduce too many changes here.

(1) In 2.1 and master the whole request is not compared:
https://github.com/ezsystems/ezplatform-http-cache/blob/2.1/tests/PurgeClient/LocalPurgeClientTest.php
so the test is not time-sensitive, but I think we can keep the dependency for future usages.

TODO:

  • Implement feature / fix a bug.
  • Implement tests + specs and passing ($ composer test)
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@mnocon mnocon force-pushed the fix-time-sensitive-unit-test branch 4 times, most recently from 997077e to db36af5 Compare September 10, 2020 10:31
@mnocon mnocon changed the title [WIP] [Tests] Added ClockMock to make time-sensitive tests more robust [Tests] Added ClockMock to make time-sensitive tests more robust Sep 10, 2020
@micszo micszo merged commit d5793ac into 1.0 Sep 14, 2020
@micszo micszo deleted the fix-time-sensitive-unit-test branch September 14, 2020 10:35
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants