Skip to content

Commit cb8ee73

Browse files
oleibmanoleibman
authored
Cache Results for Tests Involving NOW and TODAY Functions (#2311)
My bulletproofing of these tests was not yet sufficient. Although I have never had a failure in probably thousands of tests, one user submitted a PR which did fail testing NOW, fortunately not in a test that is required to pass. The problem is that it is not sufficient merely to set the cell value inside a do-while loop; it is necessary to calculate it in order to cache its result so that results based on that cell will be internally consistent. No source code is changed for this PR, just some tests.
1 parent 4001c89 commit cb8ee73

File tree

8 files changed

+69
-45
lines changed

8 files changed

+69
-45
lines changed

‎tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/NowTest.php‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,26 +9,30 @@ class NowTest extends AllSetupTeardown
99
public function testNow(): void
1010
{
1111
$sheet = $this->getSheet();
12+
$row = 0;
1213
// Loop to avoid rare edge case where first calculation
1314
// and second do not take place in same second.
1415
do {
16+
++$row;
1517
$dtStart = new DateTimeImmutable();
1618
$startSecond = $dtStart->format('s');
17-
$sheet->setCellValue('A1', '=NOW()');
19+
$sheet->setCellValue("A$row", '=NOW()');
20+
// cache result for later assertions
21+
$sheet->getCell("A$row")->getCalculatedValue();
1822
$dtEnd = new DateTimeImmutable();
1923
$endSecond = $dtEnd->format('s');
2024
} while ($startSecond !== $endSecond);
21-
$sheet->setCellValue('B1', '=YEAR(A1)');
22-
$sheet->setCellValue('C1', '=MONTH(A1)');
23-
$sheet->setCellValue('D1', '=DAY(A1)');
24-
$sheet->setCellValue('E1', '=HOUR(A1)');
25-
$sheet->setCellValue('F1', '=MINUTE(A1)');
26-
$sheet->setCellValue('G1', '=SECOND(A1)');
27-
self::assertEquals($dtStart->format('Y'), $sheet->getCell('B1')->getCalculatedValue());
28-
self::assertEquals($dtStart->format('m'), $sheet->getCell('C1')->getCalculatedValue());
29-
self::assertEquals($dtStart->format('d'), $sheet->getCell('D1')->getCalculatedValue());
30-
self::assertEquals($dtStart->format('H'), $sheet->getCell('E1')->getCalculatedValue());
31-
self::assertEquals($dtStart->format('i'), $sheet->getCell('F1')->getCalculatedValue());
32-
self::assertEquals($dtStart->format('s'), $sheet->getCell('G1')->getCalculatedValue());
25+
$sheet->setCellValue("B$row", "=YEAR(A$row)");
26+
$sheet->setCellValue("C$row", "=MONTH(A$row)");
27+
$sheet->setCellValue("D$row", "=DAY(A$row)");
28+
$sheet->setCellValue("E$row", "=HOUR(A$row)");
29+
$sheet->setCellValue("F$row", "=MINUTE(A$row)");
30+
$sheet->setCellValue("G$row", "=SECOND(A$row)");
31+
self::assertEquals($dtStart->format('Y'), $sheet->getCell("B$row")->getCalculatedValue());
32+
self::assertEquals($dtStart->format('m'), $sheet->getCell("C$row")->getCalculatedValue());
33+
self::assertEquals($dtStart->format('d'), $sheet->getCell("D$row")->getCalculatedValue());
34+
self::assertEquals($dtStart->format('H'), $sheet->getCell("E$row")->getCalculatedValue());
35+
self::assertEquals($dtStart->format('i'), $sheet->getCell("F$row")->getCalculatedValue());
36+
self::assertEquals($dtStart->format('s'), $sheet->getCell("G$row")->getCalculatedValue());
3337
}
3438
}

