From 3dfede8ada9cec021ccaa344700f9ffde7ec4081 Mon Sep 17 00:00:00 2001
From: Tim Starling <tstarling@wikimedia.org>
Date: Wed, 28 Sep 2016 16:30:25 +1000
Subject: [PATCH] Don't skip tests which lack non-tidy output

Previously some 60 tests which only had tidy output were silently
removed from the test list and were not executed. Perhaps miraculously,
they all pass (if Math is disabled).

Also, throw an error if a test lacks any kind of result section. Fix one
test which accidentally lacked a result section.

Change-Id: Ic840678b851d6fc472b834cde3d8c83359f1c272
---
 tests/parser/TestFileReader.php | 98 +++++++++++++++++++++++----------
 tests/parser/parserTests.txt    |  4 ++
 2 files changed, 74 insertions(+), 28 deletions(-)

diff --git a/tests/parser/TestFileReader.php b/tests/parser/TestFileReader.php
index a1a8d1968c..6279d68fd8 100644
--- a/tests/parser/TestFileReader.php
+++ b/tests/parser/TestFileReader.php
@@ -25,6 +25,7 @@ class TestFileReader {
 	private $section = null;
 	/** String|null: current test section being analyzed */
 	private $sectionData = [];
+	private $sectionLineNum = [];
 	private $lineNum = 0;
 	private $runDisabled;
 	private $runParsoid;
@@ -77,44 +78,83 @@ class TestFileReader {
 		// "input" and "result" are old section names allowed
 		// for backwards-compatibility.
 		$input = $this->checkSection( [ 'wikitext', 'input' ], false );
-		$result = $this->checkSection( [ 'html/php', 'html/*', 'html', 'result' ], false );
+		$nonTidySection = $this->checkSection(
+			[ 'html/php', 'html/*', 'html', 'result' ], false );
 		// Some tests have "with tidy" and "without tidy" variants
-		$tidy = $this->checkSection( [ 'html/php+tidy', 'html+tidy' ], false );
+		$tidySection = $this->checkSection( [ 'html/php+tidy', 'html+tidy' ], false );
 
-		if ( !isset( $this->sectionData['options'] ) ) {
-			$this->sectionData['options'] = '';
+		// Remove trailing newline
+		$data = array_map( 'ParserTestRunner::chomp', $this->sectionData );
+
+		// Apply defaults
+		$data += [
+			'options' => '',
+			'config' => ''
+		];
+
+		if ( $input === false ) {
+			throw new MWException( "Test at {$this->file}:{$this->sectionLineNum['test']} " .
+				"lacks input section" );
+		}
+
+		if ( preg_match( '/\\bdisabled\\b/i', $data['options'] ) &&	!$this->runDisabled ) {
+			// Disabled
+			return;
 		}
 
-		if ( !isset( $this->sectionData['config'] ) ) {
-			$this->sectionData['config'] = '';
+		if ( $tidySection === false && $nonTidySection === false ) {
+			if ( isset( $data['html/parsoid'] ) || isset( $data['wikitext/edited'] ) ) {
+				// Parsoid only
+				return;
+			} else {
+				throw new MWException( "Test at {$this->file}:{$this->sectionLineNum['test']} " .
+					"lacks result section" );
+			}
+		}
+
+		if ( preg_match( '/\\bparsoid\\b/i', $data['options'] ) && $nonTidySection === 'html'
+			&& !$this->runParsoid
+		) {
+			// A test which normally runs on Parsoid but can optionally be run with MW
+			return;
 		}
 
-		$isDisabled = preg_match( '/\\bdisabled\\b/i', $this->sectionData['options'] ) &&
-			!$this->runDisabled;
-		$isParsoidOnly = preg_match( '/\\bparsoid\\b/i', $this->sectionData['options'] ) &&
-			$result == 'html' &&
-			!$this->runParsoid;
-		$isFiltered = !preg_match( $this->regex, $this->sectionData['test'] );
-		if ( $input == false || $result == false || $isDisabled || $isParsoidOnly || $isFiltered ) {
-			// Disabled test
+		if ( !preg_match( $this->regex, $data['test'] ) ) {
+			// Filtered test
 			return;
 		}
 
-		$test = [
-			'test' => ParserTestRunner::chomp( $this->sectionData['test'] ),
-			'input' => ParserTestRunner::chomp( $this->sectionData[$input] ),
-			'result' => ParserTestRunner::chomp( $this->sectionData[$result] ),
-			'options' => ParserTestRunner::chomp( $this->sectionData['options'] ),
-			'config' => ParserTestRunner::chomp( $this->sectionData['config'] ),
+		$commonInfo = [
+			'test' => $data['test'],
+			'desc' => $data['test'],
+			'input' => $data[$input],
+			'options' => $data['options'],
+			'config' => $data['config'],
 		];
-		$test['desc'] = $test['test'];
-		$this->tests[] = $test;
-
-		if ( $tidy !== false ) {
-			$test['options'] .= " tidy";
-			$test['desc'] .= ' (with tidy)';
-			$test['result'] = ParserTestRunner::chomp( $this->sectionData[$tidy] );
-			$this->tests[] = $test;
+
+		if ( $nonTidySection !== false ) {
+			// Add non-tidy test
+			$this->tests[] = [
+				'result' => $data[$nonTidySection],
+			] + $commonInfo;
+
+			if ( $tidySection !== false ) {
+				// Add tidy subtest
+				$this->tests[] = [
+					'desc' => $data['test'] . ' (with tidy)',
+					'result' => $data[$tidySection],
+					'options' => $data['options'] . ' tidy',
+				] + $commonInfo;
+			}
+		} elseif ( $tidySection !== false ) {
+			// No need to override desc when there is no subtest
+			$this->tests[] = [
+				'result' => $data[$tidySection],
+				'options' => $data['options'] . ' tidy'
+			] + $commonInfo;
+		} else {
+			throw new MWException( "Test at {$this->file}:{$this->sectionLineNum['test']} " .
+				"lacks result section" );
 		}
 	}
 
@@ -199,6 +239,7 @@ class TestFileReader {
 						. "at line {$this->lineNum} of $this->file\n" );
 				}
 
+				$this->sectionLineNum[$this->section] = $this->lineNum;
 				$this->sectionData[$this->section] = '';
 
 				continue;
@@ -214,6 +255,7 @@ class TestFileReader {
 	 * Clear section name and its data
 	 */
 	private function clearSection() {
+		$this->sectionLineNum = [];
 		$this->sectionData = [];
 		$this->section = null;
 
diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt
index 1e511f67f5..ff1450b5a6 100644
--- a/tests/parser/parserTests.txt
+++ b/tests/parser/parserTests.txt
@@ -3681,6 +3681,10 @@ Nested definition lists using html syntax
 <dl><dt>x</dt>
 <dd>a</dd>
 <dd>b</dd></dl>
+!! html
+<dl><dt>x</dt>
+<dd>a</dd>
+<dd>b</dd></dl>
 
 !! end
 
-- 
2.20.1