Clientbase
/
PhpSpreadsheet
mirror of https://github.com/basemaster/PhpSpreadsheet.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Big Memory Leak in One Test (#2958) 

* Big Memory Leak in One Test

Many tests leak a little by not issuing disconnectWorksheets at the end. CellMatcherTest leaks a lot by opening a spreadsheet many times. Phpunit reported a high watermark of 390MB; fixing CellMatcherTest brings that figure down to 242MB. I have also changed it to use a formal assertion in many cases where markTestSkipped was (IMO inappropriately) used.

* Another Leak

Issue2301Test lacks a disconnect. Adding one reduces HWM from 242MB to 224.
This commit is contained in:
oleibman 2022-07-23 07:46:32 -07:00 committed by GitHub
parent 0ee13303f3
commit abc1d3db70
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 44 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
tests/PhpSpreadsheetTests/Reader/Xlsx/Issue2301Test.php
View File

@ -24,5 +24,7 @@ class Issue2301Test extends \PHPUnit\Framework\TestCase
self::assertSame('Arial CE', $font->getName()); self::assertSame('Arial CE', $font->getName());
self::assertSame(9.0, $font->getSize()); self::assertSame(9.0, $font->getSize());
self::assertSame('protected', $sheet->getCell('BT10')->getStyle()->getProtection()->getHidden()); self::assertSame('protected', $sheet->getCell('BT10')->getStyle()->getProtection()->getHidden());
$spreadsheet->disconnectWorksheets();
unset($spreadsheet);
} }
} }

105
tests/PhpSpreadsheetTests/Style/ConditionalFormatting/CellMatcherTest.php
View File

@ -10,15 +10,24 @@ use PHPUnit\Framework\TestCase;
class CellMatcherTest extends TestCase class CellMatcherTest extends TestCase
{ {
/** /**
* @var Spreadsheet * @var ?Spreadsheet
*/ */
protected $spreadsheet; protected $spreadsheet;
protected function setUp(): void protected function loadSpreadsheet(): Spreadsheet
{ {
$filename = 'tests/data/Style/ConditionalFormatting/CellMatcher.xlsx'; $filename = 'tests/data/Style/ConditionalFormatting/CellMatcher.xlsx';
$reader = IOFactory::createReader('Xlsx'); $reader = IOFactory::createReader('Xlsx');
$this->spreadsheet = $reader->load($filename);
return $reader->load($filename);
}
protected function tearDown(): void
{
if ($this->spreadsheet !== null) {
$this->spreadsheet->disconnectWorksheets();
$this->spreadsheet = null;
}
} }
/** /**
@ -26,16 +35,13 @@ class CellMatcherTest extends TestCase
*/ */
public function testBasicCellIsComparison(string $sheetname, string $cellAddress, array $expectedMatches): void public function testBasicCellIsComparison(string $sheetname, string $cellAddress, array $expectedMatches): void
{ {
$this->spreadsheet = $this->loadSpreadsheet();
$worksheet = $this->spreadsheet->getSheetByName($sheetname); $worksheet = $this->spreadsheet->getSheetByName($sheetname);
if ($worksheet === null) { self::assertNotNull($worksheet, "$sheetname not found in test workbook");
self::markTestSkipped("{$sheetname} not found in test workbook");
}
$cell = $worksheet->getCell($cellAddress); $cell = $worksheet->getCell($cellAddress);
$cfRange = $worksheet->getConditionalRange($cell->getCoordinate()); $cfRange = $worksheet->getConditionalRange($cell->getCoordinate());
if ($cfRange === null) { self::assertNotNull($cfRange, "{$cellAddress} is not in a Conditional Format range");
self::markTestSkipped("{$cellAddress} is not in a Conditional Format range");
}
$cfStyles = $worksheet->getConditionalStyles($cell->getCoordinate()); $cfStyles = $worksheet->getConditionalStyles($cell->getCoordinate());
$matcher = new CellMatcher($cell, $cfRange); $matcher = new CellMatcher($cell, $cfRange);
@ -82,16 +88,13 @@ class CellMatcherTest extends TestCase
*/ */
public function testRangeCellIsComparison(string $sheetname, string $cellAddress, bool $expectedMatch): void public function testRangeCellIsComparison(string $sheetname, string $cellAddress, bool $expectedMatch): void
{ {
$this->spreadsheet = $this->loadSpreadsheet();
$worksheet = $this->spreadsheet->getSheetByName($sheetname); $worksheet = $this->spreadsheet->getSheetByName($sheetname);
if ($worksheet === null) { self::assertNotNull($worksheet, "$sheetname not found in test workbook");
self::markTestSkipped("{$sheetname} not found in test workbook");
}
$cell = $worksheet->getCell($cellAddress); $cell = $worksheet->getCell($cellAddress);
$cfRange = $worksheet->getConditionalRange($cell->getCoordinate()); $cfRange = $worksheet->getConditionalRange($cell->getCoordinate());
if ($cfRange === null) { self::assertNotNull($cfRange, "$cellAddress is not in a Conditional Format range");
self::markTestSkipped("{$cellAddress} is not in a Conditional Format range");
}
$cfStyle = $worksheet->getConditionalStyles($cell->getCoordinate()); $cfStyle = $worksheet->getConditionalStyles($cell->getCoordinate());
$matcher = new CellMatcher($cell, $cfRange); $matcher = new CellMatcher($cell, $cfRange);
@ -128,16 +131,13 @@ class CellMatcherTest extends TestCase
*/ */
public function testCellIsMultipleExpression(string $sheetname, string $cellAddress, array $expectedMatches): void public function testCellIsMultipleExpression(string $sheetname, string $cellAddress, array $expectedMatches): void
{ {
$this->spreadsheet = $this->loadSpreadsheet();
$worksheet = $this->spreadsheet->getSheetByName($sheetname); $worksheet = $this->spreadsheet->getSheetByName($sheetname);
if ($worksheet === null) { self::assertNotNull($worksheet, "$sheetname not found in test workbook");
self::markTestSkipped("{$sheetname} not found in test workbook");
}
$cell = $worksheet->getCell($cellAddress); $cell = $worksheet->getCell($cellAddress);
$cfRange = $worksheet->getConditionalRange($cell->getCoordinate()); $cfRange = $worksheet->getConditionalRange($cell->getCoordinate());
if ($cfRange === null) { self::assertNotNull($cfRange, "$cellAddress is not in a Conditional Format range");
self::markTestSkipped("{$cellAddress} is not in a Conditional Format range");
}
$cfStyles = $worksheet->getConditionalStyles($cell->getCoordinate()); $cfStyles = $worksheet->getConditionalStyles($cell->getCoordinate());
$matcher = new CellMatcher($cell, $cfRange); $matcher = new CellMatcher($cell, $cfRange);
@ -167,16 +167,13 @@ class CellMatcherTest extends TestCase
*/ */
public function testCellIsExpression(string $sheetname, string $cellAddress, bool $expectedMatch): void public function testCellIsExpression(string $sheetname, string $cellAddress, bool $expectedMatch): void
{ {
$this->spreadsheet = $this->loadSpreadsheet();
$worksheet = $this->spreadsheet->getSheetByName($sheetname); $worksheet = $this->spreadsheet->getSheetByName($sheetname);
if ($worksheet === null) { self::assertNotNull($worksheet, "$sheetname not found in test workbook");
self::markTestSkipped("{$sheetname} not found in test workbook");
}
$cell = $worksheet->getCell($cellAddress); $cell = $worksheet->getCell($cellAddress);
$cfRange = $worksheet->getConditionalRange($cell->getCoordinate()); $cfRange = $worksheet->getConditionalRange($cell->getCoordinate());
if ($cfRange === null) { self::assertNotNull($cfRange, "$cellAddress is not in a Conditional Format range");
self::markTestSkipped("{$cellAddress} is not in a Conditional Format range");
}
$cfStyle = $worksheet->getConditionalStyles($cell->getCoordinate()); $cfStyle = $worksheet->getConditionalStyles($cell->getCoordinate());
$matcher = new CellMatcher($cell, $cfRange); $matcher = new CellMatcher($cell, $cfRange);
@ -216,16 +213,13 @@ class CellMatcherTest extends TestCase
*/ */
public function testTextExpressions(string $sheetname, string $cellAddress, bool $expectedMatch): void public function testTextExpressions(string $sheetname, string $cellAddress, bool $expectedMatch): void
{ {
$this->spreadsheet = $this->loadSpreadsheet();
$worksheet = $this->spreadsheet->getSheetByName($sheetname); $worksheet = $this->spreadsheet->getSheetByName($sheetname);
if ($worksheet === null) { self::assertNotNull($worksheet, "$sheetname not found in test workbook");
self::markTestSkipped("{$sheetname} not found in test workbook");
}
$cell = $worksheet->getCell($cellAddress); $cell = $worksheet->getCell($cellAddress);
$cfRange = $worksheet->getConditionalRange($cell->getCoordinate()); $cfRange = $worksheet->getConditionalRange($cell->getCoordinate());
if ($cfRange === null) { self::assertNotNull($cfRange, "$cellAddress is not in a Conditional Format range");
self::markTestSkipped("{$cellAddress} is not in a Conditional Format range");
}
$cfStyle = $worksheet->getConditionalStyles($cell->getCoordinate()); $cfStyle = $worksheet->getConditionalStyles($cell->getCoordinate());
$matcher = new CellMatcher($cell, $cfRange); $matcher = new CellMatcher($cell, $cfRange);
@ -329,16 +323,13 @@ class CellMatcherTest extends TestCase
*/ */
public function testBlankExpressions(string $sheetname, string $cellAddress, array $expectedMatches): void public function testBlankExpressions(string $sheetname, string $cellAddress, array $expectedMatches): void
{ {
$this->spreadsheet = $this->loadSpreadsheet();
$worksheet = $this->spreadsheet->getSheetByName($sheetname); $worksheet = $this->spreadsheet->getSheetByName($sheetname);
if ($worksheet === null) { self::assertNotNull($worksheet, "$sheetname not found in test workbook");
self::markTestSkipped("{$sheetname} not found in test workbook");
}
$cell = $worksheet->getCell($cellAddress); $cell = $worksheet->getCell($cellAddress);
$cfRange = $worksheet->getConditionalRange($cell->getCoordinate()); $cfRange = $worksheet->getConditionalRange($cell->getCoordinate());
if ($cfRange === null) { self::assertNotNull($cfRange, "$cellAddress is not in a Conditional Format range");
self::markTestSkipped("{$cellAddress} is not in a Conditional Format range");
}
$cfStyles = $worksheet->getConditionalStyles($cell->getCoordinate()); $cfStyles = $worksheet->getConditionalStyles($cell->getCoordinate());
$matcher = new CellMatcher($cell, $cfRange); $matcher = new CellMatcher($cell, $cfRange);
@ -365,16 +356,13 @@ class CellMatcherTest extends TestCase
*/ */
public function testErrorExpressions(string $sheetname, string $cellAddress, array $expectedMatches): void public function testErrorExpressions(string $sheetname, string $cellAddress, array $expectedMatches): void
{ {
$this->spreadsheet = $this->loadSpreadsheet();
$worksheet = $this->spreadsheet->getSheetByName($sheetname); $worksheet = $this->spreadsheet->getSheetByName($sheetname);
if ($worksheet === null) { self::assertNotNull($worksheet, "$sheetname not found in test workbook");
self::markTestSkipped("{$sheetname} not found in test workbook");
}
$cell = $worksheet->getCell($cellAddress); $cell = $worksheet->getCell($cellAddress);
$cfRange = $worksheet->getConditionalRange($cell->getCoordinate()); $cfRange = $worksheet->getConditionalRange($cell->getCoordinate());
if ($cfRange === null) { self::assertNotNull($cfRange, "$cellAddress is not in a Conditional Format range");
self::markTestSkipped("{$cellAddress} is not in a Conditional Format range");
}
$cfStyles = $worksheet->getConditionalStyles($cell->getCoordinate()); $cfStyles = $worksheet->getConditionalStyles($cell->getCoordinate());
$matcher = new CellMatcher($cell, $cfRange); $matcher = new CellMatcher($cell, $cfRange);
@ -400,16 +388,13 @@ class CellMatcherTest extends TestCase
*/ */
public function testDateOccurringExpressions(string $sheetname, string $cellAddress, bool $expectedMatch): void public function testDateOccurringExpressions(string $sheetname, string $cellAddress, bool $expectedMatch): void
{ {
$this->spreadsheet = $this->loadSpreadsheet();
$worksheet = $this->spreadsheet->getSheetByName($sheetname); $worksheet = $this->spreadsheet->getSheetByName($sheetname);
if ($worksheet === null) { self::assertNotNull($worksheet, "$sheetname not found in test workbook");
self::markTestSkipped("{$sheetname} not found in test workbook");
}
$cell = $worksheet->getCell($cellAddress); $cell = $worksheet->getCell($cellAddress);
$cfRange = $worksheet->getConditionalRange($cell->getCoordinate()); $cfRange = $worksheet->getConditionalRange($cell->getCoordinate());
if ($cfRange === null) { self::assertNotNull($cfRange, "$cellAddress is not in a Conditional Format range");
self::markTestSkipped("{$cellAddress} is not in a Conditional Format range");
}
$cfStyle = $worksheet->getConditionalStyles($cell->getCoordinate()); $cfStyle = $worksheet->getConditionalStyles($cell->getCoordinate());
$matcher = new CellMatcher($cell, $cfRange); $matcher = new CellMatcher($cell, $cfRange);
@ -447,16 +432,13 @@ class CellMatcherTest extends TestCase
*/ */
public function testDuplicatesExpressions(string $sheetname, string $cellAddress, array $expectedMatches): void public function testDuplicatesExpressions(string $sheetname, string $cellAddress, array $expectedMatches): void
{ {
$this->spreadsheet = $this->loadSpreadsheet();
$worksheet = $this->spreadsheet->getSheetByName($sheetname); $worksheet = $this->spreadsheet->getSheetByName($sheetname);
if ($worksheet === null) { self::assertNotNull($worksheet, "$sheetname not found in test workbook");
self::markTestSkipped("{$sheetname} not found in test workbook");
}
$cell = $worksheet->getCell($cellAddress); $cell = $worksheet->getCell($cellAddress);
$cfRange = $worksheet->getConditionalRange($cell->getCoordinate()); $cfRange = $worksheet->getConditionalRange($cell->getCoordinate());
if ($cfRange === null) { self::AssertNotNull($cfRange, "$cellAddress is not in a Conditional Format range");
self::markTestSkipped("{$cellAddress} is not in a Conditional Format range");
}
$cfStyles = $worksheet->getConditionalStyles($cell->getCoordinate()); $cfStyles = $worksheet->getConditionalStyles($cell->getCoordinate());
$matcher = new CellMatcher($cell, $cfRange); $matcher = new CellMatcher($cell, $cfRange);
@ -486,16 +468,13 @@ class CellMatcherTest extends TestCase
*/ */
public function testCrossWorksheetExpressions(string $sheetname, string $cellAddress, bool $expectedMatch): void public function testCrossWorksheetExpressions(string $sheetname, string $cellAddress, bool $expectedMatch): void
{ {
$this->spreadsheet = $this->loadSpreadsheet();
$worksheet = $this->spreadsheet->getSheetByName($sheetname); $worksheet = $this->spreadsheet->getSheetByName($sheetname);
if ($worksheet === null) { self::assertNotNull($worksheet, "$sheetname not found in test workbook");
self::markTestSkipped("{$sheetname} not found in test workbook");
}
$cell = $worksheet->getCell($cellAddress); $cell = $worksheet->getCell($cellAddress);
$cfRange = $worksheet->getConditionalRange($cell->getCoordinate()); $cfRange = $worksheet->getConditionalRange($cell->getCoordinate());
if ($cfRange === null) { self::assertNotNull($cfRange, "$cellAddress is not in a Conditional Format range");
self::markTestSkipped("{$cellAddress} is not in a Conditional Format range");
}
$cfStyle = $worksheet->getConditionalStyles($cell->getCoordinate()); $cfStyle = $worksheet->getConditionalStyles($cell->getCoordinate());
$matcher = new CellMatcher($cell, $cfRange); $matcher = new CellMatcher($cell, $cfRange);