Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions src/Progressable.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ trait Progressable {
*/
protected float $progress = 0;

/**
* Total steps for the process.
*/
protected ?int $totalSteps = null;

/**
* Current step of the process.
*/
protected int $currentStep = 0;

/**
* The callback function for saving cache data.
*
Expand Down Expand Up @@ -90,6 +100,64 @@ trait Progressable {
*/
protected mixed $onComplete = null;

/**
* Set the total number of steps.
*
* @return $this
*/
public function setTotalSteps(int $steps): static {
if ($steps <= 0) {
throw new \InvalidArgumentException('Total steps must be greater than 0');
}
$this->totalSteps = $steps;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset step state when redefining total steps

Calling setTotalSteps() updates only totalSteps and leaves currentStep untouched, so reusing the same instance for a new task carries over the old step counter. In that scenario, the next incrementStep() computes progress from stale state (often jumping straight to 100%), which makes the new run report incorrect progress data.

Useful? React with 👍 / 👎.


return $this;
}

/**
* Increment the current step.
*
* @return $this
*/
public function incrementStep(int $amount = 1): static {
if ($this->totalSteps === null) {
throw new \LogicException('Total steps not set. Call setTotalSteps() first.');
}
$this->currentStep += $amount;
$progress = ($this->currentStep / $this->totalSteps) * 100;

return $this->setLocalProgress($progress);
}

/**
* Set the current step.
*
* @return $this
*/
public function setStep(int $step): static {
if ($this->totalSteps === null) {
throw new \LogicException('Total steps not set. Call setTotalSteps() first.');
}
$this->currentStep = $step;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject negative step values in step-based APIs

setStep() accepts negative integers and stores them directly in currentStep, which can produce an invalid negative step count while local progress is clamped to 0. This creates inconsistent state (getCurrentStep() negative, getLocalProgress() non-negative) and can skew subsequent step calculations if a caller passes a computed value below zero.

Useful? React with 👍 / 👎.

$progress = ($this->currentStep / $this->totalSteps) * 100;

return $this->setLocalProgress($progress);
}

/**
* Get the total number of steps.
*/
public function getTotalSteps(): ?int {
return $this->totalSteps;
}

/**
* Get the current step.
*/
public function getCurrentStep(): int {
return $this->currentStep;
}

/**
* Set the callback function for saving cache data.
*
Expand Down
57 changes: 57 additions & 0 deletions tests/ProgressableTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -485,4 +485,61 @@ public function test_merge_metadata(): void {
$this->assertEquals('new_value1', $storedMetadata['key1']);
$this->assertEquals('value2', $storedMetadata['key2']);
}

public function test_set_total_steps(): void {
$this->setOverallUniqueName('test_steps_'.$this->testId);
$this->setTotalSteps(10);
$this->assertEquals(10, $this->getTotalSteps());
}

public function test_set_total_steps_exception(): void {
$this->expectException(\InvalidArgumentException::class);
$this->setTotalSteps(0);
}

public function test_increment_step(): void {
$this->setOverallUniqueName('test_inc_step_'.$this->testId);
$this->setTotalSteps(10);

$this->incrementStep();
$this->assertEquals(1, $this->getCurrentStep());
$this->assertEquals(10, $this->getLocalProgress());

$this->incrementStep(2);
$this->assertEquals(3, $this->getCurrentStep());
$this->assertEquals(30, $this->getLocalProgress());
}

public function test_set_step(): void {
$this->setOverallUniqueName('test_set_step_'.$this->testId);
$this->setTotalSteps(10);

$this->setStep(5);
$this->assertEquals(5, $this->getCurrentStep());
$this->assertEquals(50, $this->getLocalProgress());
}

public function test_increment_step_without_total_steps(): void {
$this->setOverallUniqueName('test_no_total_'.$this->testId);
$this->expectException(\LogicException::class);
$this->incrementStep();
}

public function test_step_progress_calculation(): void {
$this->setOverallUniqueName('test_step_calc_'.$this->testId);
$this->setTotalSteps(3);

$this->incrementStep();
// 1/3 = 33.333...
$this->assertEquals(33.33, $this->getLocalProgress(2));
}

public function test_step_exceeds_total(): void {
$this->setOverallUniqueName('test_step_exceed_'.$this->testId);
$this->setTotalSteps(10);

$this->setStep(11);
$this->assertEquals(11, $this->getCurrentStep());
$this->assertEquals(100, $this->getLocalProgress());
}
}