Problem/Motivation

FieldGroupRowsWebTest fails currently with PostgreSQL as database backend.

The test fails because the order of results is not guaranteed.

Proposed resolution

- Fix order by adding the delta for the field to the view sort.
- Removes created from the view sort.

Remaining tasks

Patch needs review.

User interface changes

None.

API changes

None.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category PostgreSQL Bug
Issue priority Major
Prioritized changes PostgreSQL fix that changes test data only.
Disruption None
CommentFileSizeAuthor
#7 drupal-2480959-fix-test-ungroup-rows-7.patch1.13 KBmradcliffe
#2 drupal-2480959-fix-test-ungroup-rows-2.patch943 bytesmradcliffe
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner for Drupal Association commented

Well, in that case I'd always suggest to add the most minimal order, which is NID almost ever ...

mradcliffe’s picture

mradcliffe
he/him
Primary language English
CreditAttribution: mradcliffe commented
Issue summary: View changes
Status: Active » Needs review
FileSize
drupal-2480959-fix-test-ungroup-rows-2.patch943 bytes
mradcliffe’s picture

mradcliffe
he/him
Primary language English
CreditAttribution: mradcliffe commented

Passes SQLite on my local environment.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner for Drupal Association commented 
+++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_ungroup_rows.yml
@@ -126,6 +126,18 @@ display:
+        delta:
+          id: delta
+          table: node__field_views_testing_group_rows
+          field: delta
+          relationship: none
+          group_type: group
+          admin_label: ''
+          order: ASC
+          exposed: false
+          expose:
+            label: ''
+          plugin_id: standard
   page_1:

It seems to be for me that the easiest way to fix that would be to replace the created one with sorting by field_views_testing_group_?

jaredsmith’s picture

jaredsmith CreditAttribution: jaredsmith at Bluehost commented
Status: Needs review » Reviewed & tested by the community

I can confirm that this patch indeed fixes the failing test in Drupal\views\Tests\Handler\FieldGroupRowsWebTest. I've also reviewed the patch manually, and while I don't pretend to understand everything, it looks quite straightforward. Marking as RTBC.

Before the patch:
Drupal\views\Tests\Handler\FieldGroupRowsWebTest 9 passes 1 fails

After the patch:
Drupal\views\Tests\Handler\FieldGroupRowsWebTest 10 passes

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Needs review

Doesn't look like #4 has been addressed.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
CreditAttribution: mradcliffe commented
Issue summary: View changes
Issue tags: -Needs issue summary update
FileSize
drupal-2480959-fix-test-ungroup-rows-7.patch1.13 KB

I wasn't able to generate an interdiff with this patch. Since this changed the sort, we need to run through tests again.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
CreditAttribution: mradcliffe commented

I was running views test group thru drupalci, but drupalci just stopped halfway. The two tests passed though.

Drupal\views\Tests\Handler\FieldGroupRowsTest 6 passes
Drupal\views\Tests\Handler\FieldGroupRowsWebTest 10 passes

jaredsmith’s picture

jaredsmith CreditAttribution: jaredsmith at Bluehost commented
Status: Needs review » Reviewed & tested by the community

I have reviewed the code in the patch again, as well as tested it again using the new DrupalCI infrastructure.

Before the patch:
Drupal\views\Tests\Handler\FieldGroupRowsWebTest 9 passes 1 fails

After the patch:
Drupal\views\Tests\Handler\FieldGroupRowsWebTest 10 passes

Additionally, it causes no other regressions in the views test suite. As such, I'm setting this back to RTBC.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 6f4db4b on 8.0.x 
    Issue #2480959 by mradcliffe, dawehner, jaredsmith, bzrudi71: PostgreSQL...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.