Re: [Semanticscuttle-devel] Status for configurable privacy ....

[Brett D. <bs...@fr...>]

Hello, Christian.

Thanks for the helpful suggestions. I will work to improve the tests
this weekend.

Rgrds,
Brett

On Wed, 6 Apr 2011 09:55:01 +0200, Christian Weiske
<cw...@cw...> wrote:
> Hello Brett,
> 
> 
>> I have some concerns about the fact that there were Windows
>> carriage returns in my last check-in. I am working on a Windows
>> environment--I will eventually set up a Linux environment, but not
>> for a few weeks at least--but I've been editing files with vi on
>> Cygwin and MKS Toolkit, so there should not have been any Windows
>> carriage returns.
> No problem. Someone noticed it, now it's fixed, all is fine :)
> 
>> Any tips/advice you have concerning this would be
>> appreciated.
> Sorry, I've never worked with cygwin or so. I know that many
> editors/IDEs on windows are able to save files with unix newlines only,
> i.e. Notepad++ and all the PHP IDEs.
> 
> 
>> Here are a few things I noticed when testing import.php and
>> importNetscape.php:
>> 1. import.php points the user to http://del.icio.us/api/posts/all to
>> save bookmarks to an XML file. This is evidently not a good option any
>> longer for "new" del.icio.us users who are using a Yahoo! ID to access
>> their bookmarks. A better option for these users is to go to the
>> Delicious XML Exporter at http://deliciousxml.com/. Otherwise, they
>> must use http://api.del.icio.us/v2/posts/all with open authorization,
>> and that complicates the process somewhat.
> Yep, we should change that.
> 
>> 2. I think I may have found a bug in importNetscape.php, which ignores
>> the last bookmark in the specified HTML file. I think this is
>> occurring in the regular expression given as the first parameter of
>> the preg_match_all function call. Consequently, my test data contains
>> a last entry that I have assumed will be ignored. The test will need
>> to be updated if this behavior in importNetscape.php changes.
> Ok, I'll see if I can fix that.
> 
> 
> About your tests:
> - You assume that the default privacy is configured to the default one,
>   but that's something you cannot assume. If you do such assumptions,
>   at least do a assertion on your assumed variable so the test fails
>   with an explanatory message.
> - You assume a default privacy setting since there was no way to
>   configure the privacy permanently for http requests - up to now! In
>   the jquery branch, I had the same problem and solved it; the
>   testbaseapi class has a way to set default configuration values.
> - You had one big method in BookmarksTest that checked all the
>   different api methods. In general, keeping test methods small is a
>   really good idea since in case of a failure, you can narrow down the
>   problematic code line much faster. The rule of thumb is to test one
>   thing in a test (but I don't want only one assertion in one test
>   method, that'd be overkill).
> - I began to split up the big test method and
>   moved the api/posts_add test to their corresponding Api test
>   class. By doing that I could utilize the TestBaseApi methods that
>   prepare authorized HTTP requests automatically for you if you wish
>   that. Do the same for the rest of the tests in there.
> - In tests, always use example.org as hostname, or example.com and
>   example.net (of course, you can use foo.example.org,
>   example.org/foobar and such).
> - You assume that new bookmarks always get ID 1 in your tests. That
>   might not always be the case, especially with database systems other
>   than mysql. Better than fetching bookmark ID 1 is to fetch all
>   bookmarks of the user with the ID you just created which keeps you on
>   the safe side.
> - Assuming http://localhost/ as URL fails for my setup since I have
>   "http://bm.bogo/" as URL under which my dev instance of SC is
>   available. TestBaseApi takes care of this.
> 
> I cherry-picked the two relevant changes to testbaseapi into your
> branch. You still need to merge master, though.