@mattab opened this Issue on February 10th 2015 Owner

The goal of this issue is to investigate the system test test_GetCounters_ShouldOnlyReturnResultsOfLastMinutes which randomly fails the build (maybe depending on the speed of running the ci build? or the time? not sure)

Examples where the test failed the CI jobs:

1) Piwik\Plugins\Live\tests\Integration\APITest::test_GetCounters_ShouldOnlyReturnResultsOfLastMinutes

Failed asserting that two arrays are equal.

--- Expected

+++ Actual

@@ @@

Array (

0 => Array (

- 'visits' => 24

+ 'visits' => 30

'actions' => 60

'visitors' => 20

'visitsConverted' => 40

)

)
1) Piwik\Plugins\Live\tests\Integration\APITest::test_GetCounters_ShouldOnlyReturnResultsOfLastMinutes

Failed asserting that two arrays are equal.

--- Expected

+++ Actual

@@ @@

Array (

0 => Array (

- 'visits' => 24

- 'actions' => 60

- 'visitors' => 20

- 'visitsConverted' => 40

+ 'visits' => '28'

+ 'actions' => '60'

+ 'visitors' => '20'

+ 'visitsConverted' => '40'

)

)
@tsteur commented on February 10th 2015 Owner

I think it uses the current timestamp so in case around midnight it might create some users for the previous day etc.

@mnapoli commented on March 23rd 2015 Member

It has been some time that system tests have not been failing randomly from what I've seen, maybe we can close this one? (or maybe I've just missed those random failures?)

@tsteur commented on March 23rd 2015 Owner

It is still happening all the time see eg yesterday https://travis-ci.org/piwik/piwik/jobs/55422165#L1021

It just depends on the time see my previous comment. It uses the current timestamp so it cannot work.

Unless there was a change?

@diosmosis commented on March 23rd 2015 Member

If this is still broken, here's an idea to fix: move Date::factory to class called DateFactory. Then access the class through StaticContainer, then override in test DI Config with a mock factory that lets us override the date. What do you think?

@diosmosis commented on March 23rd 2015 Member

Override the now/today date*

@mnapoli commented on March 23rd 2015 Member

Override the now/today date*

I've seen this needed in many places, done through private methods overridden using reflection in tests for example. So maybe that could be worth the trouble.

@mattab commented on September 11th 2015 Owner

Still happening https://travis-ci.org/piwik/piwik/jobs/79763078 - somehow we need to fix this random test, or skip it around midnight - in this build the bug only appeared on the MYSQLI and not on PDO

@tsteur commented on September 11th 2015 Owner

The method to fetch counter actually uses time() and not Date::now() but Date::now() uses time() as well.

Maybe we could use time via DI where needed so it returns current time but one can override it to a fixed time. Did something similar in another plugin just recently:

    'MyPlugin.currentTime' => DI\factory(function () {
        return time();
    }),

It's just important that this one is by default not cached (in my case I needed it only once anyway). Of course we could also create a class etc to get current time() and others and replace that class if needed. Just went with above solution for simplicity

@mattab commented on September 14th 2015 Owner

Maybe we could use time via DI where needed so it returns current time but one can override it to a fixed time.

+1

@sgiehl commented on November 25th 2017 Member

Haven't seen it failing in the last months. Did anyone else? Otherwise guess we could close it now

Powered by GitHub Issue Mirror