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

IndexedByDate.getDataTableFromNumeric() lack of 'order by' breaks integration tests #2154

Closed
julienmoumne opened this issue Mar 6, 2011 · 2 comments
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Milestone

Comments

@julienmoumne
Copy link
Member

I have a failing integration test concerning test_TwoVisitors_twoWebsites_differentDays__VisitFrequency.get_day.xml

Expected is

            <nb_uniq_visitors_returning>1</nb_uniq_visitors_returning>
            <nb_visits_returning>1</nb_visits_returning>
            <nb_actions_returning>4</nb_actions_returning>
            <max_actions_returning>4</max_actions_returning>
            <sum_visit_length_returning>900</sum_visit_length_returning>
            <bounce_rate_returning>0%</bounce_rate_returning>
            <nb_actions_per_visit_returning>4</nb_actions_per_visit_returning>
            <avg_time_on_site_returning>900</avg_time_on_site_returning>

I have


            <max_actions_returning>4</max_actions_returning>
            <nb_actions_returning>4</nb_actions_returning>
            <nb_uniq_visitors_returning>1</nb_uniq_visitors_returning>
            <nb_visits_returning>1</nb_visits_returning>
            <sum_visit_length_returning>900</sum_visit_length_returning>
            <bounce_rate_returning>0%</bounce_rate_returning>
            <nb_actions_per_visit_returning>4</nb_actions_per_visit_returning>
            <avg_time_on_site_returning>900</avg_time_on_site_returning>

Markups are not in the same order.

There is no 'order by' clause at https://github.com/piwik/piwik/blob/master/core/Archive/Array/IndexedByDate.php#L108 so it's not clear which order should be tested.

Same is true for test_TwoVisitors_twoWebsites_differentDays__VisitsSummary.get_day.xml.

@robocoder
Copy link
Contributor

There's a ksort on timestamps on line 136. I suppose we can either:

  • at line 141, insert a ksort($contentArray($timestamp)) inside the foreach loop, or
  • remove the ksort and add an "ORDER BY date1,name".

Not sure how much peak memory usage it'll reduce, but I'd go with the latter.

@julienmoumne
Copy link
Member Author

(In [4051]) fixes #2154 - ordering getDataTableFromNumeric query by date and name & updating expected output

@julienmoumne julienmoumne added this to the Piwik 1.2.1 milestone Jul 8, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

No branches or pull requests

2 participants