From cb8ee73355b78db0ff23d54ebf9618e0b57c7e8b Mon Sep 17 00:00:00 2001 From: oleibman Date: Sun, 3 Oct 2021 11:06:47 -0700 Subject: [PATCH] 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. --- .../Functions/DateTime/NowTest.php | 30 +++++++++++-------- .../Functions/DateTime/TodayTest.php | 30 +++++++++++-------- .../AutoFilter/AutoFilterMonthTest.php | 6 ++-- .../AutoFilter/AutoFilterQuarterTest.php | 12 ++++---- .../AutoFilter/AutoFilterTodayTest.php | 8 +++-- .../AutoFilter/AutoFilterWeekTest.php | 12 ++++---- .../AutoFilter/AutoFilterYearTest.php | 10 ++++--- .../Worksheet/AutoFilter/SetupTeardown.php | 6 +++- 8 files changed, 69 insertions(+), 45 deletions(-) diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/NowTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/NowTest.php index 42362b99..05e9d411 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/NowTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/NowTest.php @@ -9,26 +9,30 @@ class NowTest extends AllSetupTeardown public function testNow(): void { $sheet = $this->getSheet(); + $row = 0; // Loop to avoid rare edge case where first calculation // and second do not take place in same second. do { + ++$row; $dtStart = new DateTimeImmutable(); $startSecond = $dtStart->format('s'); - $sheet->setCellValue('A1', '=NOW()'); + $sheet->setCellValue("A$row", '=NOW()'); + // cache result for later assertions + $sheet->getCell("A$row")->getCalculatedValue(); $dtEnd = new DateTimeImmutable(); $endSecond = $dtEnd->format('s'); } while ($startSecond !== $endSecond); - $sheet->setCellValue('B1', '=YEAR(A1)'); - $sheet->setCellValue('C1', '=MONTH(A1)'); - $sheet->setCellValue('D1', '=DAY(A1)'); - $sheet->setCellValue('E1', '=HOUR(A1)'); - $sheet->setCellValue('F1', '=MINUTE(A1)'); - $sheet->setCellValue('G1', '=SECOND(A1)'); - self::assertEquals($dtStart->format('Y'), $sheet->getCell('B1')->getCalculatedValue()); - self::assertEquals($dtStart->format('m'), $sheet->getCell('C1')->getCalculatedValue()); - self::assertEquals($dtStart->format('d'), $sheet->getCell('D1')->getCalculatedValue()); - self::assertEquals($dtStart->format('H'), $sheet->getCell('E1')->getCalculatedValue()); - self::assertEquals($dtStart->format('i'), $sheet->getCell('F1')->getCalculatedValue()); - self::assertEquals($dtStart->format('s'), $sheet->getCell('G1')->getCalculatedValue()); + $sheet->setCellValue("B$row", "=YEAR(A$row)"); + $sheet->setCellValue("C$row", "=MONTH(A$row)"); + $sheet->setCellValue("D$row", "=DAY(A$row)"); + $sheet->setCellValue("E$row", "=HOUR(A$row)"); + $sheet->setCellValue("F$row", "=MINUTE(A$row)"); + $sheet->setCellValue("G$row", "=SECOND(A$row)"); + self::assertEquals($dtStart->format('Y'), $sheet->getCell("B$row")->getCalculatedValue()); + self::assertEquals($dtStart->format('m'), $sheet->getCell("C$row")->getCalculatedValue()); + self::assertEquals($dtStart->format('d'), $sheet->getCell("D$row")->getCalculatedValue()); + self::assertEquals($dtStart->format('H'), $sheet->getCell("E$row")->getCalculatedValue()); + self::assertEquals($dtStart->format('i'), $sheet->getCell("F$row")->getCalculatedValue()); + self::assertEquals($dtStart->format('s'), $sheet->getCell("G$row")->getCalculatedValue()); } } diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/TodayTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/TodayTest.php index b7108f75..518623b5 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/TodayTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/DateTime/TodayTest.php @@ -9,26 +9,30 @@ class TodayTest extends AllSetupTeardown public function testToday(): void { $sheet = $this->getSheet(); + $row = 0; // Loop to avoid rare edge case where first calculation // and second do not take place in same second. do { + ++$row; $dtStart = new DateTimeImmutable(); $startSecond = $dtStart->format('s'); - $sheet->setCellValue('A1', '=TODAY()'); + $sheet->setCellValue("A$row", '=TODAY()'); + // cache result for later assertions + $sheet->getCell("A$row")->getCalculatedValue(); $dtEnd = new DateTimeImmutable(); $endSecond = $dtEnd->format('s'); } while ($startSecond !== $endSecond); - $sheet->setCellValue('B1', '=YEAR(A1)'); - $sheet->setCellValue('C1', '=MONTH(A1)'); - $sheet->setCellValue('D1', '=DAY(A1)'); - $sheet->setCellValue('E1', '=HOUR(A1)'); - $sheet->setCellValue('F1', '=MINUTE(A1)'); - $sheet->setCellValue('G1', '=SECOND(A1)'); - self::assertSame((int) $dtStart->format('Y'), $sheet->getCell('B1')->getCalculatedValue()); - self::assertSame((int) $dtStart->format('m'), $sheet->getCell('C1')->getCalculatedValue()); - self::assertSame((int) $dtStart->format('d'), $sheet->getCell('D1')->getCalculatedValue()); - self::assertSame(0, $sheet->getCell('E1')->getCalculatedValue()); - self::assertSame(0, $sheet->getCell('F1')->getCalculatedValue()); - self::assertSame(0, $sheet->getCell('G1')->getCalculatedValue()); + $sheet->setCellValue("B$row", "=YEAR(A$row)"); + $sheet->setCellValue("C$row", "=MONTH(A$row)"); + $sheet->setCellValue("D$row", "=DAY(A$row)"); + $sheet->setCellValue("E$row", "=HOUR(A$row)"); + $sheet->setCellValue("F$row", "=MINUTE(A$row)"); + $sheet->setCellValue("G$row", "=SECOND(A$row)"); + self::assertSame((int) $dtStart->format('Y'), $sheet->getCell("B$row")->getCalculatedValue()); + self::assertSame((int) $dtStart->format('m'), $sheet->getCell("C$row")->getCalculatedValue()); + self::assertSame((int) $dtStart->format('d'), $sheet->getCell("D$row")->getCalculatedValue()); + self::assertSame(0, $sheet->getCell("E$row")->getCalculatedValue()); + self::assertSame(0, $sheet->getCell("F$row")->getCalculatedValue()); + self::assertSame(0, $sheet->getCell("G$row")->getCalculatedValue()); } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterMonthTest.php b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterMonthTest.php index c13e5a24..e17ccce9 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterMonthTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterMonthTest.php @@ -22,6 +22,8 @@ class AutoFilterMonthTest extends SetupTeardown { $sheet->getCell('A1')->setValue('Date'); $sheet->getCell('A2')->setValue('=TODAY()'); + // cache result for consistency in later calculations + $sheet->getCell('A2')->getCalculatedValue(); $sheet->getCell('A3')->setValue('=DATE(YEAR(A2), MONTH(A2), 1)'); if ($startMonth === 12) { $sheet->getCell('A4')->setValue('=DATE(YEAR(A2) + 1, 1, 1)'); @@ -56,7 +58,7 @@ class AutoFilterMonthTest extends SetupTeardown // Loop to avoid rare edge case where first calculation // and second do not take place in same day. do { - $sheet = $this->getSheet(); + $sheet = $this->getSpreadsheet()->createSheet(); $dtStart = new DateTimeImmutable(); $startDay = (int) $dtStart->format('d'); $startMonth = (int) $dtStart->format('m'); @@ -79,6 +81,6 @@ class AutoFilterMonthTest extends SetupTeardown $endDay = (int) $dtEnd->format('d'); } while ($startDay !== $endDay); - self::assertEquals($expectedVisible, $this->getVisible()); + self::assertEquals($expectedVisible, $this->getVisibleSheet($sheet)); } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterQuarterTest.php b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterQuarterTest.php index 859f7fdd..370158a0 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterQuarterTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterQuarterTest.php @@ -5,6 +5,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Worksheet\AutoFilter; use DateTimeImmutable; use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column; use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column\Rule; +use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; class AutoFilterQuarterTest extends SetupTeardown { @@ -17,11 +18,12 @@ class AutoFilterQuarterTest extends SetupTeardown ]; } - private function setCells(): void + private function setCells(Worksheet $sheet): void { - $sheet = $this->getSheet(); $sheet->getCell('A1')->setValue('Date'); $sheet->getCell('A2')->setValue('=TODAY()'); + // cache result for consistency in later calculations + $sheet->getCell('A2')->getCalculatedValue(); $sheet->getCell('A3')->setValue('=DATE(YEAR(A2), MONTH(A2), 1)'); $sheet->getCell('A4')->setValue('=DATE(YEAR(A2), MONTH(A2) + 3, 1)'); $sheet->getCell('A5')->setValue('=DATE(YEAR(A2), MONTH(A2) + 6, 1)'); @@ -40,10 +42,10 @@ class AutoFilterQuarterTest extends SetupTeardown // Loop to avoid rare edge case where first calculation // and second do not take place in same day. do { - $sheet = $this->getSheet(); + $sheet = $this->getSpreadsheet()->createSheet(); $dtStart = new DateTimeImmutable(); $startDay = (int) $dtStart->format('d'); - $this->setCells(); + $this->setCells($sheet); $maxRow = $this->maxRow; $autoFilter = $sheet->getAutoFilter(); @@ -62,6 +64,6 @@ class AutoFilterQuarterTest extends SetupTeardown $endDay = (int) $dtEnd->format('d'); } while ($startDay !== $endDay); - self::assertEquals($expectedVisible, $this->getVisible()); + self::assertEquals($expectedVisible, $this->getVisibleSheet($sheet)); } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterTodayTest.php b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterTodayTest.php index 0a8aa723..6805ec77 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterTodayTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterTodayTest.php @@ -25,14 +25,18 @@ class AutoFilterTodayTest extends SetupTeardown // Loop to avoid rare edge case where first calculation // and second do not take place in same day. do { - $sheet = $this->getSheet(); + $sheet = $this->getSpreadsheet()->createSheet(); $dtStart = new DateTimeImmutable(); $startDay = $dtStart->format('d'); $sheet->getCell('A1')->setValue('Date'); $sheet->getCell('A2')->setValue('=NOW()'); + // cache result for consistency in later calculations + $sheet->getCell('A2')->getCalculatedValue(); $sheet->getCell('A3')->setValue('=A2+1'); $sheet->getCell('A4')->setValue('=A2-1'); $sheet->getCell('A5')->setValue('=TODAY()'); + // cache result for consistency in later calculations + $sheet->getCell('A5')->getCalculatedValue(); $sheet->getCell('A6')->setValue('=A5+1'); $sheet->getCell('A7')->setValue('=A5-1'); $this->maxRow = $maxRow = 7; @@ -52,6 +56,6 @@ class AutoFilterTodayTest extends SetupTeardown $endDay = $dtEnd->format('d'); } while ($startDay !== $endDay); - self::assertEquals($expectedVisible, $this->getVisible()); + self::assertEquals($expectedVisible, $this->getVisibleSheet($sheet)); } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterWeekTest.php b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterWeekTest.php index 6784d226..ce506e54 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterWeekTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterWeekTest.php @@ -5,6 +5,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Worksheet\AutoFilter; use DateTimeImmutable; use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column; use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter\Column\Rule; +use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; class AutoFilterWeekTest extends SetupTeardown { @@ -17,11 +18,12 @@ class AutoFilterWeekTest extends SetupTeardown ]; } - private function setCells(): void + private function setCells(Worksheet $sheet): void { - $sheet = $this->getSheet(); $sheet->getCell('A1')->setValue('Date'); $sheet->getCell('A2')->setValue('=TODAY()'); + // cache result for consistency in later calculations + $sheet->getCell('A2')->getCalculatedValue(); $sheet->getCell('B2')->setValue('=WEEKDAY(A2) - 1'); // subtract to get to Sunday $sheet->getCell('A3')->setValue('=DATE(YEAR(A2), MONTH(A2), DAY(A2) - B2)'); $sheet->getCell('A4')->setValue('=DATE(YEAR(A3), MONTH(A3), DAY(A3) + 8)'); @@ -41,10 +43,10 @@ class AutoFilterWeekTest extends SetupTeardown // Loop to avoid rare edge case where first calculation // and second do not take place in same day. do { - $sheet = $this->getSheet(); + $sheet = $this->getSpreadsheet()->createSheet(); $dtStart = new DateTimeImmutable(); $startDay = (int) $dtStart->format('d'); - $this->setCells(); + $this->setCells($sheet); $maxRow = $this->maxRow; $autoFilter = $sheet->getAutoFilter(); @@ -63,6 +65,6 @@ class AutoFilterWeekTest extends SetupTeardown $endDay = (int) $dtEnd->format('d'); } while ($startDay !== $endDay); - self::assertEquals($expectedVisible, $this->getVisible()); + self::assertEquals($expectedVisible, $this->getVisibleSheet($sheet)); } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterYearTest.php b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterYearTest.php index 442f846e..8f463671 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterYearTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterYearTest.php @@ -28,7 +28,7 @@ class AutoFilterYearTest extends SetupTeardown // Loop to avoid rare edge case where first calculation // and second do not take place in same day. do { - $sheet = $this->getSheet(); + $sheet = $this->getSpreadsheet()->createSheet(); $dtStart = new DateTimeImmutable(); $startDay = (int) $dtStart->format('d'); $sheet->getCell('A1')->setValue('Date'); @@ -63,7 +63,7 @@ class AutoFilterYearTest extends SetupTeardown $endDay = (int) $dtEnd->format('d'); } while ($startDay !== $endDay); - self::assertEquals($expectedVisible, $this->getVisible()); + self::assertEquals($expectedVisible, $this->getVisibleSheet($sheet)); } public function testYearToDate(): void @@ -71,12 +71,14 @@ class AutoFilterYearTest extends SetupTeardown // Loop to avoid rare edge case where first calculation // and second do not take place in same day. do { - $sheet = $this->getSheet(); + $sheet = $this->getSpreadsheet()->createSheet(); $dtStart = new DateTimeImmutable(); $startDay = (int) $dtStart->format('d'); $startMonth = (int) $dtStart->format('m'); $sheet->getCell('A1')->setValue('Date'); $sheet->getCell('A2')->setValue('=TODAY()'); + // cache result for consistency in later calculations + $sheet->getCell('A2')->getCalculatedValue(); $sheet->getCell('A3')->setValue('=DATE(YEAR(A2), 12, 31)'); $sheet->getCell('A4')->setValue('=A3 + 1'); $sheet->getCell('A5')->setValue('=DATE(YEAR(A2), 1, 1)'); @@ -100,6 +102,6 @@ class AutoFilterYearTest extends SetupTeardown } while ($startDay !== $endDay); $expected = ($startMonth === 12 && $startDay === 31) ? [2, 3, 5] : [2, 5]; - self::assertEquals($expected, $this->getVisible()); + self::assertEquals($expected, $this->getVisibleSheet($sheet)); } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/SetupTeardown.php b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/SetupTeardown.php index 95d78012..3546bc32 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/SetupTeardown.php +++ b/tests/PhpSpreadsheetTests/Worksheet/AutoFilter/SetupTeardown.php @@ -54,7 +54,11 @@ class SetupTeardown extends TestCase public function getVisible(): array { - $sheet = $this->getSheet(); + return $this->getVisibleSheet($this->getSheet()); + } + + public function getVisibleSheet(Worksheet $sheet): array + { $sheet->getAutoFilter()->showHideRows(); $actualVisible = []; for ($row = 2; $row <= $this->maxRow; ++$row) {