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

CommentFileSizeAuthor
#13 2887598-13.patch23.51 KBjhedstrom
#13 interdiff-2887598-11-13.txt534 bytesjhedstrom
#11 2887598-11.patch23.5 KBjhedstrom
#11 interdiff-2887598-10-11.txt1.24 KBjhedstrom
#10 2887598-10.patch22.26 KBjhedstrom
#10 interdiff-2887598-02-10.txt21.53 KBjhedstrom
#2 fix_test_failing-2887598-2.patch1.78 KBtoncic

Comments

toncic created an issue. See original summary.

toncic’s picture

toncic commented
Issue summary: View changes
StatusFileSize
new fix_test_failing-2887598-2.patch1.78 KB

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

toncic commented
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

alan d. commented
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

alan d. commented

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

johnchque
Primary language Spanish
Location Bolivia
commented

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

jhedstrom
Primary language English
Location Portland, OR
commented

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

jhedstrom
Primary language English
Location Portland, OR
commented
Assigned: toncic » jhedstrom

I'll see if I can get these passing.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Assigned: jhedstrom » Unassigned
Status: Needs work » Needs review
StatusFileSize
new interdiff-2887598-02-10.txt21.53 KB
new 2887598-10.patch22.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

jhedstrom
Primary language English
Location Portland, OR
commented
StatusFileSize
new interdiff-2887598-10-11.txt1.24 KB
new 2887598-11.patch23.5 KB

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

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

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

jhedstrom
Primary language English
Location Portland, OR
commented
StatusFileSize
new interdiff-2887598-11-13.txt534 bytes
new 2887598-13.patch23.51 KB

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

alan d. commented
Status: Needs review » Reviewed & tested by the community

Nice work, this looks good to me.

alan d.’s picture

alan d. commented
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.