From 4397bda8cb8feee4cd6c9f4ef8be122e19574798 Mon Sep 17 00:00:00 2001
From: Ed Sanders <esanders@wikimedia.org>
Date: Mon, 11 May 2020 15:25:01 +0100
Subject: [PATCH] Modifier: Separate out test data

Change-Id: I63d868a47daebee180c229b424311d10b80d2e89
---
 extension.json                  |   3 +
 tests/qunit/cases/modified.json |  72 +++++++++++
 tests/qunit/cases/reply.json    |  30 +++++
 tests/qunit/cases/unwrap.json   |  32 +++++
 tests/qunit/modifier.test.js    | 209 +++++++-------------------------
 5 files changed, 178 insertions(+), 168 deletions(-)
 create mode 100644 tests/qunit/cases/modified.json
 create mode 100644 tests/qunit/cases/reply.json
 create mode 100644 tests/qunit/cases/unwrap.json

diff --git a/extension.json b/extension.json
index 7bf25063e..b6ab3afba 100644
--- a/extension.json
+++ b/extension.json
@@ -163,6 +163,9 @@
 			"data/plwiki-config.json",
 			"data/plwiki-data.json",
 			"cases/comments.json",
+			"cases/modified.json",
+			"cases/reply.json",
+			"cases/unwrap.json",
 			"cases/timestamp-regex.json",
 			"cases/timestamp-parser.json",
 			"cases/timestamp-parser-dst.json",
