Ship HTML test files for JS using 'packageFiles' instead of 'templates'

We originally used 'templates' because it seemed like an obvious
choice for HTML files, and because 'packageFiles' requires extra code
to include anything that isn't a .js or .json file.

However, the templates are expected to be HTML fragments rather than
whole documents, and they are parsed in a particular way that takes a
lot of code to clean up (which we needed to do, because we use the
same test files for testing PHP code).

I tried doing it in the 'packageFiles' way, and the extra code doesn't
seem that bad in comparison after all. Moreover, the 'templates'
mechanism (when used the intended way) feels vaguely deprecated in
favor of Vue.js, and I'd rather move away from it.

This makes the tests faster too (probably mostly thanks to the removal
of the clean up code) – on my machine they go from 1800ms to 1500ms.

(Simplify linearWalk tests, as we no longer need to do weird things
with document fragments to get consistent outputs in PHP and JS.)

Change-Id: I39f9b994ce5636d70fea2e935a7c87c7d56dcb26

includes/ResourceLoaderData.php

@@ -169,7 +169,17 @@ class ResourceLoaderData {
 						if ( str_ends_with( $val, '.json' ) ) {
 							$info['packageFiles'][] = substr( $val, strlen( '../' ) );
 						} elseif ( str_ends_with( $val, '.html' ) ) {
-							$info['templates'][] = $val;
+							$info['packageFiles'][] = [
+								'name' => $val,
+								'type' => 'data',
+								'callback' => static function () use ( $info, $val ) {
+									$localPath = $info['localBasePath'] . '/' . $val;
+									return file_get_contents( $localPath );
+								},
+								'versionCallback' => static function () use ( $val ) {
+									return new RL\FilePath( $val );
+								},
+							];
 						}
 					}
 				}

tests/cases/linearWalk/simple.json

@@ -1,5 +1,9 @@
 [
-	"enter #document-fragment(11)",
+	"enter #document(9)",
+	"enter html(1)",
+	"enter head(1)",
+	"leave head(1)",
+	"enter body(1)",
 	"enter p(1)",
 	"enter b(1)",
 	"enter a(1)",
@@ -146,5 +150,9 @@
 	"enter #text(3)",
 	"leave #text(3)",
 	"leave p(1)",
-	"leave #document-fragment(11)"
+	"enter #text(3)",
+	"leave #text(3)",
+	"leave body(1)",
+	"leave html(1)",
+	"leave #document(9)"
 ]

tests/phpunit/unit/CommentUtilsTest.php

@@ -5,7 +5,6 @@ namespace MediaWiki\Extension\DiscussionTools\Tests\Unit;
 use MediaWiki\Extension\DiscussionTools\CommentUtils;
 use MediaWiki\Extension\DiscussionTools\Tests\TestUtils;
 use MediaWikiUnitTestCase;
