Problem/Motivation

Head is failing in DiffRevisionTest::doTestOverviewPager and DiffPluginTest::doTestFieldWithNoPlugin

Proposed resolution

Fix head test failing.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

toncic created an issue. See original summary.

toncic’s picture

There is still one test fail in DiffRevisionTest::doTestOverviewPager.

After creating 11 revisions:

for ($i = 0; $i < 11; $i++) {
      $edit = [
        'revision' => TRUE,
        'body[0][value]' => 'change',
      ];
      $this->drupalPostForm('node/' . $node->id() . '/edit', $edit, t('Save and keep published'));
    }

On first revision page we should see 10 revision but there is not any revision:

$this->drupalGet('node/' . $node->id() . '/revisions');
    $element = $this->xpath('//*[@id="edit-node-revisions-table"]/tbody/tr');
    $this->assertEqual(count($element), 10);
    // Check that the pager exists.
    $this->assertRaw('page=1');

Any idea what is going on here?

toncic’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: fix_test_failing-2887598-2.patch, failed testing. View results

Alan D.’s picture

Priority: Normal » Critical

Failing tests isn't good, particularly if blocking other important issues.

Before retesting;

  • Drupal 8.1.x was the only minor branch with a pass
  • Drupal 8.2.x had one failure
  • Drupal 8.3.x had two failures

I've queued the 4 PHP 5.5 tests with against all Drupal 8.x branches and added PHP 7 test against D8 default branch (currently 4.x).

https://www.drupal.org/node/20492/qa

It is likely that you'll need to add a lot of verbose debugging in the tests locally to actually see what is being produced in each step to figure out what is happening here.

Alan D.’s picture

Not so good.

PHP 5.5 & MySQL 5.5, D8.1 38 pass, 9 fail
PHP 5.5 & MySQL 5.5, D8.2 45 pass, 1 fail

-- I believe Miro (miro_dietiker) only cares about supported core releases, so these can be ignored.

PHP 5.5 & MySQL 5.5, D8.3 43 pass, 3 fail

https://www.drupal.org/pift-ci-job/702812

PHP 5.5 & MySQL 5.5, D8.4 24 pass, 24 fail
PHP 7 & MySQL 5.5, D8.4 24 pass, 24 fail

https://www.drupal.org/pift-ci-job/702809

johnchque’s picture

To keep the tests passing on in previous versions of core, we need to implement something like in #2887759: Update tests with the recent changes.

jhedstrom’s picture

Also failing in DiffRevisionTest::doTestRevisionDiffOverview in 8.4 due to #2068063: Change "Save and keep un-/published" buttons to a "Published" checkbox and an included "Save" button, so the button 'Save and publish' is no longer found.

jhedstrom’s picture

Assigned: toncic » jhedstrom

I'll see if I can get these passing.

jhedstrom’s picture

Assigned: jhedstrom » Unassigned
Status: Needs work » Needs review
FileSize
21.53 KB
22.26 KB

This has the same failure as in #2, but fixes tests for 8.4 using a similar approach to #2068063: Change "Save and keep un-/published" buttons to a "Published" checkbox and an included "Save" button.

I'll take a look at the remaining fail tomorrow if I get a chance.

jhedstrom’s picture

FileSize
1.24 KB
23.5 KB

Oops, I forgot to add the new trait to the patch above.

jhedstrom’s picture

So the remaining failure, where the first page of revisions is empty, for some reason this line (in RevisionOverviewForm::buildForm()) is returning false (specifically, the isRevisionTranslationAffected() bit), and thus the row is not built.

        if ($revision->hasTranslation($langcode) && $revision->getTranslation($langcode)->isRevisionTranslationAffected()) {
jhedstrom’s picture

It seems that isRevisionTranslationAffected checks to see if anything actually changed, so this fixes the test by changing the value of the body field.

Alan D.’s picture

Status: Needs review » Reviewed & tested by the community

Nice work, this looks good to me.

Alan D.’s picture

Status: Reviewed & tested by the community » Fixed

Pushed since the D8 guys must be busy atm. There are a couple other major issues are getting held up with this one :)

Thanks!

Status: Fixed » Closed (fixed)

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