From 905a697639d904d08e9ee5ffe3402ae63bbb8d2d Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Sat, 27 Jul 2019 16:02:58 +0200 Subject: [PATCH] More work on refactoring Excel Calculation Function Unit Tests --- CHANGELOG.md | 1 + src/PhpSpreadsheet/Calculation/MathTrig.php | 7 ++- src/PhpSpreadsheet/Calculation/TextData.php | 33 ++++++------- .../Calculation/FinancialTest.php | 32 ------------- .../Functions/Financial/AccrintMTest.php | 31 ++++++++++++ .../Functions/Financial/AccrintTest.php | 31 ++++++++++++ .../Functions/LookupRef/HLookupTest.php | 31 ++++++++++++ .../Functions/LookupRef/LookupTest.php | 31 ++++++++++++ .../Functions/LookupRef/VLookupTest.php | 31 ++++++++++++ .../Calculation/LookupRefTest.php | 48 ------------------- 10 files changed, 174 insertions(+), 102 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Calculation/Functions/Financial/AccrintMTest.php create mode 100644 tests/PhpSpreadsheetTests/Calculation/Functions/Financial/AccrintTest.php create mode 100644 tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/HLookupTest.php create mode 100644 tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/LookupTest.php create mode 100644 tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 20819876..26b1d35b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Added +- Implementation of IFNA() Logical Function - When <br> appears in a table cell, set the cell to wrap [Issue #1071](https://github.com/PHPOffice/PhpSpreadsheet/issues/1071) and [PR #1070](https://github.com/PHPOffice/PhpSpreadsheet/pull/1070) - Add MAXIFS, MINIFS, COUNTIFS and Remove MINIF, MAXIF - [Issue #1056](https://github.com/PHPOffice/PhpSpreadsheet/issues/1056) - HLookup needs an ordered list even if range_lookup is set to false [Issue #1055](https://github.com/PHPOffice/PhpSpreadsheet/issues/1055) and [PR #1076](https://github.com/PHPOffice/PhpSpreadsheet/pull/1076) diff --git a/src/PhpSpreadsheet/Calculation/MathTrig.php b/src/PhpSpreadsheet/Calculation/MathTrig.php index 4e384ea3..30071c2d 100644 --- a/src/PhpSpreadsheet/Calculation/MathTrig.php +++ b/src/PhpSpreadsheet/Calculation/MathTrig.php @@ -220,10 +220,9 @@ class MathTrig return Functions::NAN(); } $factLoop = floor($factVal); - if (Functions::getCompatibilityMode() == Functions::COMPATIBILITY_GNUMERIC) { - if ($factVal > $factLoop) { - return Functions::NAN(); - } + if ((Functions::getCompatibilityMode() == Functions::COMPATIBILITY_GNUMERIC) && + ($factVal > $factLoop)) { + return Functions::NAN(); } $factorial = 1; diff --git a/src/PhpSpreadsheet/Calculation/TextData.php b/src/PhpSpreadsheet/Calculation/TextData.php index e30b2ff7..d03c4983 100644 --- a/src/PhpSpreadsheet/Calculation/TextData.php +++ b/src/PhpSpreadsheet/Calculation/TextData.php @@ -52,7 +52,7 @@ class TextData return ($stringValue) ? Calculation::getTRUE() : Calculation::getFALSE(); } - if (self::$invalidChars == null) { + if (self::$invalidChars === null) { self::$invalidChars = range(chr(0), chr(31)); } @@ -84,6 +84,15 @@ class TextData return null; } + private static function convertBooleanValue($value) + { + if (Functions::getCompatibilityMode() == Functions::COMPATIBILITY_OPENOFFICE) { + return (int)$value; + } + + return ($value) ? Calculation::getTRUE() : Calculation::getFALSE(); + } + /** * ASCIICODE. * @@ -98,11 +107,7 @@ class TextData } $characters = Functions::flattenSingleValue($characters); if (is_bool($characters)) { - if (Functions::getCompatibilityMode() == Functions::COMPATIBILITY_OPENOFFICE) { - $characters = (int) $characters; - } else { - $characters = ($characters) ? Calculation::getTRUE() : Calculation::getFALSE(); - } + $characters = self::convertBooleanValue($characters); } $character = $characters; @@ -126,11 +131,7 @@ class TextData $aArgs = Functions::flattenArray($args); foreach ($aArgs as $arg) { if (is_bool($arg)) { - if (Functions::getCompatibilityMode() == Functions::COMPATIBILITY_OPENOFFICE) { - $arg = (int) $arg; - } else { - $arg = ($arg) ? Calculation::getTRUE() : Calculation::getFALSE(); - } + $arg = self::convertBooleanValue($arg); } $returnValue .= $arg; } @@ -197,7 +198,7 @@ class TextData } if (($offset > 0) && (StringHelper::countCharacters($haystack) > $offset)) { - if (StringHelper::countCharacters($needle) == 0) { + if (StringHelper::countCharacters($needle) === 0) { return $offset; } @@ -232,7 +233,7 @@ class TextData } if (($offset > 0) && (StringHelper::countCharacters($haystack) > $offset)) { - if (StringHelper::countCharacters($needle) == 0) { + if (StringHelper::countCharacters($needle) === 0) { return $offset; } @@ -659,11 +660,7 @@ class TextData if ($ignoreEmpty && trim($arg) == '') { unset($aArgs[$key]); } elseif (is_bool($arg)) { - if (Functions::getCompatibilityMode() == Functions::COMPATIBILITY_OPENOFFICE) { - $arg = (int) $arg; - } else { - $arg = ($arg) ? Calculation::getTRUE() : Calculation::getFALSE(); - } + $arg = self::convertBooleanValue($arg); } } diff --git a/tests/PhpSpreadsheetTests/Calculation/FinancialTest.php b/tests/PhpSpreadsheetTests/Calculation/FinancialTest.php index 2d8d770a..d98d91c5 100644 --- a/tests/PhpSpreadsheetTests/Calculation/FinancialTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/FinancialTest.php @@ -13,38 +13,6 @@ class FinancialTest extends TestCase Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL); } - /** - * @dataProvider providerACCRINT - * - * @param mixed $expectedResult - */ - public function testACCRINT($expectedResult, ...$args) - { - $result = Financial::ACCRINT(...$args); - self::assertEquals($expectedResult, $result, '', 1E-8); - } - - public function providerACCRINT() - { - return require 'data/Calculation/Financial/ACCRINT.php'; - } - - /** - * @dataProvider providerACCRINTM - * - * @param mixed $expectedResult - */ - public function testACCRINTM($expectedResult, ...$args) - { - $result = Financial::ACCRINTM(...$args); - self::assertEquals($expectedResult, $result, '', 1E-8); - } - - public function providerACCRINTM() - { - return require 'data/Calculation/Financial/ACCRINTM.php'; - } - /** * @dataProvider providerAMORDEGRC * diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/Financial/AccrintMTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/Financial/AccrintMTest.php new file mode 100644 index 00000000..a89d74f1 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/Financial/AccrintMTest.php @@ -0,0 +1,31 @@ +assertEquals($expectedResult, $result, '', 1E-8); + } + + public function providerACCRINT() + { + return require 'data/Calculation/Financial/ACCRINT.php'; + } +} diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/HLookupTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/HLookupTest.php new file mode 100644 index 00000000..41a79621 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/HLookupTest.php @@ -0,0 +1,31 @@ +assertEquals($expectedResult, $result); + } + + public function providerHLOOKUP() + { + return require 'data/Calculation/LookupRef/HLOOKUP.php'; + } +} diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/LookupTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/LookupTest.php new file mode 100644 index 00000000..815d5b73 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/LookupTest.php @@ -0,0 +1,31 @@ +assertEquals($expectedResult, $result); + } + + public function providerLOOKUP() + { + return require 'data/Calculation/LookupRef/LOOKUP.php'; + } +} diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php new file mode 100644 index 00000000..1de96dff --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/VLookupTest.php @@ -0,0 +1,31 @@ +assertEquals($expectedResult, $result); + } + + public function providerVLOOKUP() + { + return require 'data/Calculation/LookupRef/VLOOKUP.php'; + } +} diff --git a/tests/PhpSpreadsheetTests/Calculation/LookupRefTest.php b/tests/PhpSpreadsheetTests/Calculation/LookupRefTest.php index f7f035e7..2d9b5e7b 100644 --- a/tests/PhpSpreadsheetTests/Calculation/LookupRefTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/LookupRefTest.php @@ -19,54 +19,6 @@ class LookupRefTest extends TestCase Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL); } - /** - * @dataProvider providerHLOOKUP - * - * @param mixed $expectedResult - */ - public function testHLOOKUP($expectedResult, ...$args) - { - $result = LookupRef::HLOOKUP(...$args); - self::assertEquals($expectedResult, $result); - } - - public function providerHLOOKUP() - { - return require 'data/Calculation/LookupRef/HLOOKUP.php'; - } - - /** - * @dataProvider providerVLOOKUP - * - * @param mixed $expectedResult - */ - public function testVLOOKUP($expectedResult, ...$args) - { - $result = LookupRef::VLOOKUP(...$args); - self::assertEquals($expectedResult, $result); - } - - public function providerVLOOKUP() - { - return require 'data/Calculation/LookupRef/VLOOKUP.php'; - } - - /** - * @dataProvider providerLOOKUP - * - * @param mixed $expectedResult - */ - public function testLOOKUP($expectedResult, ...$args) - { - $result = LookupRef::LOOKUP(...$args); - self::assertEquals($expectedResult, $result); - } - - public function providerLOOKUP() - { - return require 'data/Calculation/LookupRef/LOOKUP.php'; - } - /** * @dataProvider providerMATCH *