Leave the test suite with nothing to report, and keep it that way (#920)

Three things the suite has been printing under every run, none of them noise.

`getimagesize()` answers `false` for a file it cannot measure -- an SVG, a
video -- and both callers destructured that answer straight into a width and a
height: `Cannot use bool as array`, three tests, twice each. The size such a
file ends up with is the size it already had, which is nothing, so the fix is
to assign only when there is something to assign.

`HashTable::add()` looked the source up by `getHashIndex()`, which is null
until the table has given the object one. `isset($array[null])` reads the key
as an empty string, so the lookup was correct and merely deprecated -- 184
tests said so on every run. It now asks whether there is an index first.

The five mock objects nobody configured an expectation on are stubs, which is
what PHPUnit says in the notice, so they are built with `createStub()`.

The gates that hold this are split in two, because PHPUnit splits them.
`phpunit.xml.dist` takes the three the declared 9.3 schema knows --
`failOnWarning`, `failOnRisky`, `failOnEmptyTestSuite` -- and the rest exist
only on the command line, so the workflow passes `--fail-on-deprecation`,
`--fail-on-notice` and `--fail-on-phpunit-deprecation` on one leg. Without
them PHPUnit ends a run that had something to report with "OK, but there were
issues" and exit code 0, and CI stays green while the report grows back.

That leg is 8.5: the newest PHP is where a deprecation appears first. It also
takes the coverage run over from 8.3, so that exactly one leg of the matrix is
out of the ordinary rather than two.

Author: dkulyk

.github/workflows/php.yml

@@ -75,23 +75,26 @@ jobs:
           with:
               php-version: ${{ matrix.php }}
               extensions: gd, xml, zip
-              coverage: ${{ (matrix.php == '8.3') && 'xdebug' || 'none' }}
+              coverage: ${{ (matrix.php == '8.5') && 'xdebug' || 'none' }}
 
       -   uses: actions/checkout@v2
 
       -   name: Composer Install
           run: composer install --ansi --prefer-dist --no-interaction --no-progress
 
       -   name: Run phpunit
-          if: matrix.php != '8.3'
+          if: matrix.php != '8.5'
           run: ./vendor/bin/phpunit -c phpunit.xml.dist --no-coverage
 
+      # the newest PHP is where a new deprecation shows up first, so that leg is the one
+      # that refuses to be green while the suite still has something to report -- and the
+      # one that gathers the coverage, so that there is a single leg out of the ordinary
       -   name: Run phpunit
-          if: matrix.php == '8.3'
-          run: ./vendor/bin/phpunit -c phpunit.xml.dist --coverage-clover build/clover.xml
+          if: matrix.php == '8.5'
+          run: ./vendor/bin/phpunit -c phpunit.xml.dist --coverage-clover build/clover.xml --fail-on-deprecation --fail-on-notice --fail-on-phpunit-deprecation
 
       -   name: Upload coverage results to Coveralls
-          if: matrix.php == '8.3'
+          if: matrix.php == '8.5'
           env:
             COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           run: |

docs/changes/1.3.0.md

@@ -21,6 +21,7 @@
 - A master slide is no longer born with a white background nobody asked for by [@dkulyk](http://github.com/dkulyk) in [#909](https://github.com/PHPOffice/PHPPresentation/pull/909)
 - ODPresentation Writer : Fixed the background of a master slide being dropped on save by [@dkulyk](http://github.com/dkulyk) in [#912](https://github.com/PHPOffice/PHPPresentation/pull/912)
 - PowerPoint2007 Reader : Fixed a shape that asked for no fill being read as having no fill at all, which crashes the ODPresentation Writer on save, by [@dkulyk](http://github.com/dkulyk) in [#915](https://github.com/PHPOffice/PHPPresentation/pull/915)
+- Fixed a file with no measurable size (an SVG, a video) warning on `setPath()`, and a fresh object being looked up in the hash table under a null key, by [@dkulyk](http://github.com/dkulyk) in [#920](https://github.com/PHPOffice/PHPPresentation/pull/920)
 
 ## BC Breaks
 - `\PhpOffice\PhpPresentation\Slide\SlideMaster` is constructed without a background. A deck that relied on the white fill it used to write sets one with `setBackground()`, as [the documentation](https://github.com/PHPOffice/PHPPresentation/blob/develop/docs/usage/slides/introduction.md) shows.

phpunit.xml.dist

@@ -6,6 +6,9 @@
     colors="true"
     processIsolation="false"
     stopOnFailure="false"
+    failOnWarning="true"
+    failOnRisky="true"
+    failOnEmptyTestSuite="true"
     xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
   <source>
     <include>

src/PhpPresentation/HashTable.php

@@ -71,7 +71,7 @@ public function add(ComparableInterface $pSource): void
         // Determine hashcode
         $hashIndex = $pSource->getHashIndex();
         $hashCode = $pSource->getHashCode();
-        if (isset($this->keyMap[$hashIndex])) {
+        if (null !== $hashIndex && isset($this->keyMap[$hashIndex])) {
             $hashCode = $this->keyMap[$hashIndex];
         }
 

src/PhpPresentation/Shape/Drawing/File.php

@@ -55,7 +55,13 @@ public function setPath(string $pValue = '', bool $pVerifyFile = true): self
 
         if ($pVerifyFile) {
             if (0 == $this->width && 0 == $this->height) {
-                [$this->width, $this->height] = getimagesize($this->getPath());
+                // Not every file a shape can carry is one `getimagesize()` can measure -- an SVG
+                // and a video both come back as `false`, and a size of nothing is what they had
+                // before this asked.
+                $imageSize = getimagesize($this->getPath());
+                if (is_array($imageSize)) {
+                    [$this->width, $this->height] = $imageSize;
+                }
             }
         }
 

src/PhpPresentation/Slide/Background/Image.php

@@ -76,8 +76,11 @@ public function setPath(string $pValue = '', bool $pVerifyFile = true)
             }
 
             if (0 == $this->width && 0 == $this->height) {
-                // Get width/height
-                [$this->width, $this->height] = getimagesize($pValue);
+                // Get width/height, when the file is one that has them
+                $imageSize = getimagesize($pValue);
+                if (is_array($imageSize)) {
+                    [$this->width, $this->height] = $imageSize;
+                }
             }
         }
         $this->path = $pValue;

tests/PhpPresentation/Tests/Shape/AutoShapeTest.php

@@ -22,7 +22,7 @@
 
 use PhpOffice\PhpPresentation\Shape\AutoShape;
 use PhpOffice\PhpPresentation\Style\Outline;
-use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\MockObject\Stub;
 use PHPUnit\Framework\TestCase;
 
 class AutoShapeTest extends TestCase
@@ -39,8 +39,8 @@ public function testConstruct(): void
 
     public function testOutline(): void
     {
-        /** @var MockObject&Outline $mock */
-        $mock = $this->getMockBuilder(Outline::class)->getMock();
+        /** @var Outline&Stub $mock */
+        $mock = self::createStub(Outline::class);
 
         $object = new AutoShape();
         self::assertInstanceOf(Outline::class, $object->getOutline());

tests/PhpPresentation/Tests/Shape/Chart/AxisTest.php

@@ -24,7 +24,7 @@
 use PhpOffice\PhpPresentation\Shape\Chart\Gridlines;
 use PhpOffice\PhpPresentation\Style\Font;
 use PhpOffice\PhpPresentation\Style\Outline;
-use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\MockObject\Stub;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -105,8 +105,8 @@ public function testGridLines(): void
     {
         $object = new Axis();
 
-        /** @var Gridlines&MockObject $oMock */
-        $oMock = $this->getMockBuilder(Gridlines::class)->getMock();
+        /** @var Gridlines&Stub $oMock */
+        $oMock = self::createStub(Gridlines::class);
 
         self::assertInstanceOf(Axis::class, $object->setMajorGridlines($oMock));
         self::assertInstanceOf(Gridlines::class, $object->getMajorGridlines());
@@ -149,8 +149,8 @@ public function testLabelRotation(): void
 
     public function testOutline(): void
     {
-        /** @var MockObject&Outline $oMock */
-        $oMock = $this->getMockBuilder(Outline::class)->getMock();
+        /** @var Outline&Stub $oMock */
+        $oMock = self::createStub(Outline::class);
 
         $object = new Axis();
         self::assertInstanceOf(Outline::class, $object->getOutline());

tests/PhpPresentation/Tests/Shape/Chart/GridlinesTest.php

@@ -22,7 +22,7 @@
 
 use PhpOffice\PhpPresentation\Shape\Chart\Gridlines;
 use PhpOffice\PhpPresentation\Style\Outline;
-use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\MockObject\Stub;
 use PHPUnit\Framework\TestCase;
 
 class GridlinesTest extends TestCase
@@ -38,8 +38,8 @@ public function testGetSetOutline(): void
     {
         $object = new Gridlines();
 
-        /** @var MockObject&Outline $oStub */
-        $oStub = $this->getMockBuilder(Outline::class)->getMock();
+        /** @var Outline&Stub $oStub */
+        $oStub = self::createStub(Outline::class);
 
         self::assertInstanceOf(Outline::class, $object->getOutline());
         self::assertInstanceOf('PhpOffice\PhpPresentation\Shape\Chart\Gridlines', $object->setOutline($oStub));

tests/PhpPresentation/Tests/Shape/CommentTest.php

@@ -22,7 +22,7 @@
 
 use PhpOffice\PhpPresentation\Shape\Comment;
 use PhpOffice\PhpPresentation\Shape\Comment\Author;
-use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\MockObject\Stub;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -47,8 +47,8 @@ public function testGetSetAuthor(): void
     {
         $object = new Comment();
 
-        /** @var Author&MockObject $oStub */
-        $oStub = $this->getMockBuilder(Author::class)->getMock();
+        /** @var Author&Stub $oStub */
+        $oStub = self::createStub(Author::class);
 
         self::assertNull($object->getAuthor());
         self::assertInstanceOf(Comment::class, $object->setAuthor($oStub));