Convert web tests to browser tests for rdf module

+++ b/core/modules/rdf/tests/src/Functional/GetNamespacesTest.php
@@ -0,0 +1,36 @@
+    $this->assertTrue(strpos($prefix, 'rdfs: http://www.w3.org/2000/01/rdf-schema#') !== FALSE, 'A prefix declared once is displayed.');
+    $this->assertTrue(strpos($prefix, 'foaf: http://xmlns.com/foaf/0.1/') !== FALSE, 'The same prefix declared in several implementations of hook_rdf_namespaces() is valid as long as all the namespaces are the same.');
+    $this->assertTrue(strpos($prefix, 'foaf1: http://xmlns.com/foaf/0.1/') !== FALSE, 'Two prefixes can be assigned the same namespace.');
+    $this->assertTrue(strpos($prefix, 'dc: http://purl.org/dc/terms/') !== FALSE, 'When a prefix has conflicting namespaces, the first declared one is used.');

If you really want to use strpos you can use $this->assertContains instead which has some better readability.

I use -M and --find-copies-harder but it don't see GetNamespacesTest.php is moving and changed.

The two files are pretty different, so I am not surprised they don't count as a move:

3c3
< namespace Drupal\rdf\Tests;
---
> namespace Drupal\Tests\rdf\Functional;
5c5
< use Drupal\simpletest\WebTestBase;
---
> use Drupal\Tests\BrowserTestBase;
13c13
< class GetNamespacesTest extends WebTestBase {
---
> class GetNamespacesTest extends BrowserTestBase {
29,47c29,33
<     $element = $this->xpath('//html[contains(@prefix, :prefix_binding)]', [
<       ':prefix_binding' => 'rdfs: http://www.w3.org/2000/01/rdf-schema#',
<     ]);
<     $this->assertTrue(!empty($element), 'A prefix declared once is displayed.');
< 
<     $element = $this->xpath('//html[contains(@prefix, :prefix_binding)]', [
<       ':prefix_binding' => 'foaf: http://xmlns.com/foaf/0.1/',
<     ]);
<     $this->assertTrue(!empty($element), 'The same prefix declared in several implementations of hook_rdf_namespaces() is valid as long as all the namespaces are the same.');
< 
<     $element = $this->xpath('//html[contains(@prefix, :prefix_binding)]', [
<       ':prefix_binding' => 'foaf1: http://xmlns.com/foaf/0.1/',
<     ]);
<     $this->assertTrue(!empty($element), 'Two prefixes can be assigned the same namespace.');
< 
<     $element = $this->xpath('//html[contains(@prefix, :prefix_binding)]', [
<       ':prefix_binding' => 'dc: http://purl.org/dc/terms/',
<     ]);
<     $this->assertTrue(!empty($element), 'When a prefix has conflicting namespaces, the first declared one is used.');
---
>     $prefix = $this->xpath('//.')[0]->getAttribute('prefix');
>     $this->assertTrue(strpos($prefix, 'rdfs: http://www.w3.org/2000/01/rdf-schema#') !== FALSE, 'A prefix declared once is displayed.');
>     $this->assertTrue(strpos($prefix, 'foaf: http://xmlns.com/foaf/0.1/') !== FALSE, 'The same prefix declared in several implementations of hook_rdf_namespaces() is valid as long as all the namespaces are the same.');
>     $this->assertTrue(strpos($prefix, 'foaf1: http://xmlns.com/foaf/0.1/') !== FALSE, 'Two prefixes can be assigned the same namespace.');
>     $this->assertTrue(strpos($prefix, 'dc: http://purl.org/dc/terms/') !== FALSE, 'When a prefix has conflicting namespaces, the first declared one is used.');

Attached is the straight move w/ rename+replace. It is going to fail, which concerns me. The same xpath expression that the test generates works in Chrome element inspect, which may mean we don't have a 1-to-1 conversion with xpath in WTB and BTB (and I don't see why this xpath is failing in BTB).

Go, testbot go.

OK, I think the problem is that WTB::xpath() and BTB::xpath() work from different root elements (see http://mink.behat.org/en/latest/guides/traversing-pages.html).

This converts everything to use CSS traversal to get around the clunky xpath that you now have to use.

So, we may need a followup about BTB::xpath()...

Also noticed that Drupal\rdf\Tests\Field\TestDataConverter is orphaned, and not used in core (well, PHPStorm says so and it is smarter than me). Should we add a @deprecated to it in this patch? It's the only file left in core/modules/rdf/src/Tests now.

Created #2864201: XPath in WebTestBase and BrowserTestBase use different roots.

Also noticed that Drupal\rdf\Tests\Field\TestDataConverter is orphaned, and not used in core (well, PHPStorm says so and it is smarter than me). Should we add a @deprecated to it in this patch? It's the only file left in core/modules/rdf/src/Tests now.

I actually believe \Drupal\Tests\rdf\Kernel\Field\FieldRdfaDatatypeCallbackTest is using that ... you know callables :)

That's what I get for trusting Find Usages and not searching for the string...

Should we move that file to live alongside the test that uses it? I suspect that was missed during #2684095: Convert field kernel tests to KernelTestBaseTNG. Or is that out of scope?

Should we move that file to live alongside the test that uses it?

I think keeping the file where it is for now is totally cool. I actually like to not have tests and helper stuff in the same directory, but we could always deal with that in a followup, if you actually care.

No string feelings one way or another. Let's commit this.

1. +++ /dev/null
   similarity index 99%
   rename from core/modules/rdf/src/Tests/CommentAttributesTest.php
   
   rename from core/modules/rdf/src/Tests/CommentAttributesTest.php
   rename to core/modules/rdf/tests/src/Functional/CommentAttributesTest.php
   
   +++ b/core/modules/rdf/tests/src/Functional/CommentAttributesTest.php
   @@ -1,6 +1,6 @@
   -namespace Drupal\rdf\Tests;
   +namespace Drupal\Tests\rdf\Functional;
   

   This is still extending from a WebTestBase test... \Drupal\comment\Tests\CommentTestBase

2. +++ b/core/modules/rdf/tests/src/Functional/GetNamespacesTest.php
   similarity index 98%
   rename from core/modules/rdf/src/Tests/ImageFieldAttributesTest.php
   
   rename from core/modules/rdf/src/Tests/ImageFieldAttributesTest.php
   rename to core/modules/rdf/tests/src/Functional/ImageFieldAttributesTest.php
   
   +++ b/core/modules/rdf/tests/src/Functional/ImageFieldAttributesTest.php
   @@ -1,6 +1,6 @@
   -namespace Drupal\rdf\Tests;
   +namespace Drupal\Tests\rdf\Functional;
   

   This doesn't look right. I think we also need to update \Drupal\image\Tests\ImageFieldTestBase to point at the browsertestbase version of the base test.

#13-1 means we need to postpone on #2866513: Convert web tests to browser tests for comment module.

#13-2 is an easy switch, but the test fails b/c TestFileCreationTrait::drupalGetTestFiles() doesn't exist for BTB, so that test needs to be adjusted.

This patch will fail, but does the class switches from #13, but not the other fix.

And we are double-postponed, #2863267: Convert web tests of views

I think you meant #2863626: Convert web tests to browser tests for image module?

No, I did mean #2863267: Convert web tests of views. That issue is blocking #2866513: Convert web tests to browser tests for comment module, which is blocking this one. I suspect we may also be blocked on the image module; I kinda gave up after digging through the comment-related tests and noticing we are at least double postponed (which kinda of makes sense considering what rdf.module does).

A right, I figured it wasn't directly postponed on views but via comment.

I've been working the issue queue and changing the scope for a lot of issues so they can be unpostponed but for the rdf module it doesn't make sense. Will probably take a while before we can RTBC this one..

IMHO we should get the conversations in, and then make a final run for all the views related test classes. I strongly recommend for running things in parallel vs. serial :)

