Few tests JDocument package #1843

Merged
merged 5 commits into from Mar 16, 2013

Projects

None yet

3 participants

@piotr-cz
piotr-cz commented Mar 5, 2013

JDocumentImage
JDocumentXml
JDocumentHtml (8 methods)

This isn't much, but these are my first unit tests.

@dongilbert

Looks good. All the tests are passing on my end.

@piotr-cz
piotr-cz commented Mar 5, 2013

Pleas take a look at the code quality, as these are the first test I've ever written.

I was going to write one more for JDocumentHtml::mergeHeadData() but I'm confused, as this method is using internal methods (setMetaData, addStyleDeclaration) whilte setMetadata is setting values directly.

Is this how it's supposed to work?

@dongilbert

OK - I'll take another look when I have time to give it a proper review of quality as well as functionality.

Thanks for doing this.

@elinw elinw commented on an outdated diff Mar 10, 2013
...uites/unit/joomla/document/html/JDocumentHTMLTest.php
{
- // Remove the following lines when you implement this test.
- $this->markTestIncomplete(
- 'This test has not been implemented yet.'
+ }
+
+ /**
+ * Terst construct
@elinw elinw commented on the diff Mar 10, 2013
...tes/unit/joomla/document/image/JDocumentImageTest.php
+ $this->equalTo('image'),
+ 'JDocumentImage::__construct: Default Type does not match'
+ );
+ }
+
+ /**
+ * Test...
+ *
+ * @covers JDocumentImage::render
+ *
+ * @return void
+ */
+ public function testRender()
+ {
+ JResponse::clearHeaders();
+
@elinw
elinw Mar 10, 2013

What about doing this as a data provider. That way if one fails the rest of the tests still run. Also I usually put a message so it's easier to figure out which test is the one that failed. It isn't super important but it makes php unit easier to use.

@piotr-cz
piotr-cz Mar 15, 2013

I'll need your guidance on data provider.

@dongilbert dongilbert merged commit 72cecd0 into joomla:staging Mar 16, 2013
@piotr-cz piotr-cz deleted the unknown repository branch Apr 12, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment