Problem/Motivation

In PHPUnit 6 \PHPUnit_Util_XML no longer has the cssSelect(). We are trying to update to PHPUnit 6 so that we can test on PHP 7.2 - see #2927806: Use PHPUnit 6 for testing when PHP version >= 7.2. Therefore we need to remove the usage from \Drupal\tour\Tests\TourTestBase. Unfortunately this is a legacy test class and it has no usages.

As part of the necessary things to enable PHP7.2 testing this is critical.

Proposed resolution

Replace \PHPUnit_Util_XML::cssSelect() with xpath and add a test so we can prove we've not broken it.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#8 2932044-8.patch3.27 KBalexpott
#8 3-8-interdiff.txt588 bytesalexpott
#3 2932044-3.patch3.44 KBalexpott
#3 2-3-interdiff.txt455 bytesalexpott
#3 2932044-3.test-only.patch2.24 KBalexpott
#2 2932044-2.patch3.42 KBalexpott
#2 2932044-2.test-only.patch2.22 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 2932044-2.test-only.patch2.22 KB
new 2932044-2.patch3.42 KB

Here's a patch. The test-only patch (unlike most) should be green. Attaching it to show the assumptions the TestBase class makes are not changed by the change to the class.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 2932044-3.test-only.patch2.24 KB
new 2-3-interdiff.txt455 bytes
new 2932044-3.patch3.44 KB

Oops missed the @group.

martin107’s picture

martin107 commented

I have looked over this .. all seems correct and well reasoned.

I will lurk on this issue and come back when tests are passing to RTBC this...

I can see the smallest of small nits

+ * @code
+ * protected $tips = array(
+ * '/foo/bar' => array(
+ * 'data-id' => 'foo',
+ * 'data-class' => 'bar',
+ * ),
+ * );
+ * @endcode

The example code contains examples of array() when [] is prefered.

The last submitted patch, 2: 2932044-2.test-only.patch, failed testing. View results

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

The last submitted patch, 3: 2932044-3.test-only.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 3-8-interdiff.txt588 bytes
new 2932044-8.patch3.27 KB

@martin107 this is a cut-and-paste job from \Drupal\Tests\tour\Functional\TourTestBasic - and the code snippet there is incorrect - but the class we're really aping is \Drupal\Tests\tour\Functional\TourTest so I've just copied the docs from there. Bear in mind that this is totally deprecated code and only exists to test the deprecated TourTestBase class so it definitely has a finite shelf life and also this test is not an extension point so the docs are pointless.

Status: Needs review » Needs work

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

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
martin107’s picture

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

I think this is a good change....

  • catch committed 93b9f58 on 8.5.x 
    Issue #2932044 by alexpott: Remove \PHPUnit_Util_XML::cssSelect() from \...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 93b9f58 and pushed to 8.5.x. Thanks!

Status: Fixed » Closed (fixed)

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