Re: [VuFind-Tech] Testing protected und private methods (a pull request \o/)

[David M. <ma...@ha...>]

At Tue, 24 Jul 2012 13:33:08 +0000,
Demian Katz wrote:
>
> Thanks for this.  A few comments before I merge it into master:
>
> 1.) There are two possible solutions to eliminate the ugly require in the test bootstrap:
>
>     a.) Just put the class under src/Test/TestCase.php instead of under test/TestCase.php.  I don't think this is a bad solution: src/VuFind is the home for all auto-loaded code in the VuFind namespace.  Just because a class is used only for testing doesn't mean it can't go there -- it will only be loaded when needed, so it won't hurt anything.  I realize that the fact that the actual test classes are also in the VuFind\Test namespace may cause some confusion.  If this is a big concern, we could consider creating a different namespace (i.e. VuFindTest... though I don't like that idea much) or remove namespacing from the test cases entirely (it's not strictly necessary there, though I like having it to reduce the odds of unexpected name clashes).
>
>     b.) Add an explicit ClassMapAutoloader line for the class in module/VuFind/Module.php's getAutoloaderConfig() method.  I like this solution less since it adds a small amount of overhead throughout the VuFind module.  You could avoid this overhead by creating a VuFind\Test module which is only loaded by the test bootstrap (sort of similar to the way the CLI module loads), though that might be overkill.
>
> Right now I favor solution a -- keeping all the VuFind-namespaced code in one place seems like the simplest solution.

Agreed. a) is simple enough.

Re modifying the namespace: In other projects I put the test classes
in the same namespace as the class to be tested. Not sure where I
picked this up (supposedly from Symfony2 or Phix) and this wouldn't
address the question of the namespace for TestCase.


>
> 2.) Please use spaces instead of tabs to prevent style warnings in
> continuous integration.  (A trivial issue, and I can take care of
> style fixes on my end if you prefer...  but complying with the
> current PEAR standards will save a little bit of time).

Thanks for spotting this. Misconfigured editor.

>
> 3.) The empty Solr\QueryTest.php causes PHPUnit to issue warnings.  Do you mind removing it until it has content?
>

This class was there by accident.

> Let me know how you would like to proceed -- hopefully it won't take
> much work to get this up to speed, and I can have a go at merging in
> my first pull request.

I'll move TestCase, M-x untabify, implement methods for calling static
methods and r/w access to static and non-static properties, recreate
the branch and send a new pull request.

This will likely be done by tomorrow at last.

Best,
  -- David
--
David Maus
Herzog August Bibliothek - D-38299 Wolfenbuettel
Phone: +49-5331-808-317
Email: ma...@ha...

PGP Key 0x0CC2E093512F7385
Fingerprint 1AD2 EE67 224F 18C5 EA55  98AD 0CC2 E093 512F 7385