@c960657 opened this Pull Request on April 28th 2018 Contributor

Three tests in Piwik\Tests\Unit\DataAccess\JoinGeneratorTest fail on PHP 7.

PHP 7 contained a change to uasort() and friends that may result in different sorting of values that compare as equal.

The three tests all sort three tables where the two of them do not depend on each other and may thus be returned in order. Hence we should not make assertions on their relative order.

@sgiehl commented on April 28th 2018 Member

Not sure if changing the tests is the right approach in this case. Maybe it would be better to change the JoinGenerator to always return the same order accross all php versions

@tsteur any opinion on that?

@tsteur commented on April 29th 2018 Member

Yes definitely important to change the JoinGenerator to always return the same set. It is very critical to test the full response here as otherwise generated queries will fail.

@sgiehl commented on April 29th 2018 Member

@c960657 I will close the PR. Thanks for it nevertheless.
Maybe you are keen on changing the JoinGenerator to always return the same set?

@c960657 commented on April 29th 2018 Contributor

Sure, I'd be happy to make a new PR. I just need to understand your suggested approach.

I can change the sort to be stable. If I understand correctly, the point of this is just to make testing easier, not because it matters that the joins are sorted differently in PHP 5 and PHP 7, right?

@sgiehl commented on April 29th 2018 Member

I would say the current difference in order between php version does not cause any known problems, so the target would be to make the returned order consistent (and adjust the tests accordingly). But maybe @tsteur is able to say more.

@tsteur commented on April 29th 2018 Member

I would say the current difference in order between php version does not cause any known problems,

Exactly.

so the target would be to make the returned order consistent (and adjust the tests accordingly).

that would be good.
So instead of returning 0 in a usort, we would need to search for the index of a key or item within the original array and check for example whose index is lower or higher and sort it based on the position within the array. Just an example.

@c960657 commented on May 31st 2018 Contributor

I have filed PR #13025 that implements the suggested stable sort.

This Pull Request was closed on April 29th 2018
Powered by GitHub Issue Mirror