Skip to content

Commit d464b88

Browse files
PowerKiKiPowerKiKi
committed
Assert zip adapters return values
Unit tests did not properly tests that both adapters had the exact same behavior for returned values. By using common code for testing we can assert that the behavior is now in fact identical. Also phpdocs were moved to the interface itself instead of each adapters.
1 parent c9be70c commit d464b88

File tree

6 files changed

+92
-116
lines changed

6 files changed

+92
-116
lines changed

‎src/Common/Adapter/Zip/PclZipAdapter.php‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,31 +15,18 @@ class PclZipAdapter implements ZipInterface
1515
*/
1616
protected $tmpDir;
1717

18-
/**
19-
* @param $filename
20-
* @return $this
21-
*/
2218
public function open($filename)
2319
{
2420
$this->oPclZip = new PclZip($filename);
2521
$this->tmpDir = sys_get_temp_dir();
2622
return $this;
2723
}
2824

29-
/**
30-
* @return $this
31-
*/
3225
public function close()
3326
{
3427
return $this;
3528
}
3629

37-
/**
38-
* @param $localname
39-
* @param $contents
40-
* @return $this
41-
* @throws \Exception
42-
*/
4330
public function addFromString($localname, $contents)
4431
{
4532
$pathData = pathinfo($localname);

‎src/Common/Adapter/Zip/ZipArchiveAdapter.php‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,6 @@ class ZipArchiveAdapter implements ZipInterface
1616
*/
1717
protected $filename;
1818

19-
/**
20-
* @param string $filename
21-
* @throws \Exception Could not open $this->filename for writing.
22-
* @return mixed
23-
*/
2419
public function open($filename)
2520
{
2621
$this->filename = $filename;
@@ -35,10 +30,6 @@ public function open($filename)
3530
throw new \Exception("Could not open $this->filename for writing.");
3631
}
3732

38-
/**
39-
* @return $this
40-
* @throws \Exception Could not close zip file $this->filename.
41-
*/
4233
public function close()
4334
{
4435
if ($this->oZipArchive->close() === false) {
@@ -47,13 +38,12 @@ public function close()
4738
return $this;
4839
}
4940

50-
/**
51-
* @param $localname
52-
* @param $contents
53-
* @return bool
54-
*/
5541
public function addFromString($localname, $contents)
5642
{
57-
return $this->oZipArchive->addFromString($localname, $contents);
43+
if ($this->oZipArchive->addFromString($localname, $contents) === false) {
44+
throw new \Exception("Error zipping files : " . $localname);
45+
}
46+
47+
return $this;
5848
}
5949
}

‎src/Common/Adapter/Zip/ZipInterface.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,27 @@
44

55
interface ZipInterface
66
{
7+
/**
8+
* Open a ZIP file archive
9+
* @param string $filename
10+
* @return $this
11+
* @throws \Exception
12+
*/
713
public function open($filename);
14+
15+
/**
16+
* Close the active archive (opened or newly created)
17+
* @return $this
18+
* @throws \Exception
19+
*/
820
public function close();
21+
22+
/**
23+
* Add a file to a ZIP archive using its contents
24+
* @param string $localname The name of the entry to create.
25+
* @param string $contents The contents to use to create the entry. It is used in a binary safe mode.
26+
* @return $this
27+
* @throws \Exception
28+
*/
929
public function addFromString($localname, $contents);
1030
}

‎tests/Common/Tests/Adapter/Zip/AbstractZipAdapterTest.php‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php
2+
3+
namespace Common\Tests\Adapter\Zip;
4+
5+
use PhpOffice\Common\Tests\TestHelperZip;
6+
7+
abstract class AbstractZipAdapterTest extends \PHPUnit_Framework_TestCase
8+
{
9+
protected $zipTest;
10+
11+
/**
12+
* Returns a new instance of the adapter to test
13+
* @return \PhpOffice\Common\Adapter\Zip\ZipInterface
14+
*/
15+
abstract protected function createAdapter();
16+
17+
public function setUp()
18+
{
19+
parent::setUp();
20+
21+
$pathResources = PHPOFFICE_COMMON_TESTS_BASE_DIR.DIRECTORY_SEPARATOR.'resources'.DIRECTORY_SEPARATOR.'files'.DIRECTORY_SEPARATOR;
22+
$this->zipTest = tempnam(sys_get_temp_dir(), 'PhpOfficeCommon');
23+
copy($pathResources.'Sample_01_Simple.pptx', $this->zipTest);
24+
}
25+
26+
public function tearDown()
27+
{
28+
parent::tearDown();
29+
30+
if (is_file($this->zipTest)) {
31+
unlink($this->zipTest);
32+
}
33+
}
34+
35+
public function testOpen()
36+
{
37+
$adapter = $this->createAdapter();
38+
$this->assertSame($adapter, $adapter->open($this->zipTest));
39+
}
40+
41+
public function testClose()
42+
{
43+
$adapter = $this->createAdapter();
44+
$adapter->open($this->zipTest);
45+
$this->assertSame($adapter, $adapter->close());
46+
}
47+
48+
public function testAddFromString()
49+
{
50+
$expectedPath = 'file.test';
51+
$expectedContent = 'Content';
52+
53+
$adapter = $this->createAdapter();
54+
$adapter->open($this->zipTest);
55+
$this->assertSame($adapter, $adapter->addFromString($expectedPath, $expectedContent));
56+
$adapter->close();
57+
58+
$this->assertTrue(TestHelperZip::assertFileExists($this->zipTest, $expectedPath));
59+
$this->assertTrue(TestHelperZip::assertFileContent($this->zipTest, $expectedPath, $expectedContent));
60+
}
61+
}

‎tests/Common/Tests/Adapter/Zip/PclZipAdapterTest.php‎

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -5,50 +5,10 @@
55
use PhpOffice\Common\Adapter\Zip\PclZipAdapter;
66
use PhpOffice\Common\Tests\TestHelperZip;
77

8-
class PclZipAdapterTest extends \PHPUnit_Framework_TestCase
8+
class PclZipAdapterTest extends AbstractZipAdapterTest
99
{
10-
protected $zipTest;
11-
12-
public function setUp()
13-
{
14-
parent::setUp();
15-
16-
$pathResources = PHPOFFICE_COMMON_TESTS_BASE_DIR.DIRECTORY_SEPARATOR.'resources'.DIRECTORY_SEPARATOR.'files'.DIRECTORY_SEPARATOR;
17-
$this->zipTest = tempnam($pathResources, 'PhpOfficeCommon');
18-
copy($pathResources.'Sample_01_Simple.pptx', $this->zipTest);
19-
}
20-
21-
public function tearDown()
22-
{
23-
parent::tearDown();
24-
25-
unlink($this->zipTest);
26-
}
27-
28-
public function testOpen()
10+
protected function createAdapter()
2911
{
30-
$object = new PclZipAdapter();
31-
$this->assertInstanceOf('PhpOffice\\Common\\Adapter\\Zip\\ZipInterface', $object->open($this->zipTest));
32-
}
33-
34-
public function testClose()
35-
{
36-
$object = new PclZipAdapter();
37-
$object->open($this->zipTest);
38-
$this->assertInstanceOf('PhpOffice\\Common\\Adapter\\Zip\\ZipInterface', $object->close());
39-
}
40-
41-
public function testAddFromString()
42-
{
43-
$expectedPath = 'file.test';
44-
$expectedContent = 'Content';
45-
46-
$object = new PclZipAdapter();
47-
$object->open($this->zipTest);
48-
$object->addFromString($expectedPath, $expectedContent);
49-
$object->close();
50-
51-
$this->assertTrue(TestHelperZip::assertFileExists($this->zipTest, $expectedPath));
52-
$this->assertTrue(TestHelperZip::assertFileContent($this->zipTest, $expectedPath, $expectedContent));
12+
return new PclZipAdapter();
5313
}
5414
}

‎tests/Common/Tests/Adapter/Zip/ZipArchiveAdapterTest.php‎

Lines changed: 3 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,52 +5,10 @@
55
use PhpOffice\Common\Adapter\Zip\ZipArchiveAdapter;
66
use PhpOffice\Common\Tests\TestHelperZip;
77

8-
class ZipArchiveAdapterTest extends \PHPUnit_Framework_TestCase
8+
class ZipArchiveAdapterTest extends AbstractZipAdapterTest
99
{
10-
protected $zipTest;
11-
12-
public function setUp()
13-
{
14-
parent::setUp();
15-
16-
$pathResources = PHPOFFICE_COMMON_TESTS_BASE_DIR.DIRECTORY_SEPARATOR.'resources'.DIRECTORY_SEPARATOR.'files'.DIRECTORY_SEPARATOR;
17-
$this->zipTest = tempnam($pathResources, 'PhpOfficeCommon');
18-
copy($pathResources.'Sample_01_Simple.pptx', $this->zipTest);
19-
}
20-
21-
public function tearDown()
22-
{
23-
parent::tearDown();
24-
25-
if (is_file($this->zipTest)) {
26-
unlink($this->zipTest);
27-
}
28-
}
29-
30-
public function testOpen()
10+
protected function createAdapter()
3111
{
32-
$object = new ZipArchiveAdapter();
33-
$this->assertInstanceOf('PhpOffice\\Common\\Adapter\\Zip\\ZipInterface', $object->open($this->zipTest));
34-
}
35-
36-
public function testClose()
37-
{
38-
$object = new ZipArchiveAdapter();
39-
$object->open($this->zipTest);
40-
$this->assertInstanceOf('PhpOffice\\Common\\Adapter\\Zip\\ZipInterface', $object->close());
41-
}
42-
43-
public function testAddFromString()
44-
{
45-
$expectedPath = 'file.test';
46-
$expectedContent = 'Content';
47-
48-
$object = new ZipArchiveAdapter();
49-
$object->open($this->zipTest);
50-
$object->addFromString($expectedPath, $expectedContent);
51-
$object->close();
52-
53-
$this->assertTrue(TestHelperZip::assertFileExists($this->zipTest, $expectedPath));
54-
$this->assertTrue(TestHelperZip::assertFileContent($this->zipTest, $expectedPath, $expectedContent));
12+
return new ZipArchiveAdapter();
5513
}
5614
}

0 commit comments

Comments
 (0)