* Fixed undefined index in merge() method

* Removed code from merge() to yield correct merge results * Added test case for merging coverage files in reverse order (to prevent undefined index again) * Changed merge test cases to assert against the expected output from a single coverage run, instead of the currently "bugged" result.
WesB0sman · May 22, 2018 · 6f7ed2f · 6f7ed2f
1 parent bf912b6
commit 6f7ed2f
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 30 deletions.
diff --git a/src/CodeCoverage.php b/src/CodeCoverage.php
@@ -380,30 +380,8 @@ public function merge(self $that): void
                 continue;
             }
 
-            if ((\count($lines) > 0) && (\count($this->data[$file]) > 0) && (\count($lines) != \count($this->data[$file]))) {
-                if (\count($lines) > \count($ours = $this->data[$file])) {
-                    // More lines in the one being added in
-                    $lines = \array_filter(
-                        $lines,
-                        function ($value, $key) use ($ours) {
-                            return \array_key_exists($key, $ours);
-                        },
-                        \ARRAY_FILTER_USE_BOTH
-                    );
-                } else {
-                    // More lines in the one we currently have
-                    $this->data[$file] = \array_filter(
-                        $this->data[$file],
-                        function ($value, $key) use ($lines) {
-                            return \array_key_exists($key, $lines);
-                        },
-                        \ARRAY_FILTER_USE_BOTH
-                    );
-                }
-            }
-
             foreach ($lines as $line => $data) {
-                if ($data === null || $this->data[$file][$line] === null) {
+                if ($data === null || (array_key_exists($line, $this->data[$file]) && $this->data[$file][$line] === null)) {
                     // if the line is marked as "dead code" in either, mark it as dead code in the merged result
                     $this->data[$file][$line] = null;
                 } elseif (!isset($this->data[$file][$line])) {

diff --git a/tests/TestCase.php b/tests/TestCase.php
@@ -238,33 +238,39 @@ protected function getExpectedDataArrayForBankAccount()
                     0 => 'BankAccountTest::testBalanceIsInitiallyZero',
                     1 => 'BankAccountTest::testDepositWithdrawMoney'
                 ],
+                9  => null,
                 13 => [],
+                14 => [],
+                15 => [],
                 16 => [],
+                18 => [],
                 22 => [
                     0 => 'BankAccountTest::testBalanceCannotBecomeNegative2',
                     1 => 'BankAccountTest::testDepositWithdrawMoney'
                 ],
                 24 => [
                     0 => 'BankAccountTest::testDepositWithdrawMoney',
                 ],
+                25 => null,
                 29 => [
                     0 => 'BankAccountTest::testBalanceCannotBecomeNegative',
                     1 => 'BankAccountTest::testDepositWithdrawMoney'
                 ],
                 31 => [
                     0 => 'BankAccountTest::testDepositWithdrawMoney'
                 ],
+                32 => null
             ]
         ];
     }
 
-    protected function getExpectedDataArrayForBankAccount2()
+    protected function getExpectedDataArrayForBankAccountInReverseOrder()
     {
         return [
             TEST_FILES_PATH . 'BankAccount.php' => [
                 8 => [
-                    0 => 'BankAccountTest::testBalanceIsInitiallyZero',
-                    1 => 'BankAccountTest::testDepositWithdrawMoney'
+                    0 => 'BankAccountTest::testDepositWithdrawMoney',
+                    1 => 'BankAccountTest::testBalanceIsInitiallyZero'
                 ],
                 9  => null,
                 13 => [],
@@ -281,8 +287,8 @@ protected function getExpectedDataArrayForBankAccount2()
                 ],
                 25 => null,
                 29 => [
-                    0 => 'BankAccountTest::testBalanceCannotBecomeNegative',
-                    1 => 'BankAccountTest::testDepositWithdrawMoney'
+                    0 => 'BankAccountTest::testDepositWithdrawMoney',
+                    1 => 'BankAccountTest::testBalanceCannotBecomeNegative'
                 ],
                 31 => [
                     0 => 'BankAccountTest::testDepositWithdrawMoney'

diff --git a/tests/tests/CodeCoverageTest.php b/tests/tests/CodeCoverageTest.php
@@ -215,7 +215,7 @@ public function testCollect()
         $coverage = $this->getCoverageForBankAccount();
 
         $this->assertEquals(
-            $this->getExpectedDataArrayForBankAccount2(),
+            $this->getExpectedDataArrayForBankAccount(),
             $coverage->getData()
         );
 
@@ -241,6 +241,17 @@ public function testMerge()
         );
     }
 
+    public function testMergeReverseOrder()
+    {
+        $coverage = $this->getCoverageForBankAccountForLastTwoTests();
+        $coverage->merge($this->getCoverageForBankAccountForFirstTwoTests());
+
+        $this->assertEquals(
+            $this->getExpectedDataArrayForBankAccountInReverseOrder(),
+            $coverage->getData()
+        );
+    }
+
     public function testMerge2()
     {
         $coverage = new CodeCoverage(
@@ -251,7 +262,7 @@ public function testMerge2()
         $coverage->merge($this->getCoverageForBankAccount());
 
         $this->assertEquals(
-            $this->getExpectedDataArrayForBankAccount2(),
+            $this->getExpectedDataArrayForBankAccount(),
             $coverage->getData()
         );
     }