From f084a82dfcbad0aaaeac108ac7c7484467208d37 Mon Sep 17 00:00:00 2001
From: Mark Story <mark@mark-story.com>
Date: Sun, 9 May 2010 12:30:55 -0400
Subject: [PATCH] Refactoring how coverage diffs are generated to better use
 data from phpunit.  Removing methods made redundant by data changes.

---
 .../cases/libs/html_coverage_report.test.php  | 102 +++++++-----------
 .../lib/coverage/html_coverage_report.php     |  67 +++---------
 2 files changed, 53 insertions(+), 116 deletions(-)

diff --git a/cake/tests/cases/libs/html_coverage_report.test.php b/cake/tests/cases/libs/html_coverage_report.test.php
index 38bc9aff9..d3beb6758 100644
--- a/cake/tests/cases/libs/html_coverage_report.test.php
+++ b/cake/tests/cases/libs/html_coverage_report.test.php
@@ -95,7 +95,13 @@ class HtmlCoverageReportTest extends CakeTestCase {
 				), 
 				'executable' => array(
 					'/something/dispatcher.php' => array(
-						10 => -1
+						9 => -1
+					)
+				),
+				'dead' => array(
+					'/something/dispatcher.php' => array(
+						22 => -2,
+						23 => -2
 					)
 				)
 			),
@@ -111,6 +117,12 @@ class HtmlCoverageReportTest extends CakeTestCase {
 						12 => -1,
 						51 => -1
 					)
+				),
+				'dead' => array(
+					'/something/dispatcher.php' => array(
+						13 => -2,
+						42 => -2
+					)
 				)
 			),
 		);