‎tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/TodayTest.php‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,26 +9,30 @@ class TodayTest extends AllSetupTeardown
99
public function testToday(): void
1010
{
1111
$sheet = $this->getSheet();
12+
$row = 0;
1213
// Loop to avoid rare edge case where first calculation
1314
// and second do not take place in same second.
1415
do {
16+
++$row;
1517
$dtStart = new DateTimeImmutable();
1618
$startSecond = $dtStart->format('s');
17-
$sheet->setCellValue('A1', '=TODAY()');
19+
$sheet->setCellValue("A$row", '=TODAY()');
20+
// cache result for later assertions
21+
$sheet->getCell("A$row")->getCalculatedValue();
1822
$dtEnd = new DateTimeImmutable();
1923
$endSecond = $dtEnd->format('s');
2024
} while ($startSecond !== $endSecond);
21-
$sheet->setCellValue('B1', '=YEAR(A1)');
22-
$sheet->setCellValue('C1', '=MONTH(A1)');
23-
$sheet->setCellValue('D1', '=DAY(A1)');
24-
$sheet->setCellValue('E1', '=HOUR(A1)');
25-
$sheet->setCellValue('F1', '=MINUTE(A1)');
26-
$sheet->setCellValue('G1', '=SECOND(A1)');
27-
self::assertSame((int) $dtStart->format('Y'), $sheet->getCell('B1')->getCalculatedValue());
28-
self::assertSame((int) $dtStart->format('m'), $sheet->getCell('C1')->getCalculatedValue());
29-
self::assertSame((int) $dtStart->format('d'), $sheet->getCell('D1')->getCalculatedValue());
30-
self::assertSame(0, $sheet->getCell('E1')->getCalculatedValue());
31-
self::assertSame(0, $sheet->getCell('F1')->getCalculatedValue());
32-
self::assertSame(0, $sheet->getCell('G1')->getCalculatedValue());
25+
$sheet->setCellValue("B$row", "=YEAR(A$row)");
26+
$sheet->setCellValue("C$row", "=MONTH(A$row)");
27+
$sheet->setCellValue("D$row", "=DAY(A$row)");
28+
$sheet->setCellValue("E$row", "=HOUR(A$row)");
29+
$sheet->setCellValue("F$row", "=MINUTE(A$row)");
30+
$sheet->setCellValue("G$row", "=SECOND(A$row)");
31+
self::assertSame((int) $dtStart->format('Y'), $sheet->getCell("B$row")->getCalculatedValue());
32+
self::assertSame((int) $dtStart->format('m'), $sheet->getCell("C$row")->getCalculatedValue());
33+
self::assertSame((int) $dtStart->format('d'), $sheet->getCell("D$row")->getCalculatedValue());
34+
self::assertSame(0, $sheet->getCell("E$row")->getCalculatedValue());
35+
self::assertSame(0, $sheet->getCell("F$row")->getCalculatedValue());
36+
self::assertSame(0, $sheet->getCell("G$row")->getCalculatedValue());
3337
}
3438
}

‎tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterMonthTest.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ private function setCells(Worksheet $sheet, int $startMonth): void
2222
{
2323
$sheet->getCell('A1')->setValue('Date');
2424
$sheet->getCell('A2')->setValue('=TODAY()');
25+
// cache result for consistency in later calculations
26+
$sheet->getCell('A2')->getCalculatedValue();
2527
$sheet->getCell('A3')->setValue('=DATE(YEAR(A2), MONTH(A2), 1)');
2628
if ($startMonth === 12) {
2729
$sheet->getCell('A4')->setValue('=DATE(YEAR(A2) + 1, 1, 1)');
@@ -56,7 +58,7 @@ public function testMonths(array $expectedVisible, string $rule): void
5658
// Loop to avoid rare edge case where first calculation
5759
// and second do not take place in same day.
5860
do {
59-
$sheet = $this->getSheet();
61+
$sheet = $this->getSpreadsheet()->createSheet();
6062
$dtStart = new DateTimeImmutable();
6163
$startDay = (int) $dtStart->format('d');
6264
$startMonth = (int) $dtStart->format('m');
@@ -79,6 +81,6 @@ public function testMonths(array $expectedVisible, string $rule): void
7981
$endDay = (int) $dtEnd->format('d');
8082
} while ($startDay !== $endDay);
8183

82-
self::assertEquals($expectedVisible, $this->getVisible());
84+
self::assertEquals($expectedVisible, $this->getVisibleSheet($sheet));
8385
}
8486
}

‎tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterQuarterTest.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use DateTimeImmutable;
66
use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column;
77
use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column\Rule;
8+
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
89

