archive/phabricator
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Fix an unusual nonterminating task graph node

Browse Source 
Summary:
Fixes T12114. There were a couple of bugs here:

  - We could draw too many joining lines if a node had a parent with multiple descendants.
  - We could incorrectly ignore columns because of an `unset()`.

I //think// this fixes both things without collateral damage. This whole thing is a little hard to understand/debug and has grown beyond its original scope, so I'll probably rewrite it if there are more issues.

Test Plan:
  - Unit tests.
  - My local repro is clean now:

{F2424920}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12114

Differential Revision: https://secure.phabricator.com/D17211
This commit is contained in:
epriestley
2017-01-17 07:32:18 -08:00
parent a4a9485612
commit 48187cdbbe
2 changed files with 30 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/infrastructure/diff/view/PHUIDiffGraphView.php
View File

@@ -50,7 +50,6 @@ final class PHUIDiffGraphView extends Phobject {
$thread_count = $pos; $thread_count = $pos;
for ($n = 0; $n < $thread_count; $n++) { for ($n = 0; $n < $thread_count; $n++) {
if (empty($threads[$n])) { if (empty($threads[$n])) {
$line .= ' '; $line .= ' ';
continue; continue;
@@ -60,7 +59,7 @@ final class PHUIDiffGraphView extends Phobject {
if ($found) { if ($found) {
$line .= ' '; $line .= ' ';
$joins[] = $n; $joins[] = $n;
unset($threads[$n]); $threads[$n] = false;
} else { } else {
$line .= 'o'; $line .= 'o';
$found = true; $found = true;
@@ -114,6 +113,7 @@ final class PHUIDiffGraphView extends Phobject {
if ($thread_commit == $parent) { if ($thread_commit == $parent) {
$found = true; $found = true;
$splits[] = $idx; $splits[] = $idx;
break;
} }
} }

33
src/infrastructure/diff/view/__tests__/PHUIDiffGraphViewTestCase.php
View File

@@ -45,10 +45,10 @@ final class PHUIDiffGraphViewTestCase extends PhabricatorTestCase {
'^', '^',
'|^', '|^',
'o ', 'o ',
'|^', '| ^',
'||^', '| |^',
'o ', 'o ',
'x', 'x ',
); );
$this->assertGraph($picture, $graph, pht('Reverse Tree')); $this->assertGraph($picture, $graph, pht('Reverse Tree'));
@@ -71,7 +71,30 @@ final class PHUIDiffGraphViewTestCase extends PhabricatorTestCase {
'x ', 'x ',
); );
$this->assertGraph($picture, $graph, pht('Reverse Tree')); $this->assertGraph($picture, $graph, pht('Terminated Tree'));
}
public function testThreeWayGraphJoin() {
$nodes = array(
'A' => array('D', 'C', 'B'),
'B' => array('D'),
'C' => array('B', 'E', 'F'),
'D' => array(),
'E' => array(),
'F' => array(),
);
$graph = $this->newGraph($nodes);
$picture = array(
'^',
'||o',
'|o|',
'x| ||',
' | x|',
' | x',
);
$this->assertGraph($picture, $graph, pht('Three-Way Tree'));
} }
private function newGraph(array $nodes) { private function newGraph(array $nodes) {