Views does not respect order of area handlers

This patch looks perfect, but given it fixes a bug, we should have a test.

Adding tests.

The setup of the test in general looks pretty good. Here are some comments, but after that this is totally perfect.

1. +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_area_order.yml
   @@ -0,0 +1,53 @@
   +        entity_block_1:
   +          field: entity_block
   +          id: entity_block
   +          table: views
   +          target: 'bartik_powered'
   +          view_mode: full
   +          plugin_id: entity
   +        entity_block_2:
   +          field: entity_block
   +          id: entity_block
   +          table: views
   +          target: 'bartik_branding'
   +          view_mode: full
   +          plugin_id: entity
   

   In order to test that properly we should add position: for the two cases.

2. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaOrderTest.php
   @@ -0,0 +1,79 @@
   +/**
   + * @file
   + * Contains \Drupal\Tests\views\Kernel\Handler\AreaOrderTest.
   + */
   

   We can remove/drop that bit now.

3. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaOrderTest.php
   @@ -0,0 +1,79 @@
   +    $this->executeView($view);
   +    $rendered_view = $view->render();
   +    $output = $renderer->renderRoot($rendered_view);
   

   You should be able to achieve the same by just using $output = $this->render($view->buildRenderable())

4. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaOrderTest.php
   @@ -0,0 +1,79 @@
   +    $this->assertTrue($position_powered != 0, 'ID bartik-powered found.');
   +    $this->assertTrue($position_branding != 0, 'ID bartik-branding found');
   +    $this->assertTrue($position_powered < $position_branding, 'Block bartik-powered is positioned before block bartik-branding');
   

   Just a quick comment: You could have used assertNotEquals($position_powered, 0) and assertGreaterThan. This makes it a bit easier to read at the end.

Updated. I prefer the test with <. "assertGreaterThan" requires to swap the order which confuses me...

@k4v
Okay sure, no worries

#9.1 hasn't been addressed - is position saved to the view - I think it is. Also I think entity_block_2 should be positioned before entity_block_1 as that is more of a test. Note that changing the position should not affect the order of the blocks in the view.

#9.1 hasn't been addressed - is position saved to the view - I think it is. Also I think entity_block_2 should be positioned before entity_block_1 as that is more of a test. Note that changing the position should not affect the order of the blocks in the view.

@k4v explained me that in person. The position is not setable via configuration, but rather just the order in the files is taken into account there. Given that the testcase itself makes totally sense IMHO, but yeah I agree using the keys in a different order would be nice, just to ensure that we don't sort by keys.

I changed the labels in the yml. As dawehner mentioned, I cannot set 'position' in the yml. The order of the blocks is taken from the position in the yml.

Committed f21cdec and pushed to 8.1.x and 8.2.x. Thanks!

Can we get a followup to add a position key instead of reordering whole sections - doing that making looking configuration changesets much easier. atm the diff is unreadable.

FILE: ...al/core/modules/views/tests/src/Kernel/Handler/AreaOrderTest.php
----------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------
 65 | ERROR | [x] No space found after comma in function call
 65 | ERROR | [x] Expected one space after the comma, 0 found
 66 | ERROR | [x] No space found after comma in function call
 66 | ERROR | [x] Expected one space after the comma, 0 found
 75 | ERROR | [x] The closing brace for the class must have an empty
    |       |     line before it
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 34ms; Memory: 4.25Mb

Fix all of this on commit.