@dawehner: I agree completely but in this case there's not much left if we split all the tests that require another base class.

Fair point :)

Updated summary to show that this issue is blocked by #2866513: Convert web tests to browser tests for comment module

Work can re-start now that #2866513: Convert web tests to browser tests for comment module has landed

Here we go with the patch.

There's still one class left in the src/Tests namespace Drupal\rdf\Tests\Field\TestDataConverter which is used by the Drupal\Tests\rdf\Kernel\Field\FieldRdfaDatatypeCallbackTest This class conversion seems unrelated to this issue. Do we want to deal it here or in follow-up?

I'm going to take a look at this issue and see if I can resolve some of these failing tests.

I think this may be hard blocked on #2863626: Convert web tests to browser tests for image module now.

@mpdonadio you may be right. I am addressing the failing tests in Drupal\Tests\rdf\Functional\GetNamespacesTest for now.

Targeted attributes of the html element are made a little bit more tricky by //html being prefixed onto each xpath query. There may be a better way to do this but these changes will perform the appropriate check.

Switching to Needs review to trigger testbot

I have screwed up the patch in #29. I will resupply.

I'm resupplying the patch intended for #29. I performed the diff for 8.4.x rather than 8.5.x.

Triggering testbot.

I introduced some whitespace into my last patch. Resupplying.

We need to postpone on #2863626: Convert web tests to browser tests for image module

Doing this conversion has thrown up a few bugs in the File and Image test base conversions. No interdiff because it has been a while.

Whoops missed Image test base fix.

Whoops wrong namespace.

1. +++ b/core/modules/image/tests/src/Functional/ImageFieldTestBase.php
   @@ -87,10 +87,10 @@ public function uploadNodeImage($image, $field_name, $type, $alt = '') {
   diff --git a/core/modules/rdf/src/Tests/GetNamespacesTest.php b/core/modules/rdf/src/Tests/GetNamespacesTest.php
   
   diff --git a/core/modules/rdf/src/Tests/GetNamespacesTest.php b/core/modules/rdf/src/Tests/GetNamespacesTest.php
   deleted file mode 100644
   

   Nice merging!

2. +++ b/core/modules/rdf/tests/src/Functional/GetRdfNamespacesTest.php
   @@ -22,6 +22,25 @@ class GetRdfNamespacesTest extends BrowserTestBase {
   +    // We have to use the find() method on the driver directly because //html is
   +    // prepended to all xpath queries otherwise.
   

   Hehe and then all the find()s start with //html anyway :).

Great clean minimal conversion, completely cleans out the rdf/src/Tests dir, nice!

@vaplas nice changes!

+++ b/core/modules/rdf/tests/src/Functional/ImageFieldAttributesTest.php
@@ -14,6 +15,10 @@
+  use TestFileCreationTrait {
+    getTestFiles as drupalGetTestFiles;
+  }
+

Should this go into ImageFieldTestBase too? I think so.

Additional changes look good and feedback has been addressed.

Rerolled... well git did...

First, rewinding head to replay your work on top of it...
Applying: Finish RDF test conversion
Using index info to reconstruct a base tree...
M	core/modules/shortcut/tests/src/Functional/ShortcutTranslationUITest.php
Falling back to patching base and 3-way merge...
Auto-merging core/modules/rdf/tests/src/Functional/ImageFieldAttributesTest.php
Auto-merging core/modules/rdf/tests/src/Functional/CommentAttributesTest.php
Auto-merging core/modules/rdf/tests/rdf_test/src/TestDataConverter.php
Removing core/modules/rdf/src/Tests/GetNamespacesTest.php
Applying: Whoops
Applying: Fix test namespace

Rerolled because #2863626: Convert web tests to browser tests for image module landed which contained identical changed to core/modules/image/tests/src/Functional/ImageFieldTestBase.php.

Thanks git rebase!

First, rewinding head to replay your work on top of it...
Applying: Finish RDF test conversion
Applying: Whoops
Using index info to reconstruct a base tree...
M	core/modules/image/tests/src/Functional/ImageFieldTestBase.php
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: Fix test namespace
Applying: Add @vaplas changes

Adding review credits

Committed as 98093be and pushed to 8.6.x

Cherry-picked as 5779395 and pushed to 8.5.x

More down