910
class AutoFilterQuarterTest extends SetupTeardown
1011
{
@@ -17,11 +18,12 @@ public function providerQuarter(): array
1718
];
1819
}
1920

20-
private function setCells(): void
21+
private function setCells(Worksheet $sheet): void
2122
{
22-
$sheet = $this->getSheet();
2323
$sheet->getCell('A1')->setValue('Date');
2424
$sheet->getCell('A2')->setValue('=TODAY()');
25+
// cache result for consistency in later calculations
26+
$sheet->getCell('A2')->getCalculatedValue();
2527
$sheet->getCell('A3')->setValue('=DATE(YEAR(A2), MONTH(A2), 1)');
2628
$sheet->getCell('A4')->setValue('=DATE(YEAR(A2), MONTH(A2) + 3, 1)');
2729
$sheet->getCell('A5')->setValue('=DATE(YEAR(A2), MONTH(A2) + 6, 1)');
@@ -40,10 +42,10 @@ public function testQuarters(array $expectedVisible, string $rule): void
4042
// Loop to avoid rare edge case where first calculation
4143
// and second do not take place in same day.
4244
do {
43-
$sheet = $this->getSheet();
45+
$sheet = $this->getSpreadsheet()->createSheet();
4446
$dtStart = new DateTimeImmutable();
4547
$startDay = (int) $dtStart->format('d');
46-
$this->setCells();
48+
$this->setCells($sheet);
4749

4850
$maxRow = $this->maxRow;
4951
$autoFilter = $sheet->getAutoFilter();
@@ -62,6 +64,6 @@ public function testQuarters(array $expectedVisible, string $rule): void
6264
$endDay = (int) $dtEnd->format('d');
6365
} while ($startDay !== $endDay);
6466

65-
self::assertEquals($expectedVisible, $this->getVisible());
67+
self::assertEquals($expectedVisible, $this->getVisibleSheet($sheet));
6668
}
6769
}

‎tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterTodayTest.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,18 @@ public function testYesterdayTodayTomorrow(array $expectedVisible, string $rule)
2525
// Loop to avoid rare edge case where first calculation
2626
// and second do not take place in same day.
2727
do {
28-
$sheet = $this->getSheet();
28+
$sheet = $this->getSpreadsheet()->createSheet();
2929
$dtStart = new DateTimeImmutable();
3030
$startDay = $dtStart->format('d');
3131
$sheet->getCell('A1')->setValue('Date');
3232
$sheet->getCell('A2')->setValue('=NOW()');
33+
// cache result for consistency in later calculations
34+
$sheet->getCell('A2')->getCalculatedValue();
3335
$sheet->getCell('A3')->setValue('=A2+1');
3436
$sheet->getCell('A4')->setValue('=A2-1');
3537
$sheet->getCell('A5')->setValue('=TODAY()');
38+
// cache result for consistency in later calculations
39+
$sheet->getCell('A5')->getCalculatedValue();
3640
$sheet->getCell('A6')->setValue('=A5+1');
3741
$sheet->getCell('A7')->setValue('=A5-1');
3842
$this->maxRow = $maxRow = 7;
@@ -52,6 +56,6 @@ public function testYesterdayTodayTomorrow(array $expectedVisible, string $rule)
5256
$endDay = $dtEnd->format('d');
5357
} while ($startDay !== $endDay);
5458

55-
self::assertEquals($expectedVisible, $this->getVisible());
59+
self::assertEquals($expectedVisible, $this->getVisibleSheet($sheet));
5660
}
5761
}

‎tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterWeekTest.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use DateTimeImmutable;
66
use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column;
77
use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column\Rule;
8+
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
89

910
class AutoFilterWeekTest extends SetupTeardown
1011
{
@@ -17,11 +18,12 @@ public function providerWeek(): array
1718
];
1819
}
1920

