Core Language sort() should not work, needs to use name instead of default title

talking with Berdir in irc...

the things to sort get casted to array, so they might have a 'name' key, but they definitely do not have a title one.

so I'm still thinking we are lacking a test that has sorts languages with same weights.

guessing LanguageUnitTest is the place to add a sort test.

I'm sure this isn't the "right" way for this test to be... but it fails. :)

guess next step is to try and fix the bug with something like #2151223-20: Add a method to SortArray to sort by weight and title
telling it to use the name (instead of the default title)

a test that uses a data provider.
( looked at http://phpunit.de/manual/3.7/en/writing-tests-for-phpunit.html#writing-t...
and the example in SortArrayTest )

3 passes, 1 fail.
4 tests, for covering more cases than just the one where weight is tied and label was ignored.

@martin107 sounds like you are thinking the same thing I was.
The original title was: Core Language sort() should not work. sortByWeightAndTitleKey is only for things that implement the \ArrayAccess Interface
because sortByWeightAndTitleKey and the array sorts it calls, take arrays as input.

But in #1, I tried to pass on what Berdir said: that because (there is no type on the parameters in the function definition and) the language objects are cast as arrays, the language objects are some kind of array in the function, and php makes array elements keyed by the property names. (I have not verified with a debugger...)

I'm going to try wrapping and telling it to use the 'name' key.

sigh. this fails too, and I had hoped it would pass.

Interdiff to 6.

[edit: for anyone following along, this runs just this one test]
./vendor/bin/phpunit --filter=testSortArrayOfLanguages

uh.. data set #3 is the one that is failing...

--- Expected Other LanguageUnitTest.php 0 Drupal\Tests\Core\Language\LanguageUnitTest->testSortArrayOfLanguages with data set #3()

that is not the one I thought would be failing.

I think I didn't write the tests correctly... cause after "fixing" it, the fails are on the data sets where I thought they were already in order.

changed the test to be an array keyed by langcode...
and now the tests pass without the fix. :(

clearly I'm doing something wrong.
Cause when I change the order of my expected...

diff --git a/core/tests/Drupal/Tests/Core/Language/LanguageUnitTest.php b/core/tests/Drupal/Tests/Core/Language/LanguageUnitTest.php
index 769b977..5a803dc 100644
--- a/core/tests/Drupal/Tests/Core/Language/LanguageUnitTest.php
+++ b/core/tests/Drupal/Tests/Core/Language/LanguageUnitTest.php
@@ -156,8 +156,8 @@ public function providerTestSortArrayOfLanguages() {
           [$language10B->getId()] => $language10B,
         ),
         array(
-          [$language10A->getId()] => $language10A,
           [$language10B->getId()] => $language10B,
+          [$language10A->getId()] => $language10A,
         ),
       ),
       // Set up data, tied by weight, out of order by name.

the tests still pass.

UGH. I think the test trying to use the data provider is comparing two empty arrays. and that is why it is passing.

ok. I have two things to verify... that any test is working, and how to get the provider to work.

Here I went back to the non-provider to see if I can tell if two arrays are in different orders.

The test can... when calling sort though, the one that starts in the wrong order gets sorted correct, and the one that starts in the correct order gets sorted wrong.
Which is fine.. if sort is broke in that way... and I can get sort to work as expected.

testDirectSortArrayOfLanguagesWrong()
testDirectSortArrayOfLanguagesCorrect()
are the only two tests doing what I expect them to.

trying that now.
Note this patch has a bunch of debugging stuffs in it.
No interdiff cause it's messy anyway.

With the fix, both the correct and wrong case pass.

in my provider I was setting the array wrong, with brackets around the index. :/

testSortArrayOfLanguages() is only giving me one fail right now without the fix...
[edit: that is ok. the data is: in order with different weights, out of order with different weights.. and those two should work with the current sort.
then the other two tests have weight tied, so it should force a sort on the title/name]

I think now, I might think about how this should actually be tested.

files this time.

updated issue summary.

this one takes out the direct test (no phpunit data provider).

So now there is just one new test:
testSortArrayOfLanguages()
and the dataprovider for it.

ok. #23 is looking pretty good.

next step is to try making a patch that runs the entity query test 50 time.... without uploading 50 patches and running the full test suite each time.

Making a note of how to do that.... (thanks @sun)
In run-tests.sh, replace the line $test_list = simpletest_script_get_test_list(); with $test_list = array_fill(0, 50, 'Drupal\Test\To\Repeat');

We did the repeated test to make sure we are not introducing something like #2254011: Drupal\system\Tests\Entity\EntityQueryTest random failure in head.

@penyaskito wow! it has been a while since I looked at that whole patch on that issue!

I'm not totally sure it's a complete dup.

I thought the other was changing it to sort by label.
Which still might change what this is doing.

Hopefully, this will just get head doing what it should have, and then we can return to #2239399: Languages should be sorted by label instead of title

I'm still perplexed as to why there were not random fails in the run of 50 tests above...
*maybe* this *fixes* the random fails?

I'll do that now.

ran the phpunit test locally, and without the fix, 3 passes and 1 fail, with the fix, 4 passes.
same as in #38, fail is in data set #2. where the they are out of order by weight.

I expected head to pass on that one, and to fail on one of the tests sorting by title (name).

interdiff.

oh, good. it was the third data set that was failing (which is data set #2, since it starts counting at zero. Thanks @alexpott).
that is the set where the language title/name was *in order*. Note this means it was reordering the one that was out of order... this is because it was tieing on title (cause there was no title) and ties have unpredictable behavior.

phew.
Updated my comments so they start counting at dataset #0.

Note also that --tap is helpful (Thanks again @alexpott)

so (for those new to phpunit, and for my future self) I ran the test with:
./vendor/bin/phpunit --filter=testSortArrayOfLanguages
and
./vendor/bin/phpunit --filter=testSortArrayOfLanguages --tap

with sanity restored, I think this is ready.

thanks! done.

sorry. looks like I lost the change from 41.

great! thanks.
Moving to #2239399: Languages should be sorted by label instead of title