Support multidimensional and associative arrays when canonicalize

[alexndlm]

Fix:

• ArrayComparator#assertEquals(), canonicalizing stops it from comparing array keys #77

Relates:

• Canonicalize by sorting keys instead of values phpunit#4455
• ArrayComparator ignores order #44

[alexndlm]

Would you please fix CI or suggest how to do this?

[sebastianbergmann]

Would you please fix CI or suggest how to do this?

Please rebase against main.

[alexndlm]

Thanks. Done.

[codecov]

Codecov Report

Merging #108 (501f687) into main (a261237) will increase coverage by 0.05%.
Report is 20 commits behind head on main.
The diff coverage is 100.00%.

❗ Current head 501f687 differs from pull request most recent head bc39224. Consider uploading reports for the commit bc39224 to get more accurate results

@@             Coverage Diff              @@
##               main     #108      +/-   ##
============================================
+ Coverage     98.14%   98.20%   +0.05%     
- Complexity      118      124       +6     
============================================
  Files            14       14              
  Lines           324      334      +10     
============================================
+ Hits            318      328      +10     
  Misses            6        6              

Files	Coverage Δ
src/ArrayComparator.php	100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[cosmastech]

If an element of the parent $array is an array, won't it get passed to the ArrayComparator@assertEquals() method? It seems like we wouldn't need to recursively sort here if that's true.

[alexndlm]

Recursion is needed. After all, nesting can be as big as you want, for example:

$a = [
    'k' => [
        'h' => [
            'n' => [
                ....
            ],
        ],
    ],
];

[cosmastech]

Yes, but assertEquals() is going to loop through each sub-element of the array and then call Comparator@assertEquals() on that.

If the sub-element is an array, that means it's going to get passed to ArrayComparator@canonicalize() again.

So you're sorting everything, and then sorting it again after creating the factory on lines 70-71.

Do I have that wrong?

[alexndlm]

I finally got it. It's brilliant!

[senaria]

@cosmastech would you be so nice and review this PR and if its ok merge it? Would be really neat to have this fix in. thanks in advance

[cosmastech]

@cosmastech would you be so nice and review this PR and if its ok merge it? Would be really neat to have this fix in. thanks in advance

I don't see an issue with the implementation, however, I think it still needs reviewed and merged by the maintainer.

[senaria]

Thanks @cosmastech for answering so fast back.

@sebastianbergmann would you be so nice and merge it in if it is ok to you. Thanks alot

[localheinz]

Should we traverse into elements here when they are arrays as well?

[alexndlm]

No, we only want to sort the current array at this point.