Skip to content

Commit

Permalink
100% Coverage for Calculation/DateTime (#1870)
Browse files Browse the repository at this point in the history
* 100% Coverage for Calculation/DateTime

The code in DateTime is now completely covered.
Along the way, some errors were discovered and corrected.
- The tests which have had to be changed at the start of every year are
replaced by more robust equivalents which do not require annual changes.
- Several places in the code where Gnumeric and OpenOffice were thought to differ
from Excel do not appear to have had any justification.
I have left a comment where such code has been removed.
- Use DateTime when possible rather than date, time, or strftime functions to avoid
potential Y2038 problems.
- Some impossible code has been removed, replaced by an explanatory comment.
- NETWORKDAYS had a bug when the start date was Sunday. There had been no tests
of this condition.
- Some functions allow boolean and null arguments where a number is expected.
This is more complicated than the equivalent situations in MathTrig because
the initial date for these calculations can be Day 1 rather than Day 0.
- More testing for dates from 1900-01-01 through the fictitious
everywhere-but-Excel 1900-01-29.
    - This showed that there is an additional Excel bug - Excel evaluates
WEEKNUM(emptycell) as 0, which is not a valid result for
WEEKNUM without a second argument.
PhpSpreadsheet now duplicates this bug.
    - There is a similar and even worse bug for 1904-01-01 in 1904 calculations.
Weeknum returns 0 for this,
but returns the correct value for arguments of 0 or null.
    - DATEVALUE should accept 1900-02-29 (sigh) and relatives.
PhpSpreadsheet now duplicates this bug.
- Testing bootstrap sets default timezone. This appears to be a relic from
the releases of PHP where the unwise decision, subsequenly reversed,
was made to issue messages for
"no default timezone is set" rather than just use a sensible default.
This was a disruptive setting for some of the tests I added.
There is only one test in the entire suite which is default-timezone-dependent.
Setting and resetting of default timezone is moved to that test
(Reader/ODS/ODSTest), and out of bootstrap.
- There had been no testing of NOW() function.
- DATEVALUE test had no tests for 1904 calendar and needs some.
- DATE test changed 1900/1904 calendar in use without restoring it.
- WEEKDAY test had no tests for 1904 calendar and needs some.
    - Which revealed a bug in Shared/Date (excelToDateTimeObject was not
recognizing 1904-01-01 as valid when 1904 calendar is in use).
    - And an additional bug in that legal 1904-calendar values in the 0.0-1.0
range yielded the same "wrong" answers as 1900-calendar (see "One note" below).
Also the comment for one of the calendar-1904 tests was wrong in attempting
to identify what time of day the fraction represented.

I had wanted to break this up into a set of smaller modules, a process already
started for Engineering and MathTrig.
However the number of source code changes was sufficient that I wanted
a clean delta for this request.
If it is merged, I will work on breaking it up afterwards.

One note - Shared/Date/excelToDateTimeObject, when calendar-1900 is in use,
returns an unexpected result if its argument is between 0 and 1,
which is nominally invalid for that calendar.
It uses a base-1970 calendar in that instance. That check is not justifiable
for calendar-1904, where values in that range are legal,
so I made the check specific to calendar-1900,
and adjusted 3 1904 unit test results accordingly. However, I have to admit that
I don't understand why that check should be made even for calendar-1900.
It certainly doesn't match anything that Excel does.
I would recommend scrapping that code altogether.
If agreed, I would do this as part of the break-up into smaller modules.

Another note -
more controversially, it is clear that PhpSpreadsheet needs to support
the Excel and PHP date formats. Although it requires further study,
I am not convinced that it needs to support Unix timestamp format.
Since that is a potential source of Y2038 problems on 32-bit systems,
I would like to open a PR to deprecate the use of that format.
Please let me know if you are aware of a valid reason to continue to support it.
  • Loading branch information
oleibman authored Feb 27, 2021
1 parent 08673b5 commit 80a20fc
Show file tree
Hide file tree
Showing 19 changed files with 534 additions and 248 deletions.
446 changes: 236 additions & 210 deletions src/PhpSpreadsheet/Calculation/DateTime.php

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Shared/Date.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public static function excelToDateTimeObject($excelTimestamp, $timeZone = null)
{
$timeZone = ($timeZone === null) ? self::getDefaultTimezone() : self::validateTimeZone($timeZone);
if (Functions::getCompatibilityMode() == Functions::COMPATIBILITY_EXCEL) {
if ($excelTimestamp < 1.0) {
if ($excelTimestamp < 1 && self::$excelCalendar === self::CALENDAR_WINDOWS_1900) {
// Unix timestamp base date
$baseDate = new \DateTime('1970-01-01', $timeZone);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,21 @@

class DateTest extends TestCase
{
private $returnDateType;

private $excelCalendar;

protected function setUp(): void
{
Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
$this->returnDateType = Functions::getReturnDateType();
$this->excelCalendar = Date::getExcelCalendar();
Functions::setReturnDateType(Functions::RETURNDATE_EXCEL);
Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900);
}

protected function tearDown(): void
{
Functions::setReturnDateType($this->returnDateType);
Date::setExcelCalendar($this->excelCalendar);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\DateTime;

use DateTimeImmutable;
use DateTimeInterface;
use PhpOffice\PhpSpreadsheet\Calculation\DateTime;
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
Expand All @@ -10,11 +11,21 @@

class DateValueTest extends TestCase
{
private $returnDateType;

private $excelCalendar;

protected function setUp(): void
{
Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
$this->returnDateType = Functions::getReturnDateType();
$this->excelCalendar = Date::getExcelCalendar();
Functions::setReturnDateType(Functions::RETURNDATE_EXCEL);
Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900);
}

protected function tearDown(): void
{
Functions::setReturnDateType($this->returnDateType);
Date::setExcelCalendar($this->excelCalendar);
}

/**
Expand All @@ -25,7 +36,21 @@ protected function setUp(): void
*/
public function testDATEVALUE($expectedResult, $dateValue): void
{
$result = DateTime::DATEVALUE($dateValue);
// Loop to avoid extraordinarily rare edge case where first calculation
// and second do not take place on same day.
do {
$dtStart = new DateTimeImmutable();
$startDay = $dtStart->format('d');
if (is_string($expectedResult)) {
$replYMD = str_replace('Y', date('Y'), $expectedResult);
if ($replYMD !== $expectedResult) {
$expectedResult = DateTime::DATEVALUE($replYMD);
}
}
$result = DateTime::DATEVALUE($dateValue);
$dtEnd = new DateTimeImmutable();
$endDay = $dtEnd->format('d');
} while ($startDay !== $endDay);
self::assertEqualsWithDelta($expectedResult, $result, 1E-8);
}

Expand Down Expand Up @@ -55,4 +80,13 @@ public function testDATEVALUEtoDateTimeObject(): void
// ... with the correct value
self::assertEquals($result->format('d-M-Y'), '31-Jan-2012');
}

public function testDATEVALUEwith1904Calendar(): void
{
Date::setExcelCalendar(Date::CALENDAR_MAC_1904);
self::assertEquals(5428, DateTime::DATEVALUE('1918-11-11'));
self::assertEquals(0, DateTime::DATEVALUE('1904-01-01'));
self::assertEquals('#VALUE!', DateTime::DATEVALUE('1903-12-31'));
self::assertEquals('#VALUE!', DateTime::DATEVALUE('1900-02-29'));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\DateTime;

use DateTimeImmutable;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PHPUnit\Framework\TestCase;

class NowTest extends TestCase
{
public function testNow(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
// Loop to avoid rare edge case where first calculation
// and second do not take place in same second.
do {
$dtStart = new DateTimeImmutable();
$startSecond = $dtStart->format('s');
$sheet->setCellValue('A1', '=NOW()');
$dtEnd = new DateTimeImmutable();
$endSecond = $dtEnd->format('s');
} while ($startSecond !== $endSecond);
//echo("\n"); var_dump($sheet->getCell('A1')->getCalculatedValue()); echo ("\n");
$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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,20 @@

class TimeTest extends TestCase
{
private $returnDateType;

private $calendar;

protected function setUp(): void
{
Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
Functions::setReturnDateType(Functions::RETURNDATE_EXCEL);
Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900);
$this->returnDateType = Functions::getReturnDateType();
$this->calendar = Date::getExcelCalendar();
}

protected function tearDown(): void
{
Functions::setReturnDateType($this->returnDateType);
Date::setExcelCalendar($this->calendar);
}

/**
Expand All @@ -23,6 +32,7 @@ protected function setUp(): void
*/
public function testTIME($expectedResult, ...$args): void
{
Functions::setReturnDateType(Functions::RETURNDATE_EXCEL);
$result = DateTime::TIME(...$args);
self::assertEqualsWithDelta($expectedResult, $result, 1E-8);
}
Expand Down Expand Up @@ -52,4 +62,20 @@ public function testTIMEtoDateTimeObject(): void
// ... with the correct value
self::assertEquals($result->format('H:i:s'), '07:30:20');
}

public function testTIME1904(): void
{
Functions::setReturnDateType(Functions::RETURNDATE_EXCEL);
Date::setExcelCalendar(Date::CALENDAR_MAC_1904);
$result = DateTime::TIME(0, 0, 0);
self::assertEquals(0, $result);
}

public function testTIME1900(): void
{
Functions::setReturnDateType(Functions::RETURNDATE_EXCEL);
Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900);
$result = DateTime::TIME(0, 0, 0);
self::assertEquals(0, $result);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,21 @@
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\DateTime;

use PhpOffice\PhpSpreadsheet\Calculation\DateTime;
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
use PhpOffice\PhpSpreadsheet\Shared\Date;
use PHPUnit\Framework\TestCase;

class WeekDayTest extends TestCase
{
private $excelCalendar;

protected function setUp(): void
{
Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
Functions::setReturnDateType(Functions::RETURNDATE_EXCEL);
Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900);
$this->excelCalendar = Date::getExcelCalendar();
}

protected function tearDown(): void
{
Date::setExcelCalendar($this->excelCalendar);
}

/**
Expand All @@ -31,4 +35,12 @@ public function providerWEEKDAY()
{
return require 'tests/data/Calculation/DateTime/WEEKDAY.php';
}

public function testWEEKDAYwith1904Calendar(): void
{
Date::setExcelCalendar(Date::CALENDAR_MAC_1904);
self::assertEquals(7, DateTime::WEEKDAY('1904-01-02'));
self::assertEquals(6, DateTime::WEEKDAY('1904-01-01'));
self::assertEquals(6, DateTime::WEEKDAY(null));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,21 @@
namespace PhpOffice\PhpSpreadsheetTests\Calculation\Functions\DateTime;

use PhpOffice\PhpSpreadsheet\Calculation\DateTime;
use PhpOffice\PhpSpreadsheet\Calculation\Functions;
use PhpOffice\PhpSpreadsheet\Shared\Date;
use PHPUnit\Framework\TestCase;

class WeekNumTest extends TestCase
{
private $excelCalendar;

protected function setUp(): void
{
Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL);
Functions::setReturnDateType(Functions::RETURNDATE_EXCEL);
Date::setExcelCalendar(Date::CALENDAR_WINDOWS_1900);
$this->excelCalendar = Date::getExcelCalendar();
}

protected function tearDown(): void
{
Date::setExcelCalendar($this->excelCalendar);
}

/**
Expand All @@ -31,4 +35,14 @@ public function providerWEEKNUM()
{
return require 'tests/data/Calculation/DateTime/WEEKNUM.php';
}

public function testWEEKNUMwith1904Calendar(): void
{
Date::setExcelCalendar(Date::CALENDAR_MAC_1904);
self::assertEquals(27, DateTime::WEEKNUM('2004-07-02'));
self::assertEquals(1, DateTime::WEEKNUM('1904-01-02'));
self::assertEquals(1, DateTime::WEEKNUM(null));
// The following is a bug in Excel.
self::assertEquals(0, DateTime::WEEKNUM('1904-01-01'));
}
}
19 changes: 16 additions & 3 deletions tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,19 @@
*/
class OdsTest extends TestCase
{
private $timeZone;

protected function setUp(): void
{
$this->timeZone = date_default_timezone_get();
date_default_timezone_set('UTC');
}

protected function tearDown(): void
{
date_default_timezone_set($this->timeZone);
}

/**
* @var Spreadsheet
*/
Expand Down Expand Up @@ -153,13 +166,13 @@ public function testReadValueAndComments(): void
self::assertEquals(0, $firstSheet->getCell('G10')->getValue());

self::assertEquals(DataType::TYPE_NUMERIC, $firstSheet->getCell('A10')->getDataType()); // Date
self::assertEquals(22269.0, $firstSheet->getCell('A10')->getValue());
self::assertEquals('19-Dec-60', $firstSheet->getCell('A10')->getFormattedValue());

self::assertEquals(DataType::TYPE_NUMERIC, $firstSheet->getCell('A13')->getDataType()); // Time
self::assertEquals(25569.0625, $firstSheet->getCell('A13')->getValue());
self::assertEquals('2:30:00', $firstSheet->getCell('A13')->getFormattedValue());

self::assertEquals(DataType::TYPE_NUMERIC, $firstSheet->getCell('A15')->getDataType()); // Date + Time
self::assertEquals(22269.0625, $firstSheet->getCell('A15')->getValue());
self::assertEquals('19-Dec-60 1:30:00', $firstSheet->getCell('A15')->getFormattedValue());

self::assertEquals(DataType::TYPE_NUMERIC, $firstSheet->getCell('A11')->getDataType()); // Fraction

Expand Down
2 changes: 1 addition & 1 deletion tests/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
setlocale(LC_ALL, 'en_US.utf8');

// PHP 5.3 Compat
date_default_timezone_set('Europe/London');
//date_default_timezone_set('Europe/London');
Loading

0 comments on commit 80a20fc

Please sign in to comment.