Merge pull request #597 from tienvx/fix-record-bug-twice

Fix record bug twice
tienvx · Nov 10, 2021 · c0582ce · c0582ce
2 parents 4f64a04 + 8efbb21
commit c0582ce
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 72 deletions.
diff --git a/doc/diagram/messages.plantuml b/doc/diagram/messages.plantuml
@@ -0,0 +1,45 @@
+@startuml
+start
+:Reduce Bug;
+:Dispatch Messages;
+if (Count Messages > 0) then (yes)
+  :Increase bug messages total;
+  stop
+elseif (Finish all messages) then (yes)
+  :Stop reducing;
+  :Record and report;
+  stop
+else
+  stop
+endif
+
+start
+:Reduce Steps;
+if (Short Length) then (yes)
+  stop
+else
+  group Handle steps
+    :Create new steps;
+    if (Steps too long) then (yes)
+      stop
+    else
+      :Run new steps;
+      if (Got exact same bug) then (yes)
+        group Update bug
+          :Stop reducing;
+          :Set total = 0;
+          :Set processed = 0;
+          :Set steps;
+        end group
+        :Dispatch reduce bug again;
+      endif
+    endif
+  end group
+  :Increase bug messages processed;
+  if (Stopped Reducing) then (yes)
+    /':Stop reducing; '/
+    :Record and report;
+    stop
+  else
+    stop
+@enduml
diff --git a/src/Entity/Bug.php b/src/Entity/Bug.php
@@ -59,11 +59,6 @@ class Bug extends BugModel
      */
     protected bool $closed = false;
 
-    /**
-     * @ORM\Column(type="boolean")
-     */
-    protected bool $reducing = false;
-
     /**
      * @ORM\Column(name="created_at", type="datetime")
      */

diff --git a/src/Model/Bug.php b/src/Model/Bug.php
@@ -14,7 +14,6 @@ abstract class Bug implements BugInterface
     protected string $message;
     protected ProgressInterface $progress;
     protected bool $closed = false;
-    protected bool $reducing = false;
     protected DateTimeInterface $updatedAt;
     protected DateTimeInterface $createdAt;
 
@@ -101,16 +100,6 @@ public function setClosed(bool $closed): void
         $this->closed = $closed;
     }
 
-    public function isReducing(): bool
-    {
-        return $this->reducing;
-    }
-
-    public function setReducing(bool $reducing): void
-    {
-        $this->reducing = $reducing;
-    }
-
     public function setCreatedAt(DateTimeInterface $createdAt): void
     {
         $this->createdAt = $createdAt;

diff --git a/src/Model/BugInterface.php b/src/Model/BugInterface.php
@@ -40,10 +40,6 @@ public function isClosed(): bool;
 
     public function setClosed(bool $closed): void;
 
-    public function isReducing(): bool;
-
-    public function setReducing(bool $reducing): void;
-
     public function setCreatedAt(DateTimeInterface $createdAt): void;
 
     public function getCreatedAt(): DateTimeInterface;

diff --git a/src/Reducer/HandlerTemplate.php b/src/Reducer/HandlerTemplate.php
@@ -82,7 +82,6 @@ public function updateSteps(BugInterface $bug, array $newSteps): void
 
             if (count($newSteps) <= count($bug->getSteps())) {
                 $this->entityManager->lock($bug, LockMode::PESSIMISTIC_WRITE);
-                $bug->setReducing(false);
                 $bug->getProgress()->setTotal(0);
                 $bug->getProgress()->setProcessed(0);
                 $bug->setSteps($newSteps);

diff --git a/src/Service/Bug/BugHelper.php b/src/Service/Bug/BugHelper.php
@@ -7,7 +7,6 @@
 use Throwable;
 use Tienvx\Bundle\MbtBundle\Entity\Bug;
 use Tienvx\Bundle\MbtBundle\Exception\ExceptionInterface;
-use Tienvx\Bundle\MbtBundle\Exception\RuntimeException;
 use Tienvx\Bundle\MbtBundle\Exception\UnexpectedValueException;
 use Tienvx\Bundle\MbtBundle\Message\RecordVideoMessage;
 use Tienvx\Bundle\MbtBundle\Message\ReportBugMessage;
@@ -51,15 +50,13 @@ public function __construct(
     public function reduceBug(int $bugId): void
     {
         $bug = $this->getBug($bugId, 'reduce bug');
-        $this->startReducing($bug);
 
         $reducer = $this->reducerManager->getReducer($this->config->getReducer());
         $messagesCount = $reducer->dispatch($bug);
-        if (0 === $messagesCount && $bug->getProgress()->getProcessed() === $bug->getProgress()->getTotal()) {
-            $this->stopReducing($bug);
-            $this->recordAndReport($bug);
-        } elseif ($messagesCount > 0) {
+        if ($messagesCount > 0) {
             $this->bugProgress->increaseTotal($bug, $messagesCount);
+        } elseif ($bug->getProgress()->getProcessed() === $bug->getProgress()->getTotal()) {
+            $this->recordAndReport($bug);
         }
     }
 
@@ -75,8 +72,11 @@ public function reduceSteps(int $bugId, int $length, int $from, int $to): void
         $reducer = $this->reducerManager->getReducer($this->config->getReducer());
         $reducer->handle($bug, $from, $to);
 
-        $this->bugProgress->increaseProcessed($bug, 1);
-        if (!$bug->isReducing()) {
+        $this->bugProgress->increaseProcessed($bug);
+        if (
+            $bug->getProgress()->getTotal() > 0
+            && $bug->getProgress()->getProcessed() === $bug->getProgress()->getTotal()
+        ) {
             $this->recordAndReport($bug);
         }
     }
@@ -128,24 +128,6 @@ protected function getBug(int $bugId, string $action): BugInterface
         return $bug;
     }
 
-    protected function startReducing(BugInterface $bug): void
-    {
-        if ($bug->isReducing()) {
-            throw new RuntimeException(sprintf('Bug %d is already reducing', $bug->getId()));
-        } else {
-            $bug->setReducing(true);
-            $this->entityManager->flush();
-        }
-    }
-
-    protected function stopReducing(BugInterface $bug): void
-    {
-        $bug->setReducing(false);
-        // Reducing bug take long time. Reconnect to flush changes.
-        $this->entityManager->getConnection()->connect();
-        $this->entityManager->flush();
-    }
-
     protected function recordAndReport(BugInterface $bug): void
     {
         $this->messageBus->dispatch(new RecordVideoMessage($bug->getId()));

diff --git a/src/Service/Bug/BugProgress.php b/src/Service/Bug/BugProgress.php
@@ -23,9 +23,6 @@ public function increaseProcessed(BugInterface $bug, int $processed = 1): void
 
             $this->entityManager->lock($bug, LockMode::PESSIMISTIC_WRITE);
             $bug->getProgress()->increase($processed);
-            if ($bug->getProgress()->getProcessed() === $bug->getProgress()->getTotal()) {
-                $bug->setReducing(false);
-            }
         };
 
         $this->entityManager->transactional($callback);

diff --git a/tests/Reducer/HandlerTestCase.php b/tests/Reducer/HandlerTestCase.php
@@ -177,7 +177,6 @@ public function testRunFoundSameBug(): void
             ));
         $this->handler->handle($this->bug, 1, 2);
         $this->assertSteps($this->newSteps, $this->bug->getSteps());
-        $this->assertFalse($this->bug->isReducing());
         $this->assertSame(0, $this->bug->getProgress()->getProcessed());
         $this->assertSame(0, $this->bug->getProgress()->getTotal());
     }

