Problem/Motivation

In #2870453: Convert web tests to browser tests for node module we updated simpletest-based tests to PHPUnit-based ones. However, some were considered out of scope for that issue.

  • Upgrade tests. (got done in a bulk conversion)
  • \Drupal\node\Tests\NodeRevisionsTest needs the contextual filter parts moved to a Javascript test
  • \Drupal\Tests\node\Functional\NodeTypeTest needs \Drupal\system\Tests\Menu\AssertBreadcrumbTrait
  • \Drupal\node\Tests\PagePreviewTest uses drupalPostAjaxForm so that needs to be split to a Javascript test, no issue for that yet on #2809161: Convert Javascript/AJAX testing to use JavascriptTestBase
  • Drupal\node\Tests\Views\NodeContextualLinksTest needs to be converted to a Javascript test

This issue catches up on all those.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#10 2996030-10.patch18.74 KBlendude
#8 2996030-8.patch18.74 KBlendude
#8 interdiff-2996030-5-8.txt9.21 KBlendude
#5 2996030-5.patch8.79 KBlendude
#5 interdiff-2996030-3-5.txt5.2 KBlendude
#3 2996030_3.patch3.25 KBmile23

Comments

Mile23 created an issue. See original summary.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

#2970017: Fix "Drupal\system\Tests\Menu\AssertBreadcrumbTrait is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. Instead, use \Drupal\Tests\system\Functional\Menu\AssertBreadcrumbTrait" is postponed on this issue.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Active » Needs review
StatusFileSize
new 2996030_3.patch3.25 KB

Converts NodeTypeTest. Still needs work for the other tests.

Needed some assertSession() love because it turns out assertNoRaw() isn't a drop-in replacement.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +phpunit initiative

@Mile23 thanks for getting the ball rolling here, looks good so far. NW for the rest!

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs work » Needs review
Issue tags: +Needs followup
StatusFileSize
new interdiff-2996030-3-5.txt5.2 KB
new 2996030-5.patch8.79 KB

Some progress.

NodeContextualLinksTest::testNodeContextualLinks isn't actually testing anything at the moment, the actual assertions are all commented out, so we can just take that out and not lose any coverage. We do need a followup to make sure it gets some proper javascript coverage.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs followup

Here is a follow up #2998393: Add javascript test for NodeContextualLinksTest:: testNodeContextualLinks

The entire contextual links test is weird, not just the one without any assertions.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Reviewed & tested by the community » Needs work

#5 still leaves us with two WTB tests:

  • Drupal\node\Tests\NodeRevisionTest
  • Drupal\node\Tests\PagePreviewTest
lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2996030-5-8.txt9.21 KB
new 2996030-8.patch18.74 KB

This takes care of the remaining tests.

The ajax use is PagePreviewTest was pointless and I've replaced it with normal button clicks.

I've split the contextual links stuff from NodeRevisionTest into a WebDriverTest

Status: Needs review » Needs work

The last submitted patch, 8: 2996030-8.patch, failed testing. View results

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Status: Needs work » Needs review
StatusFileSize
new 2996030-10.patch18.74 KB

Bleh, that was against 8.6.x, now against 8.7.x

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

I don't see any nits to pick, so moving this to RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Committed 15ca693 and pushed to 8.7.x. Thanks!

  • alexpott committed 15ca693 on 8.7.x 
    Issue #2996030 by Lendude, Mile23: Convert web tests to browser tests...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

  • alexpott committed bc6b09f on 8.6.x 
    Issue #2996030 by Lendude, Mile23: Convert web tests to browser tests...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

The patch does not apply to 8.6.x but it does cherry-pick which means it was patch context conflict and not change conflict so I've gone ahead and backported to 8.6.x to keep the tests aligned.

Status: Fixed » Closed (fixed)

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