diff --git a/tests/qunit/cases/modified.json b/tests/qunit/cases/modified.json
new file mode 100644
index 000000000..ea491b635
--- /dev/null
+++ b/tests/qunit/cases/modified.json
@@ -0,0 +1,72 @@
+[
+	{
+		"name": "plwiki oldparser",
+		"dom": "cases/pl-big-oldparser/pl-big-oldparser.html",
+		"expected": "cases/pl-big-oldparser/pl-big-oldparser-modified.html",
+		"config": "./data/plwiki-config.json",
+		"data": "./data/plwiki-data.json"
+	},
+	{
+		"name": "plwiki parsoid",
+		"dom": "cases/pl-big-parsoid/pl-big-parsoid.html",
+		"expected": "cases/pl-big-parsoid/pl-big-parsoid-modified.html",
+		"config": "./data/plwiki-config.json",
+		"data": "./data/plwiki-data.json"
+	},
+	{
+		"name": "enwiki oldparser",
+		"dom": "cases/en-big-oldparser/en-big-oldparser.html",
+		"expected": "cases/en-big-oldparser/en-big-oldparser-modified.html",
+		"config": "./data/enwiki-config.json",
+		"data": "./data/enwiki-data.json"
+	},
+	{
+		"name": "enwiki parsoid",
+		"dom": "cases/en-big-parsoid/en-big-parsoid.html",
+		"expected": "cases/en-big-parsoid/en-big-parsoid-modified.html",
+		"config": "./data/enwiki-config.json",
+		"data": "./data/enwiki-data.json"
+	},
+	{
+		"name": "arwiki no-paragraph oldparser",
+		"dom": "cases/ar-no-paragraph-oldparser/ar-no-paragraph-oldparser.html",
+		"expected": "cases/ar-no-paragraph-oldparser/ar-no-paragraph-oldparser-modified.html",
+		"config": "./data/arwiki-config.json",
+		"data": "./data/arwiki-data.json"
+	},
+	{
+		"name": "arwiki no-paragraph parsoid",
+		"dom": "cases/ar-no-paragraph-parsoid/ar-no-paragraph-parsoid.html",
+		"expected": "cases/ar-no-paragraph-parsoid/ar-no-paragraph-parsoid-modified.html",
+		"config": "./data/arwiki-config.json",
+		"data": "./data/arwiki-data.json"
+	},
+	{
+		"name": "Must split a list to reply to one of the comments",
+		"dom": "cases/split-list/split-list.html",
+		"expected": "cases/split-list/split-list-modified.html",
+		"config": "./data/enwiki-config.json",
+		"data": "./data/enwiki-data.json"
+	},
+	{
+		"name": "Must split a list to reply to one of the comments (version 2)",
+		"dom": "cases/split-list2/split-list2.html",
+		"expected": "cases/split-list2/split-list2-modified.html",
+		"config": "./data/enwiki-config.json",
+		"data": "./data/enwiki-data.json"
+	},
+	{
+		"name": "Reply inserted inside/outside various wrapper elements",
+		"dom": "cases/wrappers/wrappers.html",
+		"expected": "cases/wrappers/wrappers-modified.html",
+		"config": "./data/enwiki-config.json",
+		"data": "./data/enwiki-data.json"
+	},
+	{
+		"name": "Signatures in funny places",
+		"dom": "cases/signatures-funny/signatures-funny.html",
+		"expected": "cases/signatures-funny/signatures-funny-modified.html",
+		"config": "./data/enwiki-config.json",
+		"data": "./data/enwiki-data.json"
+	}
+]
diff --git a/tests/qunit/cases/reply.json b/tests/qunit/cases/reply.json
new file mode 100644
index 000000000..809fd7822
--- /dev/null
+++ b/tests/qunit/cases/reply.json
@@ -0,0 +1,30 @@
+[
+	{
+		"name": "plwiki oldparser",
+		"dom": "cases/pl-big-oldparser/pl-big-oldparser.html",
+		"expected": "cases/pl-big-oldparser/pl-big-oldparser-reply.html",
+		"config": "./data/plwiki-config.json",
+		"data": "./data/plwiki-data.json"
+	},
+	{
+		"name": "enwiki oldparser",
+		"dom": "cases/en-big-oldparser/en-big-oldparser.html",
+		"expected": "cases/en-big-oldparser/en-big-oldparser-reply.html",
+		"config": "./data/enwiki-config.json",
+		"data": "./data/enwiki-data.json"
+	},
+	{
+		"name": "arwiki no-paragraph oldparser",
+		"dom": "cases/ar-no-paragraph-oldparser/ar-no-paragraph-oldparser.html",
+		"expected": "cases/ar-no-paragraph-oldparser/ar-no-paragraph-oldparser-reply.html",
+		"config": "./data/arwiki-config.json",
+		"data": "./data/arwiki-data.json"
+	},
+	{
+		"name": "Signatures in funny places",
+		"dom": "cases/signatures-funny/signatures-funny.html",
+		"expected": "cases/signatures-funny/signatures-funny-reply.html",
+		"config": "./data/enwiki-config.json",
+		"data": "./data/enwiki-data.json"
+	}
+]
diff --git a/tests/qunit/cases/unwrap.json b/tests/qunit/cases/unwrap.json
new file mode 100644
index 000000000..be3f3dc8d
--- /dev/null
+++ b/tests/qunit/cases/unwrap.json
@@ -0,0 +1,32 @@
+[
+	{
+		"name": "empty",
+		"html": "<dl><dd></dd></dl>",
+		"expected": ""
+	},
+	{
+		"name": "single item",
+		"html": "<dl><dd>Foo</dd></dl>",
+		"expected": "<p>Foo</p>"
+	},
+	{
+		"name": "single block item",
+		"html": "<dl><dd><pre>Foo</pre></dd></dl>",
+		"expected": "<pre>Foo</pre>"
+	},
+	{
+		"name": "mixed inline and block",
+		"html": "<dl><dd>Foo <pre>Bar</pre> Baz</dd></dl>",
+		"expected": "<p>Foo </p><pre>Bar</pre><p> Baz</p>"
+	},
+	{
+		"name": "multiple items",
+		"html": "<dl><dd>Foo</dd><dd>Bar</dd></dl>",
+		"expected": "<p>Foo</p><p>Bar</p>"
+	},
+	{
+		"name": "nested list",
+		"html": "<dl><dd>Foo<dl><dd>Bar</dd></dl></dd></dl>",
+		"expected": "<p>Foo</p><dl><dd>Bar</dd></dl>"
+	}
+]
diff --git a/tests/qunit/modifier.test.js b/tests/qunit/modifier.test.js
index 8b9ada062..31ee04187 100644
--- a/tests/qunit/modifier.test.js
+++ b/tests/qunit/modifier.test.js
@@ -6,93 +6,24 @@ var
 QUnit.module( 'mw.dt.modifier', testUtils.newEnvironment() );
 
 QUnit.test( '#addListItem/#removeListItem', function ( assert ) {
-	var i, j, cases,
-		actualHtml, expectedHtml, reverseActualHtml, reverseExpectedHtml,
-		comments, nodes, node, fixture;
+	var cases = require( './cases/modified.json' ),
+		fixture = document.getElementById( 'qunit-fixture' );
 
-	cases = [
-		{
-			name: 'plwiki oldparser',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/pl-big-oldparser/pl-big-oldparser.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/pl-big-oldparser/pl-big-oldparser-modified.html' ).render(),
-			config: require( './data/plwiki-config.json' ),
-			data: require( './data/plwiki-data.json' )
-		},
-		{
-			name: 'plwiki parsoid',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/pl-big-parsoid/pl-big-parsoid.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/pl-big-parsoid/pl-big-parsoid-modified.html' ).render(),
-			config: require( './data/plwiki-config.json' ),
-			data: require( './data/plwiki-data.json' )
-		},
-		{
-			name: 'enwiki oldparser',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/en-big-oldparser/en-big-oldparser.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/en-big-oldparser/en-big-oldparser-modified.html' ).render(),
-			config: require( './data/enwiki-config.json' ),
-			data: require( './data/enwiki-data.json' )
-		},
-		{
-			name: 'enwiki parsoid',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/en-big-parsoid/en-big-parsoid.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/en-big-parsoid/en-big-parsoid-modified.html' ).render(),
-			config: require( './data/enwiki-config.json' ),
-			data: require( './data/enwiki-data.json' )
-		},
-		{
-			name: 'arwiki no-paragraph oldparser',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/ar-no-paragraph-oldparser/ar-no-paragraph-oldparser.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/ar-no-paragraph-oldparser/ar-no-paragraph-oldparser-modified.html' ).render(),
-			config: require( './data/arwiki-config.json' ),
-			data: require( './data/arwiki-data.json' )
-		},
-		{
-			name: 'arwiki no-paragraph parsoid',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/ar-no-paragraph-parsoid/ar-no-paragraph-parsoid.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/ar-no-paragraph-parsoid/ar-no-paragraph-parsoid-modified.html' ).render(),
-			config: require( './data/arwiki-config.json' ),
-			data: require( './data/arwiki-data.json' )
-		},
-		{
-			name: 'Must split a list to reply to one of the comments',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/split-list/split-list.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/split-list/split-list-modified.html' ).render(),
-			config: require( './data/enwiki-config.json' ),
-			data: require( './data/enwiki-data.json' )
-		},
-		{
-			name: 'Must split a list to reply to one of the comments (version 2)',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/split-list2/split-list2.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/split-list2/split-list2-modified.html' ).render(),
-			config: require( './data/enwiki-config.json' ),
-			data: require( './data/enwiki-data.json' )
-		},
-		{
-			name: 'Reply inserted inside/outside various wrapper elements',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/wrappers/wrappers.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/wrappers/wrappers-modified.html' ).render(),
-			config: require( './data/enwiki-config.json' ),
-			data: require( './data/enwiki-data.json' )
-		},
-		{
-			name: 'Signatures in funny places',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/signatures-funny/signatures-funny.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/signatures-funny/signatures-funny-modified.html' ).render(),
-			config: require( './data/enwiki-config.json' ),
-			data: require( './data/enwiki-data.json' )
-		}
-	];
+	cases.forEach( function ( caseItem ) {
+		var actualHtml, expectedHtml, reverseActualHtml, reverseExpectedHtml,
+			i, comments, nodes, node,
+			dom = mw.template.get( 'test.DiscussionTools', caseItem.dom ).render(),
+			expected = mw.template.get( 'test.DiscussionTools', caseItem.expected ).render(),
+			config = require( caseItem.config ),
+			data = require( caseItem.data );
 
-	fixture = document.getElementById( 'qunit-fixture' );
+		testUtils.overrideMwConfig( config );
+		testUtils.overrideParserData( data );
 
-	for ( i = 0; i < cases.length; i++ ) {
-		testUtils.overrideMwConfig( cases[ i ].config );
-		testUtils.overrideParserData( cases[ i ].data );
-
-		$( fixture ).empty().append( cases[ i ].expected );
+		$( fixture ).empty().append( expected );
 		expectedHtml = fixture.innerHTML;
 
-		$( fixture ).empty().append( cases[ i ].dom.clone() );
+		$( fixture ).empty().append( dom.clone() );
 		reverseExpectedHtml = fixture.innerHTML;
 
 		comments = parser.getComments( fixture );
@@ -102,12 +33,12 @@ QUnit.test( '#addListItem/#removeListItem', function ( assert ) {
 		// thing, which only deals with one at a time. This isn't ideal but resetting everything after
 		// every reply would be super slow.
 		nodes = [];
-		for ( j = 0; j < comments.length; j++ ) {
-			if ( comments[ j ].type === 'heading' ) {
+		for ( i = 0; i < comments.length; i++ ) {
+			if ( comments[ i ].type === 'heading' ) {
 				continue;
 			}
-			node = modifier.addListItem( comments[ j ] );
-			node.textContent = 'Reply to ' + comments[ j ].id;
+			node = modifier.addListItem( comments[ i ] );
+			node.textContent = 'Reply to ' + comments[ i ].id;
 			nodes.push( node );
 		}
 
@@ -119,80 +50,55 @@ QUnit.test( '#addListItem/#removeListItem', function ( assert ) {
 		assert.strictEqual(
 			actualHtml,
 			expectedHtml,
-			cases[ i ].name
+			caseItem.name
 		);
 
 		// Now discard the replies and verify we get the original document back.
-		for ( j = 0; j < nodes.length; j++ ) {
-			modifier.removeListItem( nodes[ j ] );
+		for ( i = 0; i < nodes.length; i++ ) {
+			modifier.removeListItem( nodes[ i ] );
 		}
 
 		reverseActualHtml = fixture.innerHTML;
 		assert.strictEqual(
 			reverseActualHtml,
 			reverseExpectedHtml,
-			cases[ i ].name + ' (discard replies)'
+			caseItem.name + ' (discard replies)'
 		);
-	}
+	} );
 } );
 
 QUnit.test( '#addReplyLink', function ( assert ) {
-	var i, j, cases, actualHtml, expectedHtml, comments, linkNode, fixture;
+	var cases = require( './cases/reply.json' ),
+		fixture = document.getElementById( 'qunit-fixture' );
 
-	cases = [
-		{
-			name: 'plwiki oldparser',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/pl-big-oldparser/pl-big-oldparser.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/pl-big-oldparser/pl-big-oldparser-reply.html' ).render(),
-			config: require( './data/plwiki-config.json' ),
-			data: require( './data/plwiki-data.json' )
-		},
-		{
-			name: 'enwiki oldparser',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/en-big-oldparser/en-big-oldparser.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/en-big-oldparser/en-big-oldparser-reply.html' ).render(),
-			config: require( './data/enwiki-config.json' ),
-			data: require( './data/enwiki-data.json' )
-		},
-		{
-			name: 'arwiki no-paragraph oldparser',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/ar-no-paragraph-oldparser/ar-no-paragraph-oldparser.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/ar-no-paragraph-oldparser/ar-no-paragraph-oldparser-reply.html' ).render(),
-			config: require( './data/arwiki-config.json' ),
-			data: require( './data/arwiki-data.json' )
-		},
-		{
-			name: 'Signatures in funny places',
-			dom: mw.template.get( 'test.DiscussionTools', 'cases/signatures-funny/signatures-funny.html' ).render(),
-			expected: mw.template.get( 'test.DiscussionTools', 'cases/signatures-funny/signatures-funny-reply.html' ).render(),
-			config: require( './data/enwiki-config.json' ),
-			data: require( './data/enwiki-data.json' )
-		}
-	];
+	cases.forEach( function ( caseItem ) {
+		var actualHtml, expectedHtml,
+			i, comments, linkNode,
+			dom = mw.template.get( 'test.DiscussionTools', caseItem.dom ).render(),
+			expected = mw.template.get( 'test.DiscussionTools', caseItem.expected ).render(),
+			config = require( caseItem.config ),
+			data = require( caseItem.data );
 
-	fixture = document.getElementById( 'qunit-fixture' );
+		testUtils.overrideMwConfig( config );
+		testUtils.overrideParserData( data );
 
-	for ( i = 0; i < cases.length; i++ ) {
-		testUtils.overrideMwConfig( cases[ i ].config );
-		testUtils.overrideParserData( cases[ i ].data );
-
-		$( fixture ).empty().append( cases[ i ].expected );
+		$( fixture ).empty().append( expected );
 		expectedHtml = fixture.innerHTML;
 
-		$( fixture ).empty().append( cases[ i ].dom.clone() );
+		$( fixture ).empty().append( dom.clone() );
 
 		comments = parser.getComments( fixture );
 		parser.groupThreads( comments );
 
 		// Add a reply link to every comment.
-		for ( j = 0; j < comments.length; j++ ) {
-			if ( comments[ j ].type === 'heading' ) {
+		for ( i = 0; i < comments.length; i++ ) {
+			if ( comments[ i ].type === 'heading' ) {
 				continue;
 			}
 			linkNode = document.createElement( 'a' );
 			linkNode.textContent = 'Reply';
 			linkNode.href = '#';
-			modifier.addReplyLink( comments[ j ], linkNode );
+			modifier.addReplyLink( comments[ i ], linkNode );
 		}
 
 		// Uncomment this to get updated content for the "reply HTML" files, for copy/paste:
@@ -203,46 +109,13 @@ QUnit.test( '#addReplyLink', function ( assert ) {
 		assert.strictEqual(
 			actualHtml,
 			expectedHtml,
-			cases[ i ].name
+			caseItem.name
 		);
-	}
+	} );
 } );
 
 QUnit.test( '#unwrapList', function ( assert ) {
-	var cases;
-
-	cases = [
-		{
-			name: 'empty',
-			html: '<dl><dd></dd></dl>',
-			expected: ''
-		},
-		{
-			name: 'single item',
-			html: '<dl><dd>Foo</dd></dl>',
-			expected: '<p>Foo</p>'
-		},
-		{
-			name: 'single block item',
-			html: '<dl><dd><pre>Foo</pre></dd></dl>',
-			expected: '<pre>Foo</pre>'
-		},
-		{
-			name: 'mixed inline and block',
-			html: '<dl><dd>Foo <pre>Bar</pre> Baz</dd></dl>',
-			expected: '<p>Foo </p><pre>Bar</pre><p> Baz</p>'
-		},
-		{
-			name: 'multiple items',
-			html: '<dl><dd>Foo</dd><dd>Bar</dd></dl>',
-			expected: '<p>Foo</p><p>Bar</p>'
-		},
-		{
-			name: 'nested list',
-			html: '<dl><dd>Foo<dl><dd>Bar</dd></dl></dd></dl>',
-			expected: '<p>Foo</p><dl><dd>Bar</dd></dl>'
-		}
-	];
+	var cases = require( './cases/unwrap.json' );
 
 	cases.forEach( function ( caseItem ) {
 		var container = document.createElement( 'div' );