Adding more tests

.travis.yml

@@ -12,7 +12,7 @@ install:
 
 script:
   - mkdir -p build/logs
-  - php vendor/bin/phpunit --coverage-clover build/logs/clover.xml
+  - php vendor/bin/phpunit --coverage-clover build/logs/clover.xml --coverage-text
 
 after_script:
   - if [[ $TRAVIS_PHP_VERSION != 'hhvm' && $TRAVIS_PHP_VERSION != '7.0' ]]; then php vendor/bin/ocular code-coverage:upload --format=php-clover build/logs/clover.xml; fi

phpunit.xml

@@ -5,7 +5,6 @@
         colors="true"
         convertErrorsToExceptions="false"
         convertWarningsToExceptions="false"
-        strict="false"
         verbose="false">
 
     <testsuites>

src/Spout/Common/Helper/FileSystemHelper.php

@@ -63,7 +63,7 @@ class FileSystemHelper
         $filePath = $parentFolderPath . '/' . $fileName;
 
         $wasCreationSuccessful = file_put_contents($filePath, $fileContents);
-        if (!$wasCreationSuccessful) {
+        if ($wasCreationSuccessful === false) {
             throw new IOException('Unable to create file: ' . $filePath);
         }
 

src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php

@@ -154,7 +154,8 @@ class SharedStringsHelper
 
         $readError = libxml_get_last_error();
         if ($readError !== false) {
-            throw new IOException("The sharedStrings.xml file is invalid and cannot be read. [{$readError->message}]");
+            $readErrorMessage = trim($readError->message);
+            throw new IOException("The sharedStrings.xml file is invalid and cannot be read. [{$readErrorMessage}]");
         }
 
         // reset the setting to display XML warnings/errors

src/Spout/Reader/XLSX/RowIterator.php

@@ -131,12 +131,19 @@ class RowIterator implements IteratorInterface
      *
      * @return void
      * @throws \Box\Spout\Reader\Exception\SharedStringNotFoundException If a shared string was not found
+     * @throws \Box\Spout\Common\Exception\IOException If unable to read the sheet data XML
      */
     public function next()
     {
         $isInsideRowTag = false;
         $rowData = [];
 
+        // Use internal errors to avoid displaying lots of warning messages in case of invalid file
+        // For instance on HHVM, XMLReader->open() won't fail when trying to read a unexisting file within a zip...
+        // But the XMLReader->read() will fail!
+        libxml_clear_errors();
+        libxml_use_internal_errors(true);
+
         while ($this->xmlReader->read()) {
             if ($this->xmlReader->nodeType == \XMLReader::ELEMENT && $this->xmlReader->name === self::XML_NODE_DIMENSION) {
                 // Read dimensions of the sheet
@@ -180,6 +187,12 @@ class RowIterator implements IteratorInterface
             }
         }
 
+        $readError = libxml_get_last_error();
+        if ($readError !== false) {
+            $readErrorMessage = trim($readError->message);
+            throw new IOException("The {$this->sheetDataXMLFilePath} file cannot be read. [{$readErrorMessage}]");
+        }
+
         $this->rowDataBuffer = $rowData;
     }
 

tests/Spout/Reader/CSV/ReaderTest.php

@@ -54,6 +54,25 @@ class ReaderTest extends \PHPUnit_Framework_TestCase
         $reader->open($resourcePath);
     }
 
+    /**
+     * @expectedException \Box\Spout\Common\Exception\IOException
+     *
+     * @return void
+     */
+    public function testOpenShouldThrowExceptionIfCannotOpenFile()
+    {
+        $helperStub = $this->getMockBuilder('\Box\Spout\Common\Helper\GlobalFunctionsHelper')
+                        ->setMethods(['fopen'])
+                        ->getMock();
+        $helperStub->method('fopen')->willReturn(false);
+
+        $resourcePath = $this->getResourcePath('csv_standard.csv');
+
+        $reader = ReaderFactory::create(Type::CSV);
+        $reader->setGlobalFunctionsHelper($helperStub);
+        $reader->open($resourcePath);
+    }
+
 
     /**
      * @return void
@@ -161,6 +180,50 @@ class ReaderTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($expectedRows, $allRows);
     }
 
+    /**
+     * @return void
+     */
+    public function testReadMultipleTimesShouldRewindReader()
+    {
+        $allRows = [];
+        $resourcePath = $this->getResourcePath('csv_standard.csv');
+
+        $reader = ReaderFactory::create(Type::CSV);
+        $reader->open($resourcePath);
+
+        foreach ($reader->getSheetIterator() as $sheet) {
+            // do nothing
+        }
+
+        foreach ($reader->getSheetIterator() as $sheet) {
+            foreach ($sheet->getRowIterator() as $row) {
+                $allRows[] = $row;
+                break;
+            }
+
+            foreach ($sheet->getRowIterator() as $row) {
+                $allRows[] = $row;
+                break;
+            }
+        }
+
+        foreach ($reader->getSheetIterator() as $sheet) {
+            foreach ($sheet->getRowIterator() as $row) {
+                $allRows[] = $row;
+                break;
+            }
+        }
+
+        $reader->close();
+
+        $expectedRows = [
+            ['csv--11', 'csv--12', 'csv--13'],
+            ['csv--11', 'csv--12', 'csv--13'],
+            ['csv--11', 'csv--12', 'csv--13'],
+        ];
+        $this->assertEquals($expectedRows, $allRows);
+    }
+
     /**
      * @param string $fileName
      * @param string|void $fieldDelimiter

tests/Spout/Reader/XLSX/ReaderTest.php

@@ -6,6 +6,7 @@ use Box\Spout\Common\Exception\IOException;
 use Box\Spout\Common\Type;
 use Box\Spout\Reader\ReaderFactory;
 use Box\Spout\TestUsingResource;
+use Symfony\Component\Config\Definition\Exception\Exception;
 
 /**
  * Class ReaderTest
@@ -24,6 +25,7 @@ class ReaderTest extends \PHPUnit_Framework_TestCase
         return [
             ['/path/to/fake/file.xlsx'],
             ['file_with_no_sheets_in_content_types.xlsx'],
+            ['file_with_sheet_xml_not_matching_content_types.xlsx'],
             ['file_corrupted.xlsx'],
         ];
     }
@@ -37,7 +39,8 @@ class ReaderTest extends \PHPUnit_Framework_TestCase
      */
     public function testReadShouldThrowException($filePath)
     {
-        $this->getAllRowsForFile($filePath);
+        // using @ to prevent warnings/errors from being displayed
+        @$this->getAllRowsForFile($filePath);
     }
 
     /**
@@ -240,7 +243,8 @@ class ReaderTest extends \PHPUnit_Framework_TestCase
         $startTime = microtime(true);
 
         try {
-            $this->getAllRowsForFile($fileName);
+            // using @ to prevent warnings/errors from being displayed
+            @$this->getAllRowsForFile($fileName);
             $this->fail('An exception should have been thrown');
         } catch (IOException $exception) {
             $duration = microtime(true) - $startTime;
@@ -275,6 +279,60 @@ class ReaderTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($expectedRows, $allRows);
     }
 
+    /**
+     * @return void
+     */
+    public function testReadMultipleTimesShouldRewindReader()
+    {
+        $allRows = [];
+        $resourcePath = $this->getResourcePath('two_sheets_with_inline_strings.xlsx');
+
+        $reader = ReaderFactory::create(Type::XLSX);
+        $reader->open($resourcePath);
+
+        foreach ($reader->getSheetIterator() as $sheet) {
+            // do nothing
+        }
+
+        foreach ($reader->getSheetIterator() as $sheet) {
+            // this loop should only add the first row of the first sheet
+            foreach ($sheet->getRowIterator() as $row) {
+                $allRows[] = $row;
+                break;
+            }
+
+            // this loop should rewind the iterator and restart reading from the 1st row again
+            // therefore, it should only add the first row of the first sheet
+            foreach ($sheet->getRowIterator() as $row) {
+                $allRows[] = $row;
+                break;
+            }
+
+            // not reading any more sheets
+            break;
+        }
+
+        foreach ($reader->getSheetIterator() as $sheet) {
+            // this loop should only add the first row of the current sheet
+            foreach ($sheet->getRowIterator() as $row) {
+                $allRows[] = $row;
+                break;
+            }
+
+            // not breaking, so we keep reading the next sheets
+        }
+
+        $reader->close();
+
+        $expectedRows = [
+            ['s1 - A1', 's1 - B1', 's1 - C1', 's1 - D1', 's1 - E1'],
+            ['s1 - A1', 's1 - B1', 's1 - C1', 's1 - D1', 's1 - E1'],
+            ['s1 - A1', 's1 - B1', 's1 - C1', 's1 - D1', 's1 - E1'],
+            ['s2 - A1', 's2 - B1', 's2 - C1', 's2 - D1', 's2 - E1'],
+        ];
+        $this->assertEquals($expectedRows, $allRows);
+    }
+
     /**
      * @param string $fileName
      * @return array All the read rows the given file