For places where we need to crawl through the actual DOM, I much preffer to not invent our own solution. One of the PHPUnit people started a plugin for this purpouse, which we can contribute to:

https://github.com/phpunit/phpunit-dom-assertions

This uses the Symfony Dom Cralwer component:

http://symfony.com/doc/current/components/dom_crawler.html

Note that this extension introduces a new test base class while we rather need some service class to check html. As far as I can see the plugin does not add so much code so we can also add our own little assertion class imo.

Change 234291 had a related patch set uploaded (by Adrian Lang):
Replace deprecated assertTag() with new DOMTestUtils::assertTagSimple()

https://gerrit.wikimedia.org/r/234291

Change 237372 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Replace some very simple asserttag with assertContains

https://gerrit.wikimedia.org/r/237372

Change 237372 merged by jenkins-bot:
Replace some very simple asserttag with assertContains

https://gerrit.wikimedia.org/r/237372

https://github.com/phpunit/phpunit-dom-assertions doesn't actually implement assertTag yet. Also assertTag is a very ugly interface. But building something on xpath based selection like Symfony Dom Cralwer might be a good idea. We should make sure we are switching to something future proof and not something that is deprecated / technical dept on merge.

Moved to "needs discussion", since it's unclear what assertTag() should be replaced with. I don't think the proposed DOMTestUtils::assertTagSimple() is a good approach.

As I wrote on the ticket, something based on Symfony's CSS selector implementation might be more suitable http://symfony.com/doc/current/components/css_selector.html.

Copying from https://gerrit.wikimedia.org/r/#/c/234291/ :
Daniel Kinzler:

I'd prefer something that is less ad-hoc than this. Perhaps we could use Symfony's CSS selector implementation http://symfony.com/doc/current/components/css_selector.html (and skip the test if the component is missing).

But I agree with Thiemo that we should rather replace assertTag with simple assertContains and *simply* assertRegex calls, where possible.

Ricordisamoa:

How are regular expressions not ad-hoc? Are you sure it's really worth it to use complicated and fragile patterns that must take into account the order of attributes, nested nodes, different quote styles etc. whereas with a few dozens of LOCs you can leverage a real (X|HT)ML parser using a syntax that is both clear and extensible as seen in SpecialModifyTermTestCase?

I agree I'm against any regexp that tries to parse HTML or XML. We should never do that.
I dislike the assertTag matchers syntax and prefer CSS. Because more people know CSS. (Even more than XPath which I would otherwise prefer). Also it is more expressive than assertTag matchers syntax. There was a reason why assertTag was removed from PHPUnit, see https://github.com/sebastianbergmann/phpunit/issues/1292 for some of the context. We should not replicate those errors. Or if we absolutely must replicate assertTag contribute to https://github.com/phpunit/phpunit-dom-assertions .
For text content matching regexp is ok.

So e.g. http://symfony.com/doc/current/components/dom_crawler.html used with http://symfony.com/doc/current/components/css_selector.html for directly asserting something seems like a good idea. Or for more complex things one could use one selector to find and another that every result matches something. However ensuring that a string is well formed HTML or XML with DomCrawler in addition requires enabling the libxml error reporting. Or use PHPunits assertEqualXMLStructure() to match the selection of nodes, assertRegExp() for the text in text nodes. There is also assertXmlStringEqualsXmlString() and friens when you want to compare two documents in string form as is. There is probably some additional code needed to make DomCrawler into usable assertions, but that should result in something that is more long term proof.

See https://github.com/phpunit/phpunit-dom-assertions/blob/master/src/DOMTestCase.php for an example usage of DomCrawler (but it misses checking for libxml errors).

If we prefer something that better handles namespaces than DomCrawler, we probably need to look at https://github.com/FluentDOM/FluentDOM .

Quick update on my take on this: If DOMTestUtils::assertTagSimple() implements a subset of PHPUnit's assertTag(), and doesn't (contrary to my earlier understanding) invent yet another way to specify tag matches, I'm not fundamentally opposed to using it. However, I' still not very eager to go that way because:

• DOMTestUtils would be "born deprecated". We don't want to encourage more usages of it, we really want it to go away and be replaced with something better (based on CSS selectors, perhaps)
• DOMTestUtils is currently copied around. It would make more sense to put assertTagSimple() into the phpunit-dom-assertions project, which has support for assertTag as a TODO. Or at least put it into MediaWikiTestCase.

If we urgently need to migrate to a PHPUnit 5, I'd agree to using assertTagSimple() for now. But it seems that there are several more blockers to this, which gives us time to work on The Real Solution. Well, knowing how things go, nobody will until it's really urgent, but hey, that's life ;)

PS: Is it correct that the code in DOMTestUtils was mainly copied from PHPUnit? If so, it needs to attribute Sebastian Bergman to be in compliance with the BSD license: "Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer."

In T69122#1701960, @daniel wrote:

• DOMTestUtils would be "born deprecated". We don't want to encourage more usages of it, we really want it to go away and be replaced with something better

DOMTestUtils is not in good shape yet, but eventually it should implement a sensible subset of assertTag() and be quite stable.

based on CSS selectors, perhaps

I'm not aware of any CSS equivalent of 'content'.

• DOMTestUtils is currently copied around. It would make more sense to put assertTagSimple() into the phpunit-dom-assertions project, which has support for assertTag as a TODO. Or at least put it into MediaWikiTestCase.

Sure.

If we urgently need to migrate to a PHPUnit 5, I'd agree to using assertTagSimple() for now. But it seems that there are several more blockers to this, which gives us time to work on The Real Solution.

That's why I opposed https://gerrit.wikimedia.org/r/237372 in the first place.

PS: Is it correct that the code in DOMTestUtils was mainly copied from PHPUnit? If so, it needs to attribute Sebastian Bergman to be in compliance with the BSD license: "Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer."

Have a look at https://github.com/sebastianbergmann/phpunit/blob/9a0ed21d58b263ea1e32f8d2e6151f56dac088e2/PHPUnit/Util/XML.php#L515 and https://gerrit.wikimedia.org/r/#/c/234291/7/repo/tests/phpunit/includes/DOMTestUtils.php

Change 270278 had a related patch set uploaded (by Ricordisamoa):
Add DOMTestTrait to replace deprecated assertTag()

https://gerrit.wikimedia.org/r/270278

In T69122#1701985, @Ricordisamoa wrote:

In T69122#1701960, @daniel wrote:

• DOMTestUtils would be "born deprecated". We don't want to encourage more usages of it, we really want it to go away and be replaced with something better

DOMTestUtils is not in good shape yet, but eventually it should implement a sensible subset of assertTag() and be quite stable.

I don't think there is a sensible subset of it. It should really be removed. APIs based on nested arrays are a bad idea.

based on CSS selectors, perhaps

I'm not aware of any CSS equivalent of 'content'.

Select the surrounding node with CSS then pick out the content via Dom and make functions like assertCssSelectionContentContains out of it. Or use something that implements https://api.jquery.com/contains-selector/ also previously known as https://www.w3.org/TR/css3-selectors/#content-selectors . Or use Xpath text() .

Rough usage in extensions:

$ grep -R assertTag *|cut -d\/  -f1|uniq -c

      2 Capiunto
     14 MediaWikiFarm
      1 SemanticMediaWiki
      2 Spreadsheet
     26 Wikibase
      2 WikibaseLexeme
      1 WikibaseQualityConstraints
      2 WikibaseQualityExternalValidation
     31 Wikidata

https://gerrit.wikimedia.org/r/#/c/330242/ is a dummy change that mark assertTag / assertNotTag has deprecated.

So at least that is a low surface and the migration should not be too annoying.

We did not forgot this and are again working on phasing out assertTag, see https://gerrit.wikimedia.org/r/330229 as well as https://github.com/bekh6ex/hamcrest-html-matchers. The later is a draft by @Aleksey_WMDE. We are currently collecting input on this. Please let us know what you think.

Change 330391 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Replace assertTag with assertContains in Entity diff classes

https://gerrit.wikimedia.org/r/330391

Change 330391 merged by jenkins-bot:
Replace assertTag with assertContains in Entity diff classes

https://gerrit.wikimedia.org/r/330391

Change 330229 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Cleanup EntityDiffVisualizerTest

https://gerrit.wikimedia.org/r/330229

Change 330229 merged by jenkins-bot:
Cleanup EntityDiffVisualizerTest

https://gerrit.wikimedia.org/r/330229

Change 334165 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Refactoring SpecialModifyTermTestCase

https://gerrit.wikimedia.org/r/334165

Change 334169 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Simplified html assertion in SpecialGoToLinkedPageTest

https://gerrit.wikimedia.org/r/334169

Change 334333 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
WIP Refactoring SpecialItemByTitleTest

https://gerrit.wikimedia.org/r/334333

Change 334165 merged by jenkins-bot:
Refactoring SpecialModifyTermTestCase

https://gerrit.wikimedia.org/r/334165

Change 334169 merged by jenkins-bot:
Simplified html assertion in SpecialGoToLinkedPageTest

https://gerrit.wikimedia.org/r/334169

Change 335020 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Refactor SpecialMergeItemsTest to remove assertTag usage

https://gerrit.wikimedia.org/r/335020

Change 335021 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Refactor SpecialRedirectEntityTest to remove assertTag usages

https://gerrit.wikimedia.org/r/335021

Change 335027 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Refactor SpecialSetLabelDescriptionAliasesTest to remove assertTag usages

https://gerrit.wikimedia.org/r/335027

Change 335020 merged by jenkins-bot:
Refactor SpecialMergeItemsTest to remove assertTag usage

https://gerrit.wikimedia.org/r/335020

Change 335021 merged by jenkins-bot:
Refactor SpecialRedirectEntityTest to remove assertTag usages

https://gerrit.wikimedia.org/r/335021

Change 335027 merged by jenkins-bot:
Refactor SpecialSetLabelDescriptionAliasesTest to remove assertTag usages

https://gerrit.wikimedia.org/r/335027

Change 335443 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Removed assertTag usages from ChangeLineFormatterTest

https://gerrit.wikimedia.org/r/335443

Change 334333 merged by jenkins-bot:
Refactoring SpecialItemByTitleTest

https://gerrit.wikimedia.org/r/334333

Change 335650 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Removed assertTag from SpecialSetSiteLinkTest

https://gerrit.wikimedia.org/r/335650

Change 335663 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Removed assertTag from SpecialItemByTitleTest

https://gerrit.wikimedia.org/r/335663

Change 335666 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Removed assertTag from SpecialModifyTermTestCase

https://gerrit.wikimedia.org/r/335666

Change 335671 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Removed assertTag from SpecialSetLabelDescriptionAliasesTest

https://gerrit.wikimedia.org/r/335671

Change 270278 abandoned by Ricordisamoa:
Add DOMTestTrait to replace deprecated assertTag()

Reason:
@Aleksey_WMDE is working on a better solution at T69122

https://gerrit.wikimedia.org/r/270278

Change 234291 abandoned by Ricordisamoa:
Replace deprecated assertTag() with DOMTestTrait::assertTagSimple()

Reason:
@Aleksey_WMDE is working on a better solution at T69122

https://gerrit.wikimedia.org/r/234291

Change 335671 merged by jenkins-bot:
Removed assertTag from SpecialSetLabelDescriptionAliasesTest

https://gerrit.wikimedia.org/r/335671

Change 335650 merged by jenkins-bot:
Removed assertTag from SpecialSetSiteLinkTest

https://gerrit.wikimedia.org/r/335650

Change 335663 merged by jenkins-bot:
Removed assertTag from SpecialItemByTitleTest

https://gerrit.wikimedia.org/r/335663

Change 335443 merged by jenkins-bot:
Removed assertTag usages from ChangeLineFormatterTest

https://gerrit.wikimedia.org/r/335443

Change 335666 merged by jenkins-bot:
Removed assertTag from SpecialModifyTermTestCase

https://gerrit.wikimedia.org/r/335666

So if only I did my grepping right it looks like there is no longer any assertTag being used in core Wikibase. Well done @Aleksey_WMDE!
I believe before we say this task is resolved, we should take a look at other Wikibase-related extensions and make sure there's no assertTag use there or fix any uses of it, if this was the case.
List in https://phabricator.wikimedia.org/T69122#2913232 might be of help here.
Also, as this task is a "[Story]" it might make sense to create subtasks for each of extensions that needs to be fixed.

Change 338796 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Replace assertTag() with Hamcrest assertions

https://gerrit.wikimedia.org/r/338796

Change 338799 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Replace assertTag() with Hamcrest assertions

https://gerrit.wikimedia.org/r/338799

Change 338806 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Replace assertTag() with Hamcrest assertions

https://gerrit.wikimedia.org/r/338806

Change 338807 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE)):
Replace assertTag() with Hamcrest assertions

https://gerrit.wikimedia.org/r/338807

Ups. Missed part about creating tickets for each separate extension.
Anyway, there are patches removing assertTag() usages in extensions:

• WikibaseQualityConstraints
• Capiunto
• WikibaseQualityExternalValidation

Change 338807 merged by jenkins-bot:
Replace assertTag() with Hamcrest assertions

https://gerrit.wikimedia.org/r/338807

Change 338799 merged by jenkins-bot:
Replace assertTag() with Hamcrest assertions

https://gerrit.wikimedia.org/r/338799

Change 338796 merged by jenkins-bot:
Replace assertTag() with Hamcrest assertions

https://gerrit.wikimedia.org/r/338796

Change 338806 merged by jenkins-bot:
Replace assertTag() with Hamcrest assertions

https://gerrit.wikimedia.org/r/338806