20-
private function setCells(): void
21+
private function setCells(Worksheet $sheet): void
2122
{
22-
$sheet = $this->getSheet();
2323
$sheet->getCell('A1')->setValue('Date');
2424
$sheet->getCell('A2')->setValue('=TODAY()');
25+
// cache result for consistency in later calculations
26+
$sheet->getCell('A2')->getCalculatedValue();
2527
$sheet->getCell('B2')->setValue('=WEEKDAY(A2) - 1'); // subtract to get to Sunday
2628
$sheet->getCell('A3')->setValue('=DATE(YEAR(A2), MONTH(A2), DAY(A2) - B2)');
2729
$sheet->getCell('A4')->setValue('=DATE(YEAR(A3), MONTH(A3), DAY(A3) + 8)');
@@ -41,10 +43,10 @@ public function testWeek(array $expectedVisible, string $rule): void
4143
// Loop to avoid rare edge case where first calculation
4244
// and second do not take place in same day.
4345
do {
44-
$sheet = $this->getSheet();
46+
$sheet = $this->getSpreadsheet()->createSheet();
4547
$dtStart = new DateTimeImmutable();
4648
$startDay = (int) $dtStart->format('d');
47-
$this->setCells();
49+
$this->setCells($sheet);
4850

4951
$maxRow = $this->maxRow;
5052
$autoFilter = $sheet->getAutoFilter();
@@ -63,6 +65,6 @@ public function testWeek(array $expectedVisible, string $rule): void
6365
$endDay = (int) $dtEnd->format('d');
6466
} while ($startDay !== $endDay);
6567

66-
self::assertEquals($expectedVisible, $this->getVisible());
68+
self::assertEquals($expectedVisible, $this->getVisibleSheet($sheet));
6769
}
6870
}

‎tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterYearTest.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public function testYears(array $expectedVisible, string $rule): void
2828
// Loop to avoid rare edge case where first calculation
2929
// and second do not take place in same day.
3030
do {
31-
$sheet = $this->getSheet();
31+
$sheet = $this->getSpreadsheet()->createSheet();
3232
$dtStart = new DateTimeImmutable();
3333
$startDay = (int) $dtStart->format('d');
3434
$sheet->getCell('A1')->setValue('Date');
@@ -63,20 +63,22 @@ public function testYears(array $expectedVisible, string $rule): void
6363
$endDay = (int) $dtEnd->format('d');
6464
} while ($startDay !== $endDay);
6565

66-
self::assertEquals($expectedVisible, $this->getVisible());
66+
self::assertEquals($expectedVisible, $this->getVisibleSheet($sheet));
6767
}
6868

6969
public function testYearToDate(): void
7070
{
7171
// Loop to avoid rare edge case where first calculation
7272
// and second do not take place in same day.
7373
do {
74-
$sheet = $this->getSheet();
74+
$sheet = $this->getSpreadsheet()->createSheet();
7575
$dtStart = new DateTimeImmutable();
7676
$startDay = (int) $dtStart->format('d');
7777
$startMonth = (int) $dtStart->format('m');
7878
$sheet->getCell('A1')->setValue('Date');
7979
$sheet->getCell('A2')->setValue('=TODAY()');
80+
// cache result for consistency in later calculations
81+
$sheet->getCell('A2')->getCalculatedValue();
8082
$sheet->getCell('A3')->setValue('=DATE(YEAR(A2), 12, 31)');
8183
$sheet->getCell('A4')->setValue('=A3 + 1');
8284
$sheet->getCell('A5')->setValue('=DATE(YEAR(A2), 1, 1)');
@@ -100,6 +102,6 @@ public function testYearToDate(): void
100102
} while ($startDay !== $endDay);
101103

102104
$expected = ($startMonth === 12 && $startDay === 31) ? [2, 3, 5] : [2, 5];
103-
self::assertEquals($expected, $this->getVisible());
105+
self::assertEquals($expected, $this->getVisibleSheet($sheet));
104106
}
105107
}

‎tests/PhpSpreadsheetTests/Worksheet/AutoFilter/SetupTeardown.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,11 @@ protected function getSheet(): Worksheet
5454

5555
public function getVisible(): array
5656
{
57-
$sheet = $this->getSheet();
57+
return $this->getVisibleSheet($this->getSheet());
58+
}
59+
60+
public function getVisibleSheet(Worksheet $sheet): array
61+
{
5862
$sheet->getAutoFilter()->showHideRows();
5963
$actualVisible = [];
6064
for ($row = 2; $row <= $this->maxRow; ++$row) {

0 commit comments

Comments
 (0)