Skip to content

Commit

Permalink
PhpUnit 10 Compatibility Part 1 (PHPOffice#3523)
Browse files Browse the repository at this point in the history
* PhpUnit 10 Compatibility Part 1

This is not a change to move to PhpUnit 10. There is no compelling reason to do so at this time, although it is bound to happen eventually. There are a staggering number of problems (somewhere around 3,000) with the current test suite under PhpUnit 10; this is an attempt to get ahead of the curve by addressing them now.

Method `setOutputCallback` has gone away. This affects only Helper/SampleTest. It appears that `ob_start` and its allies provide an effective equivalent. FWIW, the absence of `setOutputCallback` is a good indication of whether or not PhpUnit 10 is in use, and I will use that fact in a few tests.

Class `ComplexAssert` with no constructor, and always used with `new ComplexAssert()`, extends `TestCase`. Apparently, the constructor for TestCase requires an argument, and PhpUnit 10 complains about not supplying one. Adding an empty constructor to ComplexAssert avoids this problem.

There are two very minor source changes, to Calculation/Calculation and Reader/Xlsx, where problems were exposed with PhpUnit 10 that had not been previously been exposed. AFAIK, these are the only source changes required; the rest of the changes are to test members.

The bulk of the problems are because PhpUnit 10 insists that provider methods be static. Most of those can be changed by a script without any further action; those changes will constitute the Part 2 counterpart of this PR. In this PR you will find the exceptional cases that can't be automated for one reason or another. The tests for Database functions have mild complications that are easily handled. Most of the other provider changes in this PR are because the method names didn't follow an established pattern ('provider' isn't part of the method name); those are also easily handled manually. Modifying the following tests provided significant challenges:
- Writer/Xls/WorkbookTest testAddColor
- Worksheet/Table/TableTest testSetRangeValidRange

The handling of warning messages issued by the code differs in PhpUnit 10. According to the change log, "This means that using PHP functionality which triggers E_DEPRECATED, E_NOTICE, E_STRICT, or E_WARNING or calling code which triggers E_USER_DEPRECATED, E_USER_NOTICE, or E_USER_WARNING can no longer hide a bug in your code." To me, the effect of that change seems to be exactly the opposite - such messages were available to the test with PhpUnit 9 (so we could test for them), and are no longer available (so we can't). I haven't even succeeded with a custom error message handler as part of the script. I will continue to investigate, but, for now, will skip some tests under PhpUnit 10 for the following:
- Shared/OleTest testChainedWriteMode and testChainedBadPath
- Reader/Html/HtmlLoadStringTest testLoadInvalidString
- Reader/Html/HtmlTest testBadHtml

* Scrutinize, and Parent Construct

Parent construct suggested by @MarkBaker.

* Redo Tests Dependent on Warning Messages

Warning (and other) messages are handled differently in PhpUnit 10 than in earlier versions.
  • Loading branch information
oleibman authored Apr 19, 2023
1 parent 784eb6c commit 1187825
Show file tree
Hide file tree
Showing 33 changed files with 420 additions and 206 deletions.
2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -4250,7 +4250,7 @@ private function internalParseFormula($formula, ?Cell $cell = null)
} elseif ($expectedArgumentCount != '*') {
$isOperandOrFunction = preg_match('/(\d*)([-+,])(\d*)/', $expectedArgumentCount, $argMatch);
self::doNothing($isOperandOrFunction);
switch ($argMatch[2]) {
switch ($argMatch[2] ?? '') {
case '+':
if ($argumentCount < $argMatch[1]) {
$argumentCountError = true;
Expand Down
2 changes: 1 addition & 1 deletion src/PhpSpreadsheet/Reader/Xlsx.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ private function loadZip(string $filename, string $ns = '', bool $replaceUnclose
if ($replaceUnclosedBr) {
$contents = str_replace('<br>', '<br/>', $contents);
}
$rels = simplexml_load_string(
$rels = @simplexml_load_string(
$this->getSecurityScannerOrThrow()->scan($contents),
'SimpleXMLElement',
Settings::getLibXmlLoaderOptions(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public function testNamedFormulaeCalculationsWithAdjustedRateValue(string $cellA
self::assertSame($expectedValue, $calculatedCellValue, "Failed calculation for cell {$cellAddress}");
}

public function namedRangeCalculationTest1(): array
public static function namedRangeCalculationTest1(): array
{
return [
['C4', 56.25],
Expand All @@ -84,7 +84,7 @@ public function namedRangeCalculationTest1(): array
];
}

public function namedRangeCalculationTest2(): array
public static function namedRangeCalculationTest2(): array
{
return [
['C4', 93.75],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ public function testDAverageAsWorksheetFormula($expectedResult, array $database,
self::assertEqualsWithDelta($expectedResult, $result, 1.0e-12);
}

public function providerDAverage(): array
public static function providerDAverage(): array
{
return [
[
12,
$this->database1(),
self::database1(),
'Yield',
[
['Tree', 'Height'],
Expand All @@ -49,7 +49,7 @@ public function providerDAverage(): array
],
[
268333.333333333333,
$this->database2(),
self::database2(),
'Sales',
[
['Quarter', 'Sales Rep.'],
Expand All @@ -58,7 +58,7 @@ public function providerDAverage(): array
],
[
372500,
$this->database2(),
self::database2(),
'Sales',
[
['Quarter', 'Area'],
Expand All @@ -67,25 +67,25 @@ public function providerDAverage(): array
],
'numeric column, in this case referring to age' => [
13,
$this->database1(),
self::database1(),
3,
$this->database1(),
self::database1(),
],
'null field' => [
ExcelError::VALUE(),
$this->database1(),
self::database1(),
null,
$this->database1(),
self::database1(),
],
'field unknown column' => [
ExcelError::VALUE(),
$this->database1(),
self::database1(),
'xyz',
$this->database1(),
self::database1(),
],
'multiple criteria, omit equal sign' => [
10.5,
$this->database1(),
self::database1(),
'Yield',
[
['Tree', 'Height'],
Expand All @@ -95,7 +95,7 @@ public function providerDAverage(): array
],
'multiple criteria for same field' => [
10,
$this->database1(),
self::database1(),
'Yield',
[
['Tree', 'Height', 'Age', 'Height'],
Expand All @@ -108,15 +108,15 @@ public function providerDAverage(): array
content to return #VALUE! as an invalid name would */
'field column number too high' => [
ExcelError::VALUE(),
$this->database1(),
self::database1(),
99,
$this->database1(),
self::database1(),
],
'field column number too low' => [
ExcelError::VALUE(),
$this->database1(),
self::database1(),
0,
$this->database1(),
self::database1(),
],
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ public function testDCountAAsWorksheetFormula($expectedResult, $database, $field
self::assertEqualsWithDelta($expectedResult, $result, 1.0e-12);
}

public function providerDCountA(): array
public static function providerDCountA(): array
{
return [
[
1,
$this->database1(),
self::database1(),
'Profit',
[
['Tree', 'Height', 'Height'],
Expand All @@ -51,7 +51,7 @@ public function providerDCountA(): array
],
[
2,
$this->database3(),
self::database3(),
'Score',
[
['Subject', 'Gender'],
Expand All @@ -60,7 +60,7 @@ public function providerDCountA(): array
],
[
1,
$this->database3(),
self::database3(),
'Score',
[
['Subject', 'Gender'],
Expand All @@ -69,7 +69,7 @@ public function providerDCountA(): array
],
[
3,
$this->database3(),
self::database3(),
'Score',
[
['Subject', 'Score'],
Expand All @@ -78,7 +78,7 @@ public function providerDCountA(): array
],
'invalid field name' => [
ExcelError::VALUE(),
$this->database3(),
self::database3(),
'Scorex',
[
['Subject', 'Score'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public function testDCountAsWorksheetFormula($expectedResult, $database, $field,
self::assertEqualsWithDelta($expectedResult, $result, 1.0e-12);
}

private function database4(): array
private static function database4(): array
{
return [
['Status', 'Value'],
Expand All @@ -52,12 +52,12 @@ private function database4(): array
];
}

public function providerDCount(): array
public static function providerDCount(): array
{
return [
[
1,
$this->database1(),
self::database1(),
'Age',
[
['Tree', 'Height', 'Height'],
Expand All @@ -66,7 +66,7 @@ public function providerDCount(): array
],
[
1,
$this->database3(),
self::database3(),
'Score',
[
['Subject', 'Gender'],
Expand All @@ -75,7 +75,7 @@ public function providerDCount(): array
],
[
1,
$this->database3(),
self::database3(),
'Score',
[
['Subject', 'Gender'],
Expand All @@ -84,7 +84,7 @@ public function providerDCount(): array
],
[
3,
$this->database4(),
self::database4(),
'Value',
[
['Status'],
Expand All @@ -93,7 +93,7 @@ public function providerDCount(): array
],
[
5,
$this->database4(),
self::database4(),
'Value',
[
['Status'],
Expand All @@ -102,13 +102,13 @@ public function providerDCount(): array
],
'field column number okay' => [
0,
$this->database1(),
self::database1(),
1,
$this->database1(),
self::database1(),
],
'omitted field name' => [
ExcelError::VALUE(),
$this->database3(),
self::database3(),
null,
[
['Subject', 'Score'],
Expand All @@ -121,15 +121,15 @@ public function providerDCount(): array
content to return #VALUE! as an invalid name would */
'field column number too high' => [
ExcelError::VALUE(),
$this->database1(),
self::database1(),
99,
$this->database1(),
self::database1(),
],
'field column number too low' => [
ExcelError::VALUE(),
$this->database1(),
self::database1(),
0,
$this->database1(),
self::database1(),
],
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ public function testDGetAsWorksheetFormula($expectedResult, $database, $field, $
self::assertEqualsWithDelta($expectedResult, $result, 1.0e-12);
}

public function providerDGet(): array
public static function providerDGet(): array
{
return [
[
ExcelError::NAN(),
$this->database1(),
self::database1(),
'Yield',
[
['Tree'],
Expand All @@ -52,7 +52,7 @@ public function providerDGet(): array
],
[
10,
$this->database1(),
self::database1(),
'Yield',
[
['Tree', 'Height', 'Height'],
Expand All @@ -62,7 +62,7 @@ public function providerDGet(): array
],
[
188000,
$this->database2(),
self::database2(),
'Sales',
[
['Sales Rep.', 'Quarter'],
Expand All @@ -71,7 +71,7 @@ public function providerDGet(): array
],
[
ExcelError::NAN(),
$this->database2(),
self::database2(),
'Sales',
[
['Area', 'Quarter'],
Expand All @@ -80,9 +80,9 @@ public function providerDGet(): array
],
'omitted field name' => [
ExcelError::VALUE(),
$this->database1(),
self::database1(),
null,
$this->database1(),
self::database1(),
],
];
}
Expand Down
Loading

0 comments on commit 1187825

Please sign in to comment.