Fix rounding consistency and update tests per review feedback

- Ensure total = subtotal + tax by deriving total from rounded components
- Use assertEqualsWithDelta for float comparisons in tests
- Add defensive null coalescing in calculateSummary helper
- Add missing 'count' key to test data rows
- Add testRoundingAtBoundary test case
This commit is contained in:
Ollama
2026-03-14 16:26:36 +00:00
committed by jekkos
parent b49186ec7c
commit fda40d9340
2 changed files with 54 additions and 24 deletions

View File

@@ -50,11 +50,11 @@ class Summary_taxes extends Summary_report
if ($this->config['tax_included']) {
$sale_subtotal = "ROUND($sale_amount - $sale_tax, $decimals)";
$sale_total = "ROUND($sale_amount, $decimals)";
} else {
$sale_subtotal = "ROUND($sale_amount, $decimals)";
$sale_total = "ROUND($sale_amount + $sale_tax, $decimals)";
}
$sale_tax_rounded = "ROUND($sale_tax, $decimals)";
$sale_total = "($sale_subtotal + $sale_tax_rounded)";
$subquery_builder = $this->db->table('sales_items');
$subquery_builder->select(
@@ -62,7 +62,7 @@ class Summary_taxes extends Summary_report
. "CONCAT(IFNULL(ROUND(percent, $decimals), 0), '%') AS percent, "
. "sales.sale_id AS sale_id, "
. "$sale_subtotal AS subtotal, "
. "ROUND($sale_tax, $decimals) AS tax, "
. "$sale_tax_rounded AS tax, "
. "$sale_total AS total"
);

View File

@@ -18,9 +18,10 @@ class Summary_taxes_test extends CIUnitTestCase
foreach ($rows as $row) {
$calculatedTotal = round((float) $row['subtotal'] + (float) $row['tax'], 2);
$this->assertEquals(
$this->assertEqualsWithDelta(
(float) $row['total'],
$calculatedTotal,
0.001,
"Row subtotal + tax should equal total: {$row['subtotal']} + {$row['tax']} = {$row['total']}"
);
}
@@ -48,19 +49,22 @@ class Summary_taxes_test extends CIUnitTestCase
$expectedCount += (int) $row['count'];
}
$this->assertEquals(
$this->assertEqualsWithDelta(
round($expectedSubtotal, 2),
$summary['subtotal'],
0.001,
'Summary subtotal should equal sum of row subtotals'
);
$this->assertEquals(
$this->assertEqualsWithDelta(
round($expectedTax, 2),
$summary['tax'],
0.001,
'Summary tax should equal sum of row taxes'
);
$this->assertEquals(
$this->assertEqualsWithDelta(
round($expectedTotal, 2),
$summary['total'],
0.001,
'Summary total should equal sum of row totals'
);
$this->assertEquals(
@@ -80,9 +84,10 @@ class Summary_taxes_test extends CIUnitTestCase
$summary = $this->calculateSummary($rows, 2);
$calculatedTotal = round($summary['subtotal'] + $summary['tax'], 2);
$this->assertEquals(
$this->assertEqualsWithDelta(
$summary['total'],
$calculatedTotal,
0.001,
'Summary subtotal + tax should equal summary total'
);
}
@@ -90,8 +95,8 @@ class Summary_taxes_test extends CIUnitTestCase
public function testRoundingConsistency(): void
{
$rows = [
['subtotal' => 99.99, 'tax' => 9.999, 'total' => 109.989],
['subtotal' => 33.33, 'tax' => 3.333, 'total' => 36.663],
['subtotal' => 99.99, 'tax' => 9.999, 'total' => 109.989, 'count' => 1],
['subtotal' => 33.33, 'tax' => 3.333, 'total' => 36.663, 'count' => 1],
];
$summary = $this->calculateSummary($rows, 2);
@@ -106,9 +111,10 @@ class Summary_taxes_test extends CIUnitTestCase
$expectedTotal = round($sumOfRoundedSubtotals + $sumOfRoundedTaxes, 2);
$this->assertEquals(
$this->assertEqualsWithDelta(
$expectedTotal,
$summary['total'],
0.001,
'Total should be sum of rounded subtotals + sum of rounded taxes'
);
}
@@ -156,18 +162,19 @@ class Summary_taxes_test extends CIUnitTestCase
foreach ($rows as $row) {
$calculatedTotal = round((float) $row['subtotal'] + (float) $row['tax'], 2);
$this->assertEquals(
$this->assertEqualsWithDelta(
(float) $row['total'],
$calculatedTotal,
0.001,
"Negative row: subtotal + tax should equal total"
);
}
$summary = $this->calculateSummary($rows, 2);
$this->assertEquals(100.00, $summary['subtotal'], 'Summary should handle negative values');
$this->assertEquals(10.00, $summary['tax'], 'Summary tax should handle negative values');
$this->assertEquals(110.00, $summary['total'], 'Summary total should handle negative values');
$this->assertEqualsWithDelta(100.00, $summary['subtotal'], 0.001, 'Summary should handle negative values');
$this->assertEqualsWithDelta(10.00, $summary['tax'], 0.001, 'Summary tax should handle negative values');
$this->assertEqualsWithDelta(110.00, $summary['total'], 0.001, 'Summary total should handle negative values');
}
public function testZeroTaxRows(): void
@@ -178,22 +185,45 @@ class Summary_taxes_test extends CIUnitTestCase
];
foreach ($rows as $row) {
$this->assertEquals(
$this->assertEqualsWithDelta(
(float) $row['subtotal'],
(float) $row['total'],
0.001,
'Zero tax: subtotal should equal total'
);
}
$summary = $this->calculateSummary($rows, 2);
$this->assertEquals(
$this->assertEqualsWithDelta(
$summary['subtotal'],
$summary['total'],
0.001,
'Zero tax summary: subtotal should equal total'
);
}
public function testRoundingAtBoundary(): void
{
$rows = [
['subtotal' => 10.005, 'tax' => 0.003, 'total' => 10.008, 'count' => 1],
['subtotal' => 5.004, 'tax' => 0.002, 'total' => 5.006, 'count' => 1],
];
foreach ($rows as $row) {
$roundedSubtotal = round((float) $row['subtotal'], 2);
$roundedTax = round((float) $row['tax'], 2);
$expectedTotal = round($roundedSubtotal + $roundedTax, 2);
$this->assertEqualsWithDelta(
$expectedTotal,
round((float) $row['total'], 2),
0.001,
'Rounded subtotal + rounded tax should equal rounded total'
);
}
}
private function calculateSummary(array $rows, int $decimals = 2): array
{
$subtotal = 0;
@@ -202,17 +232,17 @@ class Summary_taxes_test extends CIUnitTestCase
$count = 0;
foreach ($rows as $row) {
$subtotal += (float) $row['subtotal'];
$tax += (float) $row['tax'];
$total += (float) $row['total'];
$count += (int) $row['count'];
$subtotal += (float) ($row['subtotal'] ?? 0);
$tax += (float) ($row['tax'] ?? 0);
$total += (float) ($row['total'] ?? 0);
$count += (int) ($row['count'] ?? 0);
}
return [
'subtotal' => round($subtotal, $decimals),
'tax' => round($tax, $decimals),
'total' => round($total, $decimals),
'count' => $count,
'tax' => round($tax, $decimals),
'total' => round($total, $decimals),
'count' => $count,
];
}
}