diff --git a/tests/Service/Bug/BugHelperTest.php b/tests/Service/Bug/BugHelperTest.php
@@ -89,7 +89,7 @@ protected function setUp(): void
         $this->task->setModelRevision($this->revision);
         $this->progress = new Progress();
         $this->progress->setTotal(10);
-        $this->progress->setProcessed(10);
+        $this->progress->setProcessed(9);
         $this->bug = new Bug();
         $this->bug->setProgress($this->progress);
         $this->bug->setId(123);
@@ -99,7 +99,6 @@ protected function setUp(): void
             $this->createMock(StepInterface::class),
             $this->createMock(StepInterface::class),
         ]);
-        $this->bug->setReducing(false);
         $this->driver = $this->createMock(RemoteWebDriver::class);
         $this->capabilities = new DesiredCapabilities();
     }
@@ -126,22 +125,33 @@ public function testReduceMissingBug(): void
         $this->helper->reduceBug(123);
     }
 
-    public function testNotFinishReduceBug(): void
+    public function testReduceBugDispatchMessages(): void
     {
         $reducer = $this->createMock(ReducerInterface::class);
         $reducer->expects($this->once())->method('dispatch')->with($this->bug)->willReturn(5);
         $this->config->expects($this->once())->method('getReducer')->willReturn('random');
         $this->reducerManager->expects($this->once())->method('getReducer')->with('random')->willReturn($reducer);
         $this->messageBus->expects($this->never())->method('dispatch');
         $this->entityManager->expects($this->once())->method('find')->with(Bug::class, 123)->willReturn($this->bug);
-        $this->entityManager->expects($this->once())->method('flush');
         $this->bugProgress->expects($this->once())->method('increaseTotal')->with($this->bug, 5);
         $this->helper->reduceBug(123);
-        $this->assertTrue($this->bug->isReducing());
+    }
+
+    public function testReduceBugNotDispatchMessages(): void
+    {
+        $reducer = $this->createMock(ReducerInterface::class);
+        $reducer->expects($this->once())->method('dispatch')->with($this->bug)->willReturn(0);
+        $this->config->expects($this->once())->method('getReducer')->willReturn('random');
+        $this->reducerManager->expects($this->once())->method('getReducer')->with('random')->willReturn($reducer);
+        $this->messageBus->expects($this->never())->method('dispatch');
+        $this->entityManager->expects($this->once())->method('find')->with(Bug::class, 123)->willReturn($this->bug);
+        $this->bugProgress->expects($this->never())->method('increaseTotal');
+        $this->helper->reduceBug(123);
     }
 
     public function testFinishReduceBug(): void
     {
+        $this->progress->setProcessed(10);
         $reducer = $this->createMock(ReducerInterface::class);
         $reducer->expects($this->once())->method('dispatch')->with($this->bug)->willReturn(0);
         $this->config->expects($this->once())->method('getReducer')->willReturn('random');
@@ -159,12 +169,8 @@ public function testFinishReduceBug(): void
             }))
             ->willReturn(new Envelope(new \stdClass()));
         $this->entityManager->expects($this->once())->method('find')->with(Bug::class, 123)->willReturn($this->bug);