-use Wikimedia\Parsoid\Utils\DOMCompat;
 
 /**
  * @group DiscussionTools
@@ -20,18 +19,16 @@ class CommentUtilsTest extends MediaWikiUnitTestCase {
 	 */
 	public function testLinearWalk( string $name, string $htmlPath, string $expectedPath ) {
 		$html = static::getHtml( $htmlPath );
-		// Slightly awkward to get the same output as in the JS version
-		$fragment = ( DOMCompat::newDocument( true ) )->createDocumentFragment();
-		$fragment->appendXML( trim( $html ) );
+		$doc = static::createDocument( $html );
 		$expected = static::getJson( $expectedPath );
 
 		$actual = [];
-		CommentUtils::linearWalk( $fragment, static function ( $event, $node ) use ( &$actual ) {
+		CommentUtils::linearWalk( $doc, static function ( $event, $node ) use ( &$actual ) {
 			$actual[] = "$event {$node->nodeName}({$node->nodeType})";
 		} );
 
 		$actualBackwards = [];
-		CommentUtils::linearWalkBackwards( $fragment, static function ( $event, $node ) use ( &$actualBackwards ) {
+		CommentUtils::linearWalkBackwards( $doc, static function ( $event, $node ) use ( &$actualBackwards ) {
 			$actualBackwards[] = "$event {$node->nodeName}({$node->nodeType})";
 		} );
 

tests/qunit/modifier.test.js

@@ -21,23 +21,19 @@ require( '../cases/modified.json' ).forEach( function ( caseItem ) {
 		return;
 	}
 	QUnit.test( testName, function ( assert ) {
-		var fixture = document.getElementById( 'qunit-fixture' );
-
-		var dom = mw.template.get( 'test.DiscussionTools', caseItem.dom ).render(),
-			expected = mw.template.get( 'test.DiscussionTools', caseItem.expected ).render(),
+		var dom = ve.createDocumentFromHtml( require( '../' + caseItem.dom ) ),
+			expected = ve.createDocumentFromHtml( require( '../' + caseItem.expected ) ),
 			config = require( caseItem.config ),
 			data = require( caseItem.data ),
 			title = mw.Title.newFromText( caseItem.title );
 
 		testUtils.overrideMwConfig( config );
 
-		$( fixture ).empty().append( testUtils.getThreadContainer( expected ).children() );
-		var expectedHtml = fixture.innerHTML;
+		var expectedHtml = testUtils.getThreadContainer( expected ).innerHTML;
+		var reverseExpectedHtml = testUtils.getThreadContainer( dom ).innerHTML;
 
-		$( fixture ).empty().append( testUtils.getThreadContainer( dom ).children() );
-		var reverseExpectedHtml = fixture.innerHTML;
-
-		var threadItemSet = new Parser( data ).parse( fixture, title );
+		var container = testUtils.getThreadContainer( dom );
+		var threadItemSet = new Parser( data ).parse( container, title );
 		var comments = threadItemSet.getCommentItems();
 
 		// Add a reply to every comment. Note that this inserts *all* of the replies, unlike the real
@@ -51,9 +47,9 @@ require( '../cases/modified.json' ).forEach( function ( caseItem ) {
 		} );
 
 		// Uncomment this to get updated content for the "modified HTML" files, for copy/paste:
-		// console.log( fixture.innerHTML );
+		// console.log( container.innerHTML );
 
-		var actualHtml = fixture.innerHTML;
+		var actualHtml = container.innerHTML;
 
 		assert.strictEqual(
 			actualHtml,
@@ -66,7 +62,7 @@ require( '../cases/modified.json' ).forEach( function ( caseItem ) {
 			modifier.removeAddedListItem( node );
 		} );
 
-		var reverseActualHtml = fixture.innerHTML;
+		var reverseActualHtml = container.innerHTML;
 		assert.strictEqual(
 			reverseActualHtml,
 			reverseExpectedHtml,
@@ -76,24 +72,22 @@ require( '../cases/modified.json' ).forEach( function ( caseItem ) {
 } );
 
 QUnit.test( '#addReplyLink', function ( assert ) {
-	var cases = require( '../cases/reply.json' ),
-		fixture = document.getElementById( 'qunit-fixture' );
+	var cases = require( '../cases/reply.json' );
 
 	cases.forEach( function ( caseItem ) {
-		var dom = mw.template.get( 'test.DiscussionTools', caseItem.dom ).render(),
-			expected = mw.template.get( 'test.DiscussionTools', caseItem.expected ).render(),
+		var dom = ve.createDocumentFromHtml( require( '../' + caseItem.dom ) ),
+			expected = ve.createDocumentFromHtml( require( '../' + caseItem.expected ) ),
 			config = require( caseItem.config ),
 			data = require( caseItem.data ),
 			title = mw.Title.newFromText( caseItem.title );
 
 		testUtils.overrideMwConfig( config );
 
-		$( fixture ).empty().append( testUtils.getThreadContainer( expected ).children() );
-		var expectedHtml = fixture.innerHTML;
+		var expectedHtml = testUtils.getThreadContainer( expected ).innerHTML;
 
-		$( fixture ).empty().append( testUtils.getThreadContainer( dom ).children() );
+		var container = testUtils.getThreadContainer( dom );
 
-		var threadItemSet = new Parser( data ).parse( fixture, title );
+		var threadItemSet = new Parser( data ).parse( container, title );
 		var comments = threadItemSet.getCommentItems();
 
 		// Add a reply link to every comment.
@@ -105,9 +99,9 @@ QUnit.test( '#addReplyLink', function ( assert ) {
 		} );
 
 		// Uncomment this to get updated content for the "reply HTML" files, for copy/paste:
-		// console.log( fixture.innerHTML );
+		// console.log( container.innerHTML );
 
-		var actualHtml = fixture.innerHTML;
+		var actualHtml = container.innerHTML;
 
 		assert.strictEqual(
 			actualHtml,

tests/qunit/parser.test.js

@@ -85,21 +85,20 @@ require( '../cases/comments.json' ).forEach( function ( caseItem ) {
 	}
 
 	QUnit.test( testName, function ( assert ) {
-		var fixture = document.getElementById( 'qunit-fixture' );
-		var $dom = mw.template.get( 'test.DiscussionTools', caseItem.dom ).render(),
+		var dom = ve.createDocumentFromHtml( require( '../' + caseItem.dom ) ),
 			expected = require( caseItem.expected ),
 			config = require( caseItem.config ),
 			data = require( caseItem.data ),
 			title = mw.Title.newFromText( caseItem.title );
 
-		$( fixture ).empty().append( testUtils.getThreadContainer( $dom ).children() );
+		var container = testUtils.getThreadContainer( dom );
 		testUtils.overrideMwConfig( config );
 
-		var threadItemSet = new Parser( data ).parse( fixture, title );
+		var threadItemSet = new Parser( data ).parse( container, title );
 		var threads = threadItemSet.getThreads();
 
 		threads.forEach( function ( thread, i ) {
-			testUtils.serializeComments( thread, fixture );
+			testUtils.serializeComments( thread, container );
 
 			assert.deepEqual(
 				JSON.parse( JSON.stringify( thread ) ),

tests/qunit/testUtils.js

@@ -18,20 +18,15 @@ module.exports.overrideMwConfig = function ( config ) {
 /**
  * Return the node that is expected to contain thread items.
  *
- * @param {jQuery} $nodes
- * @return {jQuery}
+ * @param {Document} doc
+ * @return {Element}
  */
-module.exports.getThreadContainer = function ( $nodes ) {
-	// In tests created from Parsoid output, comments are contained directly in <body>. This becomes a
-	// huge mess of <section> nodes when we insert it into the existing document, oh well…
+module.exports.getThreadContainer = function ( doc ) {
+	// In tests created from Parsoid output, comments are contained directly in <body>.
 	// In tests created from old parser output, comments are contained in <div class="mw-parser-output">.
-	if ( $nodes.filter( 'section' ).length ) {
-		return $( '<div>' )
-			.append( $nodes.filter( 'section' ) )
-			.append( $nodes.filter( 'base' ) );
-	} else {
-		return $nodes.find( 'div.mw-parser-output' ).addBack( 'div.mw-parser-output' );
-	}
+	var body = doc.body;
+	var wrapper = body.querySelector( 'div.mw-parser-output' );
+	return wrapper || body;
 };
 
 /**

tests/qunit/utils.test.js

@@ -7,16 +7,16 @@ QUnit.test( '#linearWalk', function ( assert ) {
 
 	cases.forEach( function ( caseItem ) {
 		var
-			$dom = mw.template.get( 'test.DiscussionTools', caseItem.dom ).render(),
+			doc = ve.createDocumentFromHtml( require( '../' + caseItem.dom ) ),
 			expected = require( caseItem.expected );
 
 		var actual = [];
-		utils.linearWalk( $dom[ 0 ].parentNode, function ( event, node ) {
+		utils.linearWalk( doc, function ( event, node ) {
 			actual.push( event + ' ' + node.nodeName.toLowerCase() + '(' + node.nodeType + ')' );
 		} );
 
 		var actualBackwards = [];
-		utils.linearWalkBackwards( $dom[ 0 ].parentNode, function ( event, node ) {
+		utils.linearWalkBackwards( doc, function ( event, node ) {
 			actualBackwards.push( event + ' ' + node.nodeName.toLowerCase() + '(' + node.nodeType + ')' );
 		} );