From 1a0aab1a4ffc1f027f4e78b2d3c0050b1de4a4ce Mon Sep 17 00:00:00 2001 From: Owen Leibman Date: Fri, 27 Nov 2020 06:50:01 -0800 Subject: [PATCH 1/2] Improve Coverage in src/PhpSpreadsheet There are no changes to code. Additional tests are added, so that the following 6 items now have 100% test coverage: - Comment - DefinedName - DocumentGenerator - IOFactory - NamedFormula - NamedRange --- samples/Reader/sampleData/example1xls | Bin 0 -> 22528 bytes .../DefinedNameFormulaTest.php | 21 +++++ tests/PhpSpreadsheetTests/DefinedNameTest.php | 82 ++++++++++++++++++ .../DocumentGeneratorTest.php | 10 +++ .../Functional/CommentsTest.php | 15 +++- tests/PhpSpreadsheetTests/IOFactoryTest.php | 41 +++++++++ 6 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 samples/Reader/sampleData/example1xls diff --git a/samples/Reader/sampleData/example1xls b/samples/Reader/sampleData/example1xls new file mode 100644 index 0000000000000000000000000000000000000000..bd9bb110b9a41b0aab816ba9e20682ee5bb43aea GIT binary patch literal 22528 zcmeHPd2AfldH-g)%R?l!yhO^f#EePVk|=F`4@lQe*F_)S@#2pguA6q-dJdo`YODG=K%}1 z3Fr$81Z}D;$orXW1$77wy4q{--(IMSdscj7mxJl9htIz0_w= z-J8|xfciBWBvDH%M_NRwHFb8Ry(YCcxmHrrDbn}JzW#0f{X=)2AGD9X$qktQYpZAv zo%C`KS2OfV1G*wCynU#;JfPlWzcfnkVvdLuWxE|K%aF(gRzBdWCKm>cE)_7K_6_!d zc407cVX!y3O}5A;7Ob$HuX-ka5&vDXO)w;?D}n-%x(udL-N;C#!d_C%L#@(Y|*@B6#lU%HJ5}cb1XERdT5u!HBMn|8z}! zgROTW(YH!gr6jpd$7coa_DQOOr+T&Z{7g;xrd@r)CCh|)_rr;#w9VBmFmIo$*Supc zsd?vIgXS_Pns?2m79P>P;0esiS1?&cT4i6hoOMr4cwX7C|5xH6GNzPFTp)Q39vOk_)iyhnLnp`69w**`u(!U6wUx#X9D zxYsXS0{x(0_6o=k6?6G4DmgrZa-<*07kp$#^HVH-EI;W%o8$hY9+gIc6Vv0_5-4NY zD=c^4jCau+2mYi#<@rFI%6p{}bvm6bUS`#2iup@u3`qCq$IE^Rs0SwU)05sTn9dHK z%+7*zE?b=RJcxa8rZ9tS^kRXn&?5_uHT(R_K<>{@&jLVOP~u+CFJ1Nk9`vt*cBt%4 z0XUo|;}IAe*->Z>-0^G?z=?}KC60MzZ3guAihxi0uqg6VGnchlr?VxpohgnNJZhRR z6!O_Alt1TBlJKB+vBY+dUb#~6rtz?Mrd;r7kbN_kE&<-3yP7TK@Nl5y<#KG}!EC8S zy$=;9A@Q(3Q7q=!uSfl2&M)HOn2*NKLd5ZzGDV&!!E~&BY$lgOQ}CqfYN?Yx`k@T5 zr~Rp^S&;6}7d_~Xa;hvPpY=-DRJn5;lvm#Y$)l%FxMLH3(JSZ2T^%+TF)KD7^K)LQ zxWz?_Pt~Fi@7XmxIK1cfJGN}5t!A?Y_jvZv+1jkbyTCfUdkfk+IzEZ^?Df17fPH=u zda6D0?2vo4r}s}yWq~%0od$X=e+A9jKRxXcI_-}m#gIXJ&iN%AU5C*^ZyMcKbPtq# zOr%o<*y#pT@os_cqX%kp?x51Uc5GRY_u{zo0cg~Msr^0V9`Y?1{LdkuK>qK@zleMf zmV$plK8yU;CXp{A&m#XJ@@J6`VYB^b-W%_;O+3EDdTrVegTK6cj92Z zM|Q||tu+`g~I@Jmo`5Km^*DA9T9A=x~{E4%>@Gz>u!UT@G81RFNu4IbPGK6HwBk6!>t(fB_MEg!KJGqI98U?+vfNptIPNS_ z9P5wbZ~|V0d!3GCgdWYmuKT9L`}+KQGDtI1OrM^aM>_XdCMo2n(miOk$k*Z4rG0J^ z2mDA+{$2(yVOYVznQV=`R`}~*`^#6qeYe3R!Pj&) z&#LvXTLWbCCNKajU-DpdF4V~=RoDO`8=BA}B!c;k2I*~KZ`QvIr_%>v+^68qy&q@! z+hn7j;qOj#$n$J~WS9hD0|e2c2xe@G{0McVaQNq)s2jBy8at01R68YKah?z-#exe9 zBgw>E6;DMIV}cvHM(Cl8LV~a-$UlnQ)LO|&0{{E<(5H&h+QT=_US zpIj&M`3?Vody2gSQ#c`}eaqOBXj*~Nr z<7ABDsB09*X%fY8`HkXuDUIUTM^PM|j8Pn~5m8)g4ICH$YCbM<47A>>BX+SgkXkg8 z&?S^tWWtRZrfrH{i)mOFlr+4CBZU%cvG^KVo0jO(bZy4aQUPtfVqH!9l-7&ht{2LA zb);G*pW8A`kj&RdhQrDv%}O#HjR8lWO5UjHZJKTlWbiFD%MB=Yo2J_}9a4^sSl(7C zM=d>AvMaM1+H8a$-SDl|(pC?0*xC(WOTcIKSX#T`YYq5@RNk8_yO)dAn_ zioHYAcWJs?(>>sO8S`irhVxoyHMWIrY*+mC1;5Q=e!9#;-x|xTT}sWJx3bLM$kM@` z^4U!y8*sRfy+vx@C5hd@stLq#35vZ%#wuK9t#^pe3>N0|O1(cT@@mlcWU@u3_H7-X{98Z5y98V@u z9CyYjjyrV}$IDR^$BR%D$BR%D#}j81$M!^Vya+{cya+{cya+{cZMYOfaqTs59W`*B zHE>-uaBFJdx+6IHRv274f%F^rbdjzEH~1M37yT=)+!xXtLi#H86gi-z(Hi4hq6;Lg zorixux-<#ySiw35b^y1M!-4X9zU_;@cuaYXnr0(gf(Pkj6{V-2r4S+4M59 zoViN2hY)q?2qEf%PYVN1>cV%zrW|z{2_fn-5JJ=?6GGHwG=!+j-T*R}d%k0)Oyi5= zX~sa*g>%I~)CFF}0HQAZjK|QZ%k~hWF872Gb=em})MbANQI`V&#Cem3f6>UK$1e?^ zpM|K)`VgWn@WE*9^yjAOISU}_a&HJxmxCchT@Hm1b-6u+sLSC1;(SfR>1SlpahAr2 zvJiE-C4{I;UkFi`{t%)rbi~n0DO}alcoT0S>T)!MsLQbsqAte+h<@!fuGXepJ-nA` zyz@5@*9%w1kaAu$LQgmNnR&shzbo$v>Ct&%oGiBn(welLw}lYvwIjMoofNphYFnDr z=9@!^?K~MmZ0D&EqP`U=Z0G5a#&+6y!C9Whx2UED&dbV(<~&K`vr$W<&9{XRZ9W)6 z)a6VFQJ4Eeh`MA#h`Nl15OuNhf-6O3MDydoG(TC@wl}LTc3w2AU4}v$b>aIv_INX_ zUoC~Y*m=>c{rEu0NnPx`Fiw=S0nIqGc7zah*%?B#%dQZjUF?W%(LTB(q*0f1Aw*p& zQfL=DFIwQkPRoNKC++f30I?q{Bf3@X;)XQpvO9#T=S8dPLZ>k8(yDf`^P*Mf#rcp% zT`E$ji=AD@8T4?#$xqKKGl8SO0&(whdlUbLYGH~5+TfcsclE(DU=RRcR>+f|Ys zvF&ONJ7U|RePzUQ=c}y29V*4H!5u0k6V%Y5QtVjkP$_n-aj&e5l1`OkM@gqjv8!jN zO0lD)Q>EC|le=rBf4fwQ?cXkyV*9sCrPxu@rBZDFazC#0FFwcwWP4_fO0hk&My1%E zS))>H&vYv%*Ql4VD?EZl=4mH`_ZjxLUKT?x_5k~P6ZaeBAvh*W{v)=#6uOpO&834O ziHf--ax3AHV3oqm7dz{e{8LKa!26q&lUO2+dR~9xcmM6jCnnmy@av7Tb<|y*L%N(&O!L6p8oB4uH$nYVhh9;h%FFXAhtkkf!G4E1!4=t7Kkkn zTOhVTY=K1KpMmVX?P{(z?tdH4S_#A<&4eF+S_!>2bjt1QBl$q>zZQ3-kZx?|B_ z=`O+SSc2IpEhx>}>4~6Z{LSs;H2=(ZT7|}r1lS}Fu?1oa#1@Dx5L+O&Kx~270 z&+kH9gUGu+{Q+wc`M*K=Q5-*oTaU=k-gx8Y$7wDi>3zui5jP^N32#HNBGNfEk19A@cW)HxSze}YRfNv z%VA$y+8K5Rz)oI{*HH(EbGJk`mAEQOCt`LB(rgZVcaE7NE9|DgT1RMr3A!Bl4j literal 0 HcmV?d00001 diff --git a/tests/PhpSpreadsheetTests/DefinedNameFormulaTest.php b/tests/PhpSpreadsheetTests/DefinedNameFormulaTest.php index 50304991..14843091 100644 --- a/tests/PhpSpreadsheetTests/DefinedNameFormulaTest.php +++ b/tests/PhpSpreadsheetTests/DefinedNameFormulaTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests; use PhpOffice\PhpSpreadsheet\DefinedName; +use PhpOffice\PhpSpreadsheet\NamedFormula; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PHPUnit\Framework\TestCase; @@ -164,4 +165,24 @@ class DefinedNameFormulaTest extends TestCase 'utf-8 named ranges in a formula' => ['Здравствуй+мир', true], ]; } + + public function testEmptyNamedFormula(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + $spreadSheet = new Spreadsheet(); + $workSheet1 = $spreadSheet->getActiveSheet(); + new NamedFormula('namedformula', $workSheet1); + } + + public function testChangeFormula(): void + { + $spreadSheet = new Spreadsheet(); + $workSheet1 = $spreadSheet->getActiveSheet(); + $namedFormula = new NamedFormula('namedformula', $workSheet1, '=1'); + self::assertEquals('=1', $namedFormula->getFormula()); + $namedFormula->setFormula('=2'); + self::assertEquals('=2', $namedFormula->getFormula()); + $namedFormula->setFormula(''); + self::assertEquals('=2', $namedFormula->getFormula()); + } } diff --git a/tests/PhpSpreadsheetTests/DefinedNameTest.php b/tests/PhpSpreadsheetTests/DefinedNameTest.php index 3f5b75a0..c199e7f2 100644 --- a/tests/PhpSpreadsheetTests/DefinedNameTest.php +++ b/tests/PhpSpreadsheetTests/DefinedNameTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests; use PhpOffice\PhpSpreadsheet\DefinedName; +use PhpOffice\PhpSpreadsheet\NamedRange; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; use PHPUnit\Framework\TestCase; @@ -121,4 +122,85 @@ class DefinedNameTest extends TestCase $this->spreadsheet->getDefinedName('foo')->getValue() ); } + + public function testDefinedNameNoWorksheetNoScope(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class); + new NamedRange('xyz'); + } + + public function testSetAndGetRange(): void + { + $this->spreadsheet->addDefinedName( + DefinedName::createInstance('xyz', $this->spreadsheet->getActiveSheet(), 'A1') + ); + + $namedRange = $this->spreadsheet->getDefinedName('XYZ'); + self::assertInstanceOf(NamedRange::class, $namedRange); + if ($namedRange instanceof NamedRange) { + self::assertEquals('A1', $namedRange->getRange()); + self::assertEquals('A1', $namedRange->getValue()); + $namedRange->setRange('A2'); + self::assertEquals('A2', $namedRange->getValue()); + } + } + + public function testChangeWorksheet(): void + { + $sheet1 = $this->spreadsheet->getSheetByName('Sheet #1'); + $sheet2 = $this->spreadsheet->getSheetByName('Sheet #2'); + $sheet1->getCell('A1')->setValue(1); + $sheet2->getCell('A1')->setValue(2); + $namedRange = new NamedRange('xyz', $sheet2, '$A$1'); + $namedRange->setWorksheet($sheet1); + $this->spreadsheet->addNamedRange($namedRange); + $sheet1->getCell('B2')->setValue('=XYZ'); + self::assertEquals(1, $sheet1->getCell('B2')->getCalculatedValue()); + $sheet2->getCell('B2')->setValue('=XYZ'); + self::assertEquals(1, $sheet2->getCell('B2')->getCalculatedValue()); + } + + public function testLocalOnly(): void + { + $sheet1 = $this->spreadsheet->getSheetByName('Sheet #1'); + $sheet2 = $this->spreadsheet->getSheetByName('Sheet #2'); + $sheet1->getCell('A1')->setValue(1); + $sheet2->getCell('A1')->setValue(2); + $namedRange = new NamedRange('abc', $sheet2, '$A$1'); + $namedRange->setWorksheet($sheet1)->setLocalOnly(true); + $this->spreadsheet->addNamedRange($namedRange); + $sheet1->getCell('C2')->setValue('=ABC'); + self::assertEquals(1, $sheet1->getCell('C2')->getCalculatedValue()); + $sheet2->getCell('C2')->setValue('=ABC'); + self::assertEquals('#NAME?', $sheet2->getCell('C2')->getCalculatedValue()); + } + + public function testScope(): void + { + $sheet1 = $this->spreadsheet->getSheetByName('Sheet #1'); + $sheet2 = $this->spreadsheet->getSheetByName('Sheet #2'); + $sheet1->getCell('A1')->setValue(1); + $sheet2->getCell('A1')->setValue(2); + $namedRange = new NamedRange('abc', $sheet2, '$A$1'); + $namedRange->setScope($sheet1); + $this->spreadsheet->addNamedRange($namedRange); + $sheet1->getCell('C2')->setValue('=ABC'); + self::assertEquals(2, $sheet1->getCell('C2')->getCalculatedValue()); + $sheet2->getCell('C2')->setValue('=ABC'); + self::assertEquals('#NAME?', $sheet2->getCell('C2')->getCalculatedValue()); + } + + public function testClone(): void + { + $sheet1 = $this->spreadsheet->getSheetByName('Sheet #1'); + $sheet2 = $this->spreadsheet->getSheetByName('Sheet #2'); + $sheet1->getCell('A1')->setValue(1); + $sheet2->getCell('A1')->setValue(2); + $namedRange = new NamedRange('abc', $sheet2, '$A$1'); + $namedRangeClone = clone $namedRange; + $ss1 = $namedRange->getWorksheet(); + $ss2 = $namedRangeClone->getWorksheet(); + self::assertNotSame($ss1, $ss2); + self::assertEquals($ss1->getTitle(), $ss2->getTitle()); + } } diff --git a/tests/PhpSpreadsheetTests/DocumentGeneratorTest.php b/tests/PhpSpreadsheetTests/DocumentGeneratorTest.php index 8a34f1c0..45000f8e 100644 --- a/tests/PhpSpreadsheetTests/DocumentGeneratorTest.php +++ b/tests/PhpSpreadsheetTests/DocumentGeneratorTest.php @@ -7,6 +7,7 @@ use PhpOffice\PhpSpreadsheet\Calculation\Functions; use PhpOffice\PhpSpreadsheet\Calculation\Logical; use PhpOffice\PhpSpreadsheet\DocumentGenerator; use PHPUnit\Framework\TestCase; +use UnexpectedValueException; class DocumentGeneratorTest extends TestCase { @@ -137,4 +138,13 @@ EXPECTED ], ]; } + + public function testGenerateFunctionBadArray(): void + { + $this->expectException(UnexpectedValueException::class); + $phpSpreadsheetFunctions = [ + 'ABS' => ['category' => Cat::CATEGORY_MATH_AND_TRIG, 'functionCall' => 1], + ]; + DocumentGenerator::generateFunctionListByName($phpSpreadsheetFunctions); + } } diff --git a/tests/PhpSpreadsheetTests/Functional/CommentsTest.php b/tests/PhpSpreadsheetTests/Functional/CommentsTest.php index 6f1c5340..5ba4e7c8 100644 --- a/tests/PhpSpreadsheetTests/Functional/CommentsTest.php +++ b/tests/PhpSpreadsheetTests/Functional/CommentsTest.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Functional; use PhpOffice\PhpSpreadsheet\Spreadsheet; +use PhpOffice\PhpSpreadsheet\Style\Alignment; class CommentsTest extends AbstractFunctional { @@ -35,10 +36,22 @@ class CommentsTest extends AbstractFunctional $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $format); - $commentsLoaded = $reloadedSpreadsheet->getSheet(0)->getComments(); + $sheet = $reloadedSpreadsheet->getSheet(0); + $commentsLoaded = $sheet->getComments(); self::assertCount(1, $commentsLoaded); $commentCoordinate = key($commentsLoaded); self::assertSame('E10', $commentCoordinate); + $comment = $commentsLoaded[$commentCoordinate]; + self::assertEquals('Comment to test', (string) $comment); + $commentClone = clone $comment; + self::assertEquals($comment, $commentClone); + self::assertNotSame($comment, $commentClone); + if ($format === 'Xlsx') { + self::assertEquals('feb0c24b880a8130262dadf801f85e94', $comment->getHashCode()); + self::assertEquals(Alignment::HORIZONTAL_GENERAL, $comment->getAlignment()); + $comment->setAlignment(Alignment::HORIZONTAL_RIGHT); + self::assertEquals(Alignment::HORIZONTAL_RIGHT, $comment->getAlignment()); + } } } diff --git a/tests/PhpSpreadsheetTests/IOFactoryTest.php b/tests/PhpSpreadsheetTests/IOFactoryTest.php index f9512a34..9b07ad33 100644 --- a/tests/PhpSpreadsheetTests/IOFactoryTest.php +++ b/tests/PhpSpreadsheetTests/IOFactoryTest.php @@ -161,4 +161,45 @@ class IOFactoryTest extends TestCase IOFactory::registerReader('foo', 'bar'); } + + public function testCreateInvalidWriter(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Writer\Exception::class); + $spreadsheet = new Spreadsheet(); + IOFactory::createWriter($spreadsheet, 'bad'); + } + + public function testCreateInvalidReader(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Reader\Exception::class); + IOFactory::createReader('bad'); + } + + public function testCreateReaderUnknownExtension(): void + { + $filename = 'samples/Reader/sampleData/example1.tsv'; + $reader = IOFactory::createReaderForFile($filename); + self::assertEquals('PhpOffice\\PhpSpreadsheet\\Reader\\Csv', get_class($reader)); + } + + public function testCreateReaderCsvExtension(): void + { + $filename = 'samples/Reader/sampleData/example1.csv'; + $reader = IOFactory::createReaderForFile($filename); + self::assertEquals('PhpOffice\\PhpSpreadsheet\\Reader\\Csv', get_class($reader)); + } + + public function testCreateReaderNoExtension(): void + { + $filename = 'samples/Reader/sampleData/example1xls'; + $reader = IOFactory::createReaderForFile($filename); + self::assertEquals('PhpOffice\\PhpSpreadsheet\\Reader\\Xls', get_class($reader)); + } + + public function testCreateReaderNotSpreadsheet(): void + { + $this->expectException(\PhpOffice\PhpSpreadsheet\Reader\Exception::class); + $filename = __FILE__; + $reader = IOFactory::createReaderForFile($filename); + } } From 6b4feb6142e3422554ae058c4983dccfc3a555fc Mon Sep 17 00:00:00 2001 From: Owen Leibman Date: Fri, 27 Nov 2020 07:16:23 -0800 Subject: [PATCH 2/2] Changes for Scrutinizer Two changes to fix minor problems reported by Scrutinizer. --- tests/PhpSpreadsheetTests/DefinedNameTest.php | 10 ++++------ tests/PhpSpreadsheetTests/IOFactoryTest.php | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/PhpSpreadsheetTests/DefinedNameTest.php b/tests/PhpSpreadsheetTests/DefinedNameTest.php index c199e7f2..8a411775 100644 --- a/tests/PhpSpreadsheetTests/DefinedNameTest.php +++ b/tests/PhpSpreadsheetTests/DefinedNameTest.php @@ -137,12 +137,10 @@ class DefinedNameTest extends TestCase $namedRange = $this->spreadsheet->getDefinedName('XYZ'); self::assertInstanceOf(NamedRange::class, $namedRange); - if ($namedRange instanceof NamedRange) { - self::assertEquals('A1', $namedRange->getRange()); - self::assertEquals('A1', $namedRange->getValue()); - $namedRange->setRange('A2'); - self::assertEquals('A2', $namedRange->getValue()); - } + self::assertEquals('A1', $namedRange->getRange()); + self::assertEquals('A1', $namedRange->getValue()); + $namedRange->setRange('A2'); + self::assertEquals('A2', $namedRange->getValue()); } public function testChangeWorksheet(): void diff --git a/tests/PhpSpreadsheetTests/IOFactoryTest.php b/tests/PhpSpreadsheetTests/IOFactoryTest.php index 9b07ad33..886fcb36 100644 --- a/tests/PhpSpreadsheetTests/IOFactoryTest.php +++ b/tests/PhpSpreadsheetTests/IOFactoryTest.php @@ -200,6 +200,6 @@ class IOFactoryTest extends TestCase { $this->expectException(\PhpOffice\PhpSpreadsheet\Reader\Exception::class); $filename = __FILE__; - $reader = IOFactory::createReaderForFile($filename); + IOFactory::createReaderForFile($filename); } }