-        $this->entityManager->expects($this->exactly(2))->method('flush');
-        $this->connection->expects($this->once())->method('connect');
-        $this->entityManager->expects($this->once())->method('getConnection')->willReturn($this->connection);
         $this->bugProgress->expects($this->never())->method('increaseTotal');
         $this->helper->reduceBug(123);
-        $this->assertFalse($this->bug->isReducing());
     }
 
     public function testReportMissingBug(): void
@@ -209,7 +215,6 @@ public function testReduceReducedSteps(): void
 
     public function testNotStopReduceSteps(): void
     {
-        $this->progress->setProcessed(5);
         $this->bug->setSteps([
             $this->createMock(StepInterface::class),
             $this->createMock(StepInterface::class),
@@ -222,11 +227,7 @@ public function testNotStopReduceSteps(): void
         $this->reducerManager->expects($this->once())->method('getReducer')->with('random')->willReturn($reducer);
         $this->messageBus->expects($this->never())->method('dispatch');
         $this->entityManager->expects($this->once())->method('find')->with(Bug::class, 123)->willReturn($this->bug);
-        $this->bugProgress
-            ->expects($this->once())
-            ->method('increaseProcessed')
-            ->with($this->bug, 1)
-            ->willReturnCallback(fn () => $this->bug->setReducing(true));
+        $this->bugProgress->expects($this->once())->method('increaseProcessed')->with($this->bug, 1);
         $this->helper->reduceSteps(123, 4, 1, 2);
     }
 
@@ -250,7 +251,11 @@ public function testStopReduceStepsAndRecordVideo(): void
             ))
             ->willReturn(new Envelope(new \stdClass()));
         $this->entityManager->expects($this->once())->method('find')->with(Bug::class, 123)->willReturn($this->bug);
-        $this->bugProgress->expects($this->once())->method('increaseProcessed')->with($this->bug, 1);
+        $this->bugProgress
+            ->expects($this->once())
+            ->method('increaseProcessed')
+            ->with($this->bug, 1)
+            ->willReturnCallback(fn () => $this->progress->setProcessed(10));
         $this->helper->reduceSteps(123, 4, 1, 2);
     }
 
@@ -284,7 +289,11 @@ public function testStopReduceStepsAndReportBug(): void
             )
             ->willReturn(new Envelope(new \stdClass()));
         $this->entityManager->expects($this->once())->method('find')->with(Bug::class, 123)->willReturn($this->bug);
-        $this->bugProgress->expects($this->once())->method('increaseProcessed')->with($this->bug, 1);
+        $this->bugProgress
+            ->expects($this->once())
+            ->method('increaseProcessed')
+            ->with($this->bug, 1)
+            ->willReturnCallback(fn () => $this->progress->setProcessed(10));
         $this->helper->reduceSteps(123, 4, 1, 2);
     }
 

diff --git a/tests/Service/Bug/BugProgressTest.php b/tests/Service/Bug/BugProgressTest.php
@@ -29,7 +29,6 @@ protected function setUp(): void
         $progress->setProcessed(5);
         $this->bug = new Bug();
         $this->bug->setProgress($progress);
-        $this->bug->setReducing(true);
     }
 
     public function testIncreaseProcessed(): void
@@ -48,7 +47,6 @@ public function testIncreaseProcessed(): void
         $bugProgress->increaseProcessed($this->bug, 2);
         $this->assertSame(7, $this->bug->getProgress()->getProcessed());
         $this->assertSame(10, $this->bug->getProgress()->getTotal());
-        $this->assertTrue($this->bug->isReducing());
     }
 
     public function testIncreaseProcessedReachLimit(): void
@@ -67,7 +65,6 @@ public function testIncreaseProcessedReachLimit(): void
         $bugProgress->increaseProcessed($this->bug, 6);
         $this->assertSame(10, $this->bug->getProgress()->getProcessed());
         $this->assertSame(10, $this->bug->getProgress()->getTotal());
-        $this->assertFalse($this->bug->isReducing());
     }
 
     public function testIncreaseTotal(): void
@@ -86,6 +83,5 @@ public function testIncreaseTotal(): void
         $bugProgress->increaseTotal($this->bug, 3);
         $this->assertSame(5, $this->bug->getProgress()->getProcessed());
         $this->assertSame(13, $this->bug->getProgress()->getTotal());
-        $this->assertTrue($this->bug->isReducing());
     }
 }