@@ -119,56 +131,9 @@ class HtmlCoverageReportTest extends CakeTestCase {
 
 		$path = '/something/dispatcher.php';
 		$this->assertTrue(isset($result[$path]));
-		$this->assertEquals(1, $result[$path][10]);
-		$this->assertEquals(1, $result[$path][12]);
-		$this->assertEquals(1, $result[$path][50]);
-		$this->assertEquals(-1, $result[$path][51]);
-	}
-
-/**
- * test the features of getExecutableLines
- *
- * @return void
- */
-	function testGetExecutableLines() {
-		$contents = <<<PHP
-<?php
-/**
- * A comment line.
- */
-function thing() {
-	echo 'thinger';
-}
-
-function other_thing() {
-	if (
-		\$something == true
-	) {
-		doSomethingElse();
-	}
-}
-?>
-PHP;
-		$result = $this->Coverage->getExecutableLines(explode("\n", $contents));
-		$expected = array(
-			0 => false,
-			1 => false,
-			2 => false,
-			3 => false,
-			4 => true,
-			5 => true,
-			6 => false,
-			7 => true,
-			8 => true,
-			9 => true,
-			10 => true,
-			11 => false,
-			12 => true,
-			13 => false,
-			14 => false,
-			15 => false
-		);
-		$this->assertEquals($expected, $result);
+		$this->assertEquals(array(10, 12, 50), array_keys($result[$path]['covered']));
+		$this->assertEquals(array(9, 12, 51), array_keys($result[$path]['executable']));
+		$this->assertEquals(array(22, 23, 13, 42), array_keys($result[$path]['dead']));
 	}
 
 /**
@@ -190,16 +155,22 @@ PHP;
 			'line 10',
 		);
 		$coverage = array(
-			1 => 1,
-			2 => -2,
-			3 => 1,
-			4 => 1,
-			5 => -1,
-			6 => 1,
-			7 => 1,
-			8 => 1,
-			9 => -1,
-			10 => 1,
+			'covered' => array(
+				1 => 1,
+				3 => 1,
+				4 => 1,
+				6 => 1,
+				7 => 1,
+				8 => 1,
+				10 => 1
+			),
+			'executable' => array(
+				5 => -1,
+				9 => -1
+			),
+			'dead' => array(
+				2 => -2
+			)
 		);
 		$result = $this->Coverage->generateDiff('myfile.php', $file, $coverage);
 		$this->assertRegExp('/<h2>myfile\.php Code coverage\: \d+\.?\d*\%<\/h2>/', $result);
@@ -207,9 +178,12 @@ PHP;
 		$this->assertRegExp('/<pre>/', $result);
 		foreach ($file as $i => $line) {
 			$this->assertTrue(strpos($line, $result) !== 0, 'Content is missing ' . $i);
-			$class = 'uncovered';
-			if ($coverage[$i + 1] > 0) {
-				$class = 'covered';
+			$class = 'covered';
+			if (in_array($i + 1, array(5, 9, 2))) {
+				$class = 'uncovered';
+			}
+			if ($i + 1 == 2) {
+				$class .= ' dead';
 			}
 			$this->assertTrue(strpos($class, $result) !== 0, 'Class name is wrong ' . $i);
 		}
diff --git a/cake/tests/lib/coverage/html_coverage_report.php b/cake/tests/lib/coverage/html_coverage_report.php
index 16b09dffa..63d2d0dba 100644
--- a/cake/tests/lib/coverage/html_coverage_report.php
+++ b/cake/tests/lib/coverage/html_coverage_report.php
@@ -128,52 +128,21 @@ class HtmlCoverageReport {
 				$executable = isset($testRun['executable'][$filename]) ? $testRun['executable'][$filename] : array();
 		
 				if (!isset($files[$filename])) {
-					$files[$filename] = array();
+					$files[$filename] = array(
+						'covered' => array(),
+						'dead' => array(),
+						'executable' => array()
+					);
 				}
-				$files[$filename] = $files[$filename] + $fileCoverage + $executable + $dead;
+				$files[$filename]['covered'] += $fileCoverage;
+				$files[$filename]['executable'] += $executable;
+				$files[$filename]['dead'] += $dead;
 			}
 		}
 		ksort($files);
 		return $files;
 	}
 
-/**
- * Removes non executable lines of code from a file contents string.
- *
- * @param array $lines in the file.
- * @return array Array for the file with lines marked as not runnable.
- */
-	public function getExecutableLines($lines) {
-		$output = array();
-
-		$phpTagPattern = '/^[ |\t]*[<\?php|\?>]+[ |\t]*/';
-		$basicallyEmptyPattern = '/^[ |\t]*[{|}|\(|\)]+[ |\t]*/';
-		$commentStart = '/\/\*\*/';
-		$commentEnd = '/\*\//';
-		$ignoreStart = '/@codeCoverageIgnoreStart/';
-		$ignoreStop = '/@codeCoverageIgnoreEnd/';
-		$inComment = false;
-
-		foreach ($lines as $lineno => $line) {
-			$runnable = true;
-			if (preg_match($phpTagPattern, $line) || preg_match($basicallyEmptyPattern, $line)) {
-				$runnable = false;
-			}
-			if ($runnable && preg_match($commentStart, $line)) {
-				$runnable = false;
-				$inComment = true;
-			}
-			if ($inComment == true) {
-				$runnable = false;
-			}
-			if (!$runnable && preg_match($commentEnd, $line)) {
-				$inComment = false;
-			}
-			$output[$lineno] = $runnable;
-		}
-		return $output;
-	}
-
 /**
  * Generates an HTML diff for $file based on $coverageData.
  *
@@ -191,23 +160,17 @@ class HtmlCoverageReport {
 		array_unshift($fileLines, ' ');
 		unset($fileLines[0]);
 
-		$executableLines = $this->getExecutableLines($fileLines);
-
 		foreach ($fileLines as $lineno => $line) {
-			$manualFind = (
-				isset($executableLines[$lineno]) && 
-				$executableLines[$lineno] == true &&
-				trim($line) != ''
-			);
-
 			$class = 'ignored';
-			if ($manualFind) {
+			if (isset($coverageData['covered'][$lineno])) {
+				$class = 'covered';
+				$covered++;
+				$total++;
+			} elseif (isset($coverageData['executable'][$lineno])) {
 				$class = 'uncovered';
 				$total++;
-				if (isset($coverageData[$lineno]) && $coverageData[$lineno] > 0) {
-					$class = 'covered';
-					$covered++;
-				}
+			} elseif (isset($coverageData['dead'][$lineno])) {
+				$class .= ' dead';
 			}
 			$diff[] = $this->_paintLine($line, $lineno, $class);
 		}