Issues

ZF-2334: Promote Zend_Service_Technorati to core for release

Issue Type: Task Created: 2007-12-19T13:21:01.000+0000 Last Updated: 2008-03-21T16:25:16.000+0000 Status: Resolved Fix version(s): - 1.5.0 (17/Mar/08)

Reporter: Darby Felton (darby) Assignee: Darby Felton (darby) Tags: - Zend_Service_Technorati

Related issues: Attachments: - Zend_Service_Technorati_AllTests.out

Description

Once unit testing for each file of the component reaches 80% LOC coverage and there exists satisfactory DocBook documentation for the APIs, the Zend_Service_Technorati component needs to be promoted to core (i.e., moved to trunk with unit tests and documentation integrated).

Comments

Posted by Simone Carletti (weppos) on 2007-12-23T19:32:34.000+0000

r7247 includes 100% of Technorati API support and 90% of unit tests.

Next step is to reach 99% of unit tests. It shouldn't be hard, all classes have been stressed by custom tests thus I only need to validate them, no more changes should be required.

Before starting the documentation it would be great to receive an overall feedback about the final product. :)

Posted by Simone Carletti (weppos) on 2007-12-25T17:31:42.000+0000

@ Darby

With r7263, I feel the library is ready. I've included 99% of unit tests and full documentation.

What do you think? :)

Posted by Darby Felton (darby) on 2007-12-26T07:41:13.000+0000

Great! I'll review this today and post again here. Thanks for these contributions! :)

Posted by Darby Felton (darby) on 2007-12-27T16:05:55.000+0000

Sorry, Simone, for the delay in getting back with you... I haven't forgotten! :) I plan to take a look first thing tomorrow and post again here.

Posted by Darby Felton (darby) on 2007-12-28T08:23:31.000+0000

I get a fatal error when running the unit tests on r7284 with PHP 5.1.4 (WinXP):

<pre class="literal">
$ phpunit --verbose --report Zend_Service_Technorati_AllTests Zend_Service_Technorati_AllTests > Zend_Service_Technorati_AllTests.out 2>&1 &



<pre class="literal">
$ tail -f Zend_Service_Technorati_AllTests.out
PHPUnit 3.2.8 by Sebastian Bergmann.

Zend Framework - Zend_Service_Technorati
 Zend_Service_Technorati_TechnoratiTest
 ......................
Fatal error: Wrong parameters for Exception([string $exception [, long $code ]]) in trunk\incubator\library\Zend\Service\Technorati\Utils.php on line 67

Call Stack:
    0.0054   1. {main}() c:\wamp\php\PEAR\PHPUnit\TextUI\Command.php:0
    1.8072   2. PHPUnit_TextUI_Command::main() c:\wamp\php\PEAR\PHPUnit\TextUI\Command.php:528
    3.0258   3. PHPUnit_TextUI_TestRunner->doRun() c:\wamp\php\PEAR\PHPUnit\TextUI\Command.php:112
    3.0294   4. PHPUnit_Framework_TestSuite->run() C:\wamp\php\PEAR\PHPUnit\TextUI\TestRunner.php:298
    3.0322   5. PHPUnit_Framework_TestSuite->run() C:\wamp\php\PEAR\PHPUnit\Framework\TestSuite.php:633
   17.7259   6. PHPUnit_Framework_TestSuite->runTest() C:\wamp\php\PEAR\PHPUnit\Framework\TestSuite.php:650
   17.7259   7. PHPUnit_Framework_TestCase->run() C:\wamp\php\PEAR\PHPUnit\Framework\TestSuite.php:670
   17.7259   8. PHPUnit_Framework_TestResult->run() C:\wamp\php\PEAR\PHPUnit\Framework\TestCase.php:351
   17.7288   9. PHPUnit_Framework_TestCase->runBare() C:\wamp\php\PEAR\PHPUnit\Framework\TestResult.php:597
   17.8634  10. PHPUnit_Framework_TestCase->runTest() C:\wamp\php\PEAR\PHPUnit\Framework\TestCase.php:376
   17.8657  11. ReflectionMethod->invoke() C:\wamp\php\PEAR\PHPUnit\Framework\TestCase.php:449
   17.8658  12. Zend_Service_Technorati_TechnoratiTest->testBlogInfo() trunk\incubator\tests\Zend\Service\Technorati\TechnoratiTest.php:0
   17.8854  13. Zend_Service_Technorati->blogInfo() trunk\incubator\tests\Zend\Service\Technorati\TechnoratiTest.php:327
   18.0550  14. Zend_Service_Technorati_BlogInfoResult->__construct() trunk\incubator\library\Zend\Service\Technorati.php:371
   18.1702  15. Zend_Service_Technorati_Utils::setUriHttp() trunk\incubator\library\Zend\Service\Technorati\BlogInfoResult.php:106
   18.1927  16. Exception->__construct() trunk\incubator\library\Zend\Service\Technorati\Utils.php:67

Posted by Simone Carletti (weppos) on 2007-12-28T08:47:53.000+0000

On my Mac it works. :|

<pre class="highlight">
weppos-mac:~/Sites/svn/zend.com.framework/trunk/incubator/tests weppos$ phpunit --verbose Zend_Service_Technorati_AllTests
PHPUnit 3.2.5 by Sebastian Bergmann.

Zend Framework - Zend_Service_Technorati
 Zend_Service_Technorati_TechnoratiTest
 ..........................................

 Zend_Service_Technorati_AuthorTest
 ....

 Zend_Service_Technorati_WeblogTest
 .....

 Zend_Service_Technorati_BlogInfoResultTest
 ....

 Zend_Service_Technorati_GetInfoResultTest
 ...

 Zend_Service_Technorati_KeyInfoResultTest
 .....

 Zend_Service_Technorati_CosmosResultTest
 .......

 Zend_Service_Technorati_CosmosResultSetTest
 .........

 Zend_Service_Technorati_DailyCountsResultTest
 ...

 Zend_Service_Technorati_DailyCountsResultSetTest
 ...

 Zend_Service_Technorati_SearchResultTest
 ....

 Zend_Service_Technorati_SearchResultSetTest
 ...

 Zend_Service_Technorati_TagResultTest
 ...

 Zend_Service_Technorati_TagResultSetTest
 ...

 Zend_Service_Technorati_TagsResultTest
 ....

 Zend_Service_Technorati_TagsResultSetTest
 ...


Time: 1 second


OK (105 tests)

I'm going to investigate the error. First I'll upgrade my phpunit installation, I see you have 3.2.8.

I'll be back in a moment...

Posted by Simone Carletti (weppos) on 2007-12-28T08:54:12.000+0000

I've just committed an update, it should be fixed now. Could you please let me know your PHP version?

Posted by Darby Felton (darby) on 2007-12-28T09:12:50.000+0000

The PHP version I noted above as 5.1.4.

With r7285 I now get the following fatal error:

<pre class="literal">
PHPUnit 3.2.8 by Sebastian Bergmann.

Zend Framework - Zend_Service_Technorati
 Zend_Service_Technorati_TechnoratiTest
 ..........................................

 Zend_Service_Technorati_AuthorTest
 .
Fatal error: Argument 1 passed to Zend_Service_Technorati_Author::__construct() must be an object of class DomElement,
called in trunk\incubator\tests\Zend\Service\Technorati\AuthorTest.php on line 63 and defined in 
trunk\incubator\library\Zend\Service\Technorati\Author.php on line 95

Call Stack:
    0.0042   1. {main}() c:\wamp\php\PEAR\PHPUnit\TextUI\Command.php:0
    1.9327   2. PHPUnit_TextUI_Command::main() c:\wamp\php\PEAR\PHPUnit\TextUI\Command.php:528
    3.2590   3. PHPUnit_TextUI_TestRunner->doRun() c:\wamp\php\PEAR\PHPUnit\TextUI\Command.php:112
    3.2601   4. PHPUnit_Framework_TestSuite->run() C:\wamp\php\PEAR\PHPUnit\TextUI\TestRunner.php:298
   25.3584   5. PHPUnit_Framework_TestSuite->run() C:\wamp\php\PEAR\PHPUnit\Framework\TestSuite.php:633
   25.4709   6. PHPUnit_Framework_TestSuite->runTest() C:\wamp\php\PEAR\PHPUnit\Framework\TestSuite.php:650
   25.4710   7. PHPUnit_Framework_TestCase->run() C:\wamp\php\PEAR\PHPUnit\Framework\TestSuite.php:670
   25.4710   8. PHPUnit_Framework_TestResult->run() C:\wamp\php\PEAR\PHPUnit\Framework\TestCase.php:351
   25.4731   9. PHPUnit_Framework_TestCase->runBare() C:\wamp\php\PEAR\PHPUnit\Framework\TestResult.php:597
   25.5085  10. PHPUnit_Framework_TestCase->runTest() C:\wamp\php\PEAR\PHPUnit\Framework\TestCase.php:376
   25.5105  11. ReflectionMethod->invoke() C:\wamp\php\PEAR\PHPUnit\Framework\TestCase.php:449
   25.5105  12. Zend_Service_Technorati_AuthorTest->testConstructThrowsExceptionWithInvalidDom() trunk\incubator\tests\Zend\Service\Technorati\AuthorTest.php:0
   25.5121  13. Zend_Service_Technorati_Author->__construct() trunk\incubator\tests\Zend\Service\Technorati\AuthorTest.php:63

Posted by Simone Carletti (weppos) on 2007-12-28T09:25:19.000+0000

Mmm, this should be a problem.

The PHP version I noted above as 5.1.4.

Yes, I saw the version when the message was already posted and I haven't permission to edit my comments.

About the error, this is a strange behavior:

<pre class="highlight">
    public function testConstructThrowsExceptionWithInvalidDom() 
    {
        try {
            $object = new Zend_Service_Technorati_Author('foo');
            $this->fail('Expected Zend_Service_Technorati_Exception not thrown');
        } catch (Exception $e) {
            $this->assertContains("DOMElement", $e->getMessage());
        }
    }

As you can see, I test whether the method returns an exception if the constructor is called with an invalid argument. The error should be catched by try/catch block, and in fact it is in my environment.

Do you have an idea why PHP 5.1.4 isn't aware of try/catch block here?

Posted by Darby Felton (darby) on 2007-12-28T09:33:33.000+0000

I haven't pored over the code yet, but it seems that this fatal error is not the result of an uncaught exception. Instead, a failure to meet a type hint in PHP < 5.2.0, I believe, results in a non-catchable fatal error.

Posted by Simone Carletti (weppos) on 2007-12-28T09:40:39.000+0000

The only alternative I can see right now is to skip all these tests whether PHP < 5.2.0. This one is not the only test passing an invalid constructor but almost each class is tested against an invalid call.

What do you think?

Posted by Darby Felton (darby) on 2007-12-28T09:44:52.000+0000

Yes, this is what was done with other unit tests of the same type (e.g., see Zend_Db_Adapter_StaticTest and look for PHP_VERSION).

Posted by Darby Felton (darby) on 2007-12-28T09:59:28.000+0000

Now that I look into it more, it seems that I have based my assertion that type hinting failure results in non-catchable fatal errors on another person's incomplete work. I recommend that if you want to test failure to satisfy a type hint, try using a custom error handler.

Posted by Darby Felton (darby) on 2007-12-28T10:16:43.000+0000

Okay, maybe I should not have second-guessed myself. :) Or am I now doubting my doubts? ;) I ran the following script on my machine:

<pre class="highlight">
<?php

error_reporting(E_ALL | E_STRICT);

set_error_handler('errorHandler');

function errorHandler($errno, $errstr, $errfile, $errline, $errcontext)
{
    echo 'errorHandler() called';
    throw new Exception($errstr, $errno);
}

function foo(stdClass $obj)
{
    echo 'foo';
}

try {
    foo('scalar');
} catch (Exception $e) {
    echo 'Caught exception';
}

Results:

Fatal error: Argument 1 passed to foo() must be an object of class stdClass, called in C:\wamp\www\test.php on line 19 and defined in C:\wamp\www\test.php on line 13

I'm pretty sure that this means that at least on my machine running 5.1.4, the failure to meet the type hint does not result in a catchable fatal error.

Posted by Simone Carletti (weppos) on 2007-12-28T10:19:19.000+0000

I've just updated unit tests. Now "invalid argument" tests are skipped if PHP < 5.2.0.

I'm going to install and configure PHP 5.1.4 in the next few days to be able to develop with this specific release.

Posted by Darby Felton (darby) on 2007-12-28T12:13:59.000+0000

I get very good LOC coverage percentages from the unit tests, though I see the following anomalies under library/Zend/Service/Technorati:

|ResultSet.php|52.85%| |TagsResultSet.php|66.67%| |Utils.php|61.11%|

Posted by Simone Carletti (weppos) on 2007-12-28T13:08:49.000+0000

I've just installed xdebug and run the test coverage report.

Utils.php includes 3 methods defined but never implemented, this is the reason. They are utility methods useful for DRY development, perhaps I have to comment them for now.

ResultSet.php is not a class intended to be used alone, as Result.php. It should be tested directly from parent classes, hovewer I'm going to have a look at it.

TagsResultSet.php has 2 methods, one is the constructor. The second method is missing a test method, this is why the class has about 50% of code coverage.

I'm going to fix missing tests in the next hours. Any other overall feedback? :)

Posted by Darby Felton (darby) on 2007-12-28T13:55:02.000+0000

Should Zend_Service_Technorati_ResultSet be designated as abstract, since the documentation states:

bq. ...it should never be used to initialize a standalone object

Posted by Darby Felton (darby) on 2007-12-28T13:58:15.000+0000

...same with Zend_Service_Technorati_Result.

Posted by Darby Felton (darby) on 2007-12-28T14:52:03.000+0000

{quote} Utils.php includes 3 methods defined but never implemented, this is the reason. They are utility methods useful for DRY development, perhaps I have to comment them for now. {quote}

There are a couple other solutions to consider for these methods:

  • Keep them available, and use unit tests to demonstrate what they /would/ do, if they were implemented as intended.
  • Keep them available, but using @codeCoverageIgnoreStart and @codeCoverageIgnoreEnd phpdoc tags.

I think that I would prefer the first approach over the second.

Posted by Darby Felton (darby) on 2007-12-28T14:54:10.000+0000

Oops, a correction to my previous comment:

-phpdoc tags- +PHPUnit annotations+

Posted by Simone Carletti (weppos) on 2007-12-28T16:03:07.000+0000

I collected all your feedbacks and committed an update. I updated the manual to reflect some public changes (I forgot to remove a public property from ResultSet class).

Posted by Darby Felton (darby) on 2007-12-31T14:08:14.000+0000

I just committed some small changes in SVN r7318 to get the code coverage of the Utils class above 80%. I notice there are quite a few things marked "@todo" in the sources...

Under library/:

<pre class="literal">
./Technorati/CosmosResult.php:         * @todo Each field needs to be filtered and converted
./Technorati/CosmosResult.php:     * @todo    filter as Zend_Uri_Http
./Technorati/CosmosResult.php:     * @todo    Zend_Date
./Technorati/CosmosResult.php:     * @todo    Zend_Uri_Http
./Technorati/CosmosResultSet.php:        // @todo    Improve xpath expressions
./Technorati/DailyCountsResultSet.php:        // @todo    Improve xpath expressions
./Technorati/KeyInfoResult.php:        /** @todo improve xpath expression */
./Technorati/Result.php:     * @todo    XPath and DOM elements cannot be serialized, don't cache them
./Technorati/ResultSet.php:     * @todo
./Technorati/ResultSet.php:     * @todo    XPath and DOM elements cannot be serialized, don't cache them
./Technorati/ResultSet.php:     * @todo    XPath and DOM elements cannot be serialized, don't cache them
./Technorati/ResultSet.php:        // @todo Use constants
./Technorati/ResultSet.php:        // @todo Improve xpath expression
./Technorati/SearchResult.php:     * @todo    Zend_Date
./Technorati/SearchResult.php:     * @todo    Zend_Uri_Http
./Technorati/SearchResult.php:         * @todo Each field needs to be filtered and converted
./Technorati/SearchResultSet.php:        // @todo    Improve xpath expressions
./Technorati/TagResult.php:     * @todo    Zend_Date
./Technorati/TagResult.php:     * @todo    Zend_Date
./Technorati/TagResult.php:     * @todo    Zend_Uri_Http
./Technorati/TagResult.php:         * @todo Each field needs to be filtered and converted
./Technorati/TagResultSet.php:        // @todo    Improve xpath expressions
./Technorati/TagResultSet.php:        /** @todo Validate the following assertion */
./Technorati/Utils.php:         * @todo Should this method return a value or set the value?
./Technorati/Utils.php:     * @todo public static function xpathQueryAndSet() {}
./Technorati/Utils.php:     * @todo public static function xpathQueryAndSetIf() {}
./Technorati/Utils.php:     * @todo public static function xpathQueryAndSetUnless() {}
./Technorati/Weblog.php:     * @todo    Zend_Date
./Technorati.php:     * @todo    support for Zend_Uri_Http

Under tests/:

<pre class="literal">
./Technorati/SearchResultTest.php:        // @todo Zend_Uri_Http
./Technorati/SearchResultTest.php:        // @todo Zend_Date
./Technorati/TagResultTest.php:        // @todo Zend_Uri_Http
./Technorati/TagResultTest.php:        // @todo Zend_Date
./Technorati/TechnoratiTest.php:                 * @todo    validate converted value
./Technorati/WeblogTest.php:         * @todo lat, lon, hasphoto

I think these should be reduced to only those "@todos" that cannot reasonably be addressed prior to promotion and release.

If some of the issues are large enough to be worth documenting, perhaps for these you can create sub-tasks or new issues on which this one would depend?

Posted by Simone Carletti (weppos) on 2008-01-01T08:39:38.000+0000

Hi Darby, happy new Year! ;)

I saw your changes and I agree with all of them except one I'd like to ask you for more details. Here you removed the second piece of code with the explanation

{quote} * removed code impossible to execute from setUriHttp() method {quote}

This is true right now, but according to Shahar's Zend_Uri improvements proposal Zend_Uri is going to support additional schemas. Even if this is just a proposal, is reasonable to suppose that the future of Zend_Uri is open to new schemas thus the second check was created to be sure the result of


is a valid instance of a Zend\_Uri\_Http and not, for example, Zend\_Uri\_Mailto. Unit tests are good enough to point out the error as soon as Zend\_Uri will support more protocols, but restoring the control will prevent a 'last minute fix'. What do you think?

About @todos you are right, there's an high number of @todo but just because I usually first develop the basic feature and I like to add notes to remember the feature might be improved in a second time. I mean, @todos don't mean the code doesn't work, only it could have a better interaction with the environment (for example, @todo Zend\_Uri\_Http). ;)

I'm going to review the code addressing first all todos that may fit the classification 'should have', then I will create a new issue for any 'nice to have' as I did, for example, for ZF-2350.

 

 

Posted by Simone Carletti (weppos) on 2008-01-01T11:41:16.000+0000

Darby, I've cleared/fixed all reasonable @todos, added more unit tests focused on SeekableIterator interface.

Just a final question: should Zend\_Service\_Technorati\_ResultSet#getXML() be renamed to Zend\_Service\_Technorati\_ResultSet#getXml(), isn't it?

 

 

Posted by Darby Felton (darby) on 2008-01-02T15:34:46.000+0000

bq. I saw your changes and I agree with all of them except one I'd like to ask you for more details. [Here](http://framework.zend.com/fisheye/browse/Zend_Framework/trunk/incubator/library/Zend/Service/Technorati/Utils.php?r1=7294&r2=7318) you removed the second piece of code...

Oops, I had in mind that the check was against Zend\_Uri and not Zend\_Uri\_Http. Sorry I misunderstood. Please feel free to resurrect that section of code, or if you'd like, I can do it. :)

bq. I've cleared/fixed all reasonable @todos, added more unit tests focused on SeekableIterator interface.

Awesome; thanks! :)

bq. Just a final question: should Zend\_Service\_Technorati\_ResultSet#getXML() be renamed to Zend\_Service\_Technorati\_ResultSet#getXml(), isn't it?

Yes, it should be getXml() according to the naming conventions in the coding standards.

I'll perform what I hope might be a final review as soon as possible. I also suggest contacting the fw-webservices mailing list (and forwarding to fw-general, perhaps) to give the code a whirl to test it and provide feedback, to catch any lingering API issues, etc. that should be addressed prior to release with the framework core.

 

 

Posted by Simone Carletti (weppos) on 2008-01-02T17:22:56.000+0000

Fixes committed in r7333.

 

 

Posted by Simone Carletti (weppos) on 2008-01-15T02:37:49.000+0000

Hi Darby, I was wondering if you are going to move the component to core before the upcoming code freeze. :)

 

 

Posted by Darby Felton (darby) on 2008-01-29T15:46:52.000+0000

Attached test output from HEAD of the trunk/ (PHP 5.1.4, WinXP)

 

 

Posted by Simone Carletti (weppos) on 2008-01-30T02:29:29.000+0000

Gasp, I'm absolutely shocked by the output. It seems something goes wrong with Zend\_Date, I will investigate.

 

 

Posted by Simone Carletti (weppos) on 2008-01-30T02:52:36.000+0000

Hi Darby, I investigated the issue and, as I supposed, it's related to Zend\_Date. It seems Zend\_Date no longer recognizes valid dates (ZF-2524).

I'm waiting for a Thomas's answer to check what is the origin of the issue. The last time I run the suite (02/Jan/08), all tests passed without any problem.

 

 

Posted by Darby Felton (darby) on 2008-01-30T13:28:27.000+0000

Yes, it seems as though something has changed with Zend\_Date, as I have never seen such errors in the Zend\_Service\_Technorati tests until now.

Probably it would be a simple matter to provide the date format, as Thomas suggests, to get the tests passing again.

I am concerned, however, that there has been a backward compatibility break with Zend\_Date, inasmuch as people have been depending on certain behavior that is now broken. :( That said, perhaps it is not a backward compatibility break in the sense that perhaps the usage within Zend\_Service\_Technorati was broken, non-standard, not documented, or otherwise unsupported behavior. Thomas could probably shed some light on this. All things considered, I don't really understand why Zend\_Date::isDate() cannot grok the same date value that new Zend\_Date() supports. Strange, indeed.

 

 

Posted by Simone Carletti (weppos) on 2008-01-30T16:15:24.000+0000

I found the guilty, the change that broke BC compatibility (at least in Zend\_Service\_Technorati case). I'm waiting for Thomas answer.

It would be hard to provide date format because Technorati is not so coherent! You should consider that they created an own DTD but the most part of API XML responses is invalid against their own DTD!

 

 

Posted by Simone Carletti (weppos) on 2008-02-01T04:35:49.000+0000

I fixed the issue with r7751 using strtotime to validate the input.

 

 

Posted by Darby Felton (darby) on 2008-02-01T10:21:34.000+0000

Hi Simone,

It looks as though Zend\_Service\_Technorati is ready to be moved to core. Are you aware of anything that should be done beforehand?

Would you like to migrate the component, or would you like me to do it on your behalf?

Thanks for these excellent contributions! :)

 

 

Posted by Simone Carletti (weppos) on 2008-02-01T10:30:24.000+0000

Hi Darby, according to official documentation only a core contributor could promote a component to core this is why I never moved a single bit. ;)

By the way, if you want I can move it and left you free for other activities. I suppose I only need to \* svn move incubator/library incubator/test to /library /test \* promote documentation \* update docbook single files and main index. I guess there's nothing more to do, isn't it?

 

 

Posted by Darby Felton (darby) on 2008-02-01T11:32:43.000+0000

Sorry, Simone, I didn't realize that it was documented that only a Zender could perform the actual promotion. I have confidence that you would have no problem doing it, but in the interest of following the rules, I'll go ahead and make promote it today as soon as possible. :)

Thanks again!

 

 

Posted by Darby Felton (darby) on 2008-02-01T12:08:00.000+0000

Resolved with [SVN r7757](http://framework.zend.com/fisheye/changelog/Zend_Framework/?cs=7757).

 

 

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts