From c8743061a3444a8cd4f294e64a7e941ef601a5da Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 27 Apr 2023 20:20:25 -0700 Subject: [PATCH] PhpUnit 10 Compatibility Part 3 (Last) (#3530) * PhpUnit 10 Compatibility Part 3 (Last) Final changes for PhpUnit 10, including enabling it for testing. This finishes the work of PR #3523 and PR #3526. The major remaining problem with PhpUnit 10 is that earlier releases converted notices and warnings to exceptions, and 10 does not. Not having the information provided by the messages seems risky to me. Fortunately, it appears that you can add an error handler to the test bootstrap for 10 and make it act like earlier versions; I have done so. In order to demonstrate the effectiveness of this handler, a new otherwise unused class Helper/Handler and tests for that class are added. As part of the testing of this change, it became apparent that the fopen in OLE::getStream attempts to create a dynamic property $context in Shared/OLE/ChainedBlockStream, and that action is deprecated in Php8.2. Adding the property to the class eliminates that problem. No executable code is added, and this is the only change to source code. There also seems to have been a change in assertXmlStringEqualsXmlString in PhpUnit 10. The only test which uses that method is Chart/Issue589Test, and both the places which use that method could just as easily and effectively use assertSame. They are changed to do so. * Remove Phpunit Verbose Option Not supported in PhpUnit 10. I'm not at all sure that this is the correct solution for this problem. * Try Changing Phpunit Command for Different Php Releases Not sure how to test this locally. We'll see if it works on github. * Another main.yml attempt We shall see. * Eliminate One Test Not sure why testDeprecated is not working in Github; it works locally. Disable it for now and continue to research. * Show Incomplete and Other Messages in PhpUnit 10 6 new command line args to replace the 1 they got rid of. * Restore Disabled Test Deprecated messages are suppressed by default setting for error_reporting. Switch that to E_ALL in bootstrap and restore original test. * Add Deprecation Tests for PhpUnit 9- Default configuration option caused deprecation messages to be suppressed. Change the option. --- .github/workflows/main.yml | 9 ++- composer.json | 2 +- phpunit.xml.dist | 3 +- phpunit10.xml.dist | 15 ++++ src/PhpSpreadsheet/Helper/Handler.php | 46 ++++++++++++ .../Shared/OLE/ChainedBlockStream.php | 3 + .../Chart/Issue589Test.php | 4 +- .../Helper/HandlerTest.php | 75 +++++++++++++++++++ tests/bootstrap.php | 32 ++++++++ 9 files changed, 184 insertions(+), 5 deletions(-) create mode 100644 phpunit10.xml.dist create mode 100644 src/PhpSpreadsheet/Helper/Handler.php create mode 100644 tests/PhpSpreadsheetTests/Helper/HandlerTest.php diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 35e4f4455d..a478a1b6d8 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -57,11 +57,18 @@ jobs: - name: Setup problem matchers for PHPUnit run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" - - name: "Run PHPUnit tests (Experimental: ${{ matrix.experimental }})" + - name: "Run PHPUnit tests 1 (Experimental: ${{ matrix.experimental }})" env: FAILURE_ACTION: "${{ matrix.experimental == true }}" + if: ${{ matrix.php-version == '7.4' || matrix.php-version == '8.0' }} run: vendor/bin/phpunit --verbose || $FAILURE_ACTION + - name: "Run PHPUnit tests 2 (Experimental: ${{ matrix.experimental }})" + env: + FAILURE_ACTION: "${{ matrix.experimental == true }}" + if: ${{ matrix.php-version == '8.1' || matrix.php-version == '8.2' || matrix.php-version == 'nightly' }} + run: vendor/bin/phpunit -c phpunit10.xml.dist --display-incomplete --display-skipped --display-deprecations --display-errors --display-notices --display-warnings || $FAILURE_ACTION + php-cs-fixer: runs-on: ubuntu-latest steps: diff --git a/composer.json b/composer.json index de3afb1e55..3678c8eb62 100644 --- a/composer.json +++ b/composer.json @@ -92,7 +92,7 @@ "phpcompatibility/php-compatibility": "^9.3", "phpstan/phpstan": "^1.1", "phpstan/phpstan-phpunit": "^1.0", - "phpunit/phpunit": "^8.5 || ^9.0", + "phpunit/phpunit": "^8.5 || ^9.0 || ^10.0", "squizlabs/php_codesniffer": "^3.7", "tecnickcom/tcpdf": "^6.5" }, diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 9b195eb8f8..896b73339f 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,5 +1,5 @@ - + ./src @@ -7,6 +7,7 @@ + ./tests/PhpSpreadsheetTests diff --git a/phpunit10.xml.dist b/phpunit10.xml.dist new file mode 100644 index 0000000000..207d8ec978 --- /dev/null +++ b/phpunit10.xml.dist @@ -0,0 +1,15 @@ + + + + + + + + ./tests/PhpSpreadsheetTests + + + + ./src + + + diff --git a/src/PhpSpreadsheet/Helper/Handler.php b/src/PhpSpreadsheet/Helper/Handler.php new file mode 100644 index 0000000000..d05197cef0 --- /dev/null +++ b/src/PhpSpreadsheet/Helper/Handler.php @@ -0,0 +1,46 @@ +', $actualXml); + self::assertSame('', $actualXml); } } } @@ -153,7 +153,7 @@ public function testLineChartFillIgnoresColorArray(): void if ($actualXml === false) { self::fail('Failure saving the spPr element as xml string!'); } else { - self::assertXmlStringEqualsXmlString('', $actualXml); + self::assertSame('', $actualXml); } } } diff --git a/tests/PhpSpreadsheetTests/Helper/HandlerTest.php b/tests/PhpSpreadsheetTests/Helper/HandlerTest.php new file mode 100644 index 0000000000..8eee3064f3 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Helper/HandlerTest.php @@ -0,0 +1,75 @@ +getMessage()); + } + } + + public function testNotice(): void + { + try { + Handler::notice('invalidtz'); + self::fail('Expected error/exception did not happen'); + } catch (Throwable $e) { + self::assertStringContainsString('Timezone', $e->getMessage()); + } + } + + public function testWarning(): void + { + try { + Handler::warning(); + self::fail('Expected error/exception did not happen'); + } catch (Throwable $e) { + self::assertStringContainsString('ailed to open stream', $e->getMessage()); + } + } + + public function testUserDeprecated(): void + { + try { + Handler::userDeprecated(); + self::fail('Expected error/exception did not happen'); + } catch (Throwable $e) { + self::assertStringContainsString('hello', $e->getMessage()); + } + } + + public function testUserNotice(): void + { + try { + Handler::userNotice(); + self::fail('Expected error/exception did not happen'); + } catch (Throwable $e) { + self::assertStringContainsString('userNotice', $e->getMessage()); + } + } + + public function testUserWarning(): void + { + try { + Handler::userWarning(); + self::fail('Expected error/exception did not happen'); + } catch (Throwable $e) { + self::assertStringContainsString('userWarning', $e->getMessage()); + } + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 9ebd3f2623..6b6dc01c88 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -4,3 +4,35 @@ // PHP 5.3 Compat //date_default_timezone_set('Europe/London'); + +function phpunit10ErrorHandler(int $errno, string $errstr, string $filename, int $lineno): bool +{ + $x = error_reporting() & $errno; + if ( + in_array( + $errno, + [ + E_DEPRECATED, + E_WARNING, + E_NOTICE, + E_USER_DEPRECATED, + E_USER_NOTICE, + E_USER_WARNING, + ], + true + ) + ) { + if (0 === $x) { + return true; // message suppressed - stop error handling + } + + throw new \Exception("$errstr $filename $lineno"); + } + + return false; // continue error handling +} + +if (!method_exists(\PHPUnit\Framework\TestCase::class, 'setOutputCallback')) { + ini_set('error_reporting', E_ALL); + set_error_handler('phpunit10ErrorHandler'); +}