Problem/Motivation

FieldGroupRowsTest fails currently with PostgreSQL as database backend. This is because the test expects the values in a given order but that is not guaranteed because the all have the same timestamp. As this happens over and over again let's change the test.

Proposed resolution

Change test to just check that all expected values are rendered but in no given order.

Remaining tasks

Review patch.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#8 drupal-2480799-fix-field-group-rows-8.patch1.11 KBmradcliffe
#5 drupal-2480799-fix-field-group-rows-5.patch919 bytesmradcliffe
field_group_rows_test_fix.patch1.19 KBbzrudi71

Comments

bzrudi71’s picture

bzrudi71 commented

Not sure if this is the way to go but at least a start, comments welcome :)

dawehner’s picture

dawehner
Primary language German
commented

Can we short by NID and be done? Its more simple anyway to sort by that.

dawehner’s picture

dawehner
Primary language German
commented

Can we short by NID and be done? Its more simple anyway to sort by that.

bzrudi71’s picture

bzrudi71 commented

Unfortunately I guess not. If I don't get it totally wrong we are working with just a single NID ;)

$edit = array(
'title' => $this->randomMachineName(),
$this->fieldName => array('a', 'b', 'c'),
);
$this->drupalCreateNode($edit);
mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
StatusFileSize
new drupal-2480799-fix-field-group-rows-5.patch919 bytes

This patch adds delta for that field in the sort for test_group_rows.

Let's see if mysql has anything to say about it. @jaredsmith will run on new testbot postgresql

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented

Passes SQLite on my local environment.

dawehner’s picture

dawehner
Primary language German
commented

Removing created seems to be something we should really better do here.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
StatusFileSize
new drupal-2480799-fix-field-group-rows-8.patch1.11 KB

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

mradcliffe’s picture

mradcliffe
he/him
Primary language English
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

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
StatusFileSize
new 2480799-8-test-results.txt14.67 KB
new 2480799-8-DefaultViewsTest-randomfail.txt2.83 KB

Ran full Views group test and did not have any regressions. The ViewsDisplayTest in the first result is a random test fail as demonstrated by the subsequent run of that test class.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented

@jaredsmith is running without the patch as well.

erik.erskine’s picture

erik.erskine commented

Tried #8 a few times on a local environment - FieldGroupRowsTest & FieldGroupRowsWebTest all pass.

Have also tried a full run all the views tests - no new failures introduced on PostgreSQL and all the tests passed on MySQL and SQLite.

jaredsmith’s picture

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

I've reviewed this code, and run this patch through the new DrupalCI test infrastructure. It fixes the failing test, and does not cause any additional test failures in the Views testgroup.

Before the patch:
Drupal\views\Tests\Handler\FieldGroupRowsTest 4 passes 2 fails

After the patch:
Drupal\views\Tests\Handler\FieldGroupRowsTest 6 passes

I believe this patch is ready to be committed, so I'm setting it to RTBC.

  • webchick committed cb64bb7 on 8.0.x 
    Issue #2480799 by mradcliffe, bzrudi71, dawehner, ingaro, jaredsmith:...
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Awesome work, all! I was a bit confused on why we were using delta here instead of nid, but #4 points out that this is because the test is only working on one nid.

Committed and pushed to 8.0.x. Thanks!

webchick’s picture

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

Status: Fixed » Closed (fixed)

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