https://bugzilla.wikimedia.org/show_bug.cgi?id=20077
Chad H. <innocentkil...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |innocentkil...@gmail.com
--- Comment #1 from Chad H. <innocentkil...@gmail.com> 2009-08-07 19:32:59 UTC
---
Just some early feedback, haven't looked into the run all stuff yet.
(In reply to comment #0)
> * ArticleTest.php: fixed bug related to global variables that may or may not
> exist. Tests for global variable existence before attempting to save it for
> later restoration.
>
> * LocalFileTest.php: fixed bug related to a change in the directory structure
> used to store thumbnails in file based repositories.
>
These two fixes look fine.
> * MediaWiki_TestCase.php: fixed bug related to a change in the name of the
> global variables $wgDBadminuser (-> $wgDBuser) and $wgDBadminpassword
> (->$wgDBpassword). Also fixed bug that wrote over $wgDBprefix, causing
> creation
> of test tables to fail. Tested code for both MySQL version >= 4.1 and < 4.1
> (by
> creating temporary debug version of if statement - tests occured using MySQL
> version 5.0.41).
>
This is incorrect. $wgDBadminuser/$wgDBadminpassword are used for attaining
"administrative" access to the DB. In practice, most scripts don't need this.
Since we're creating temp tables though, it's probably worth using
$wgDBadminuser here, we don't know if $wgDBuser has access to do that.
> * README: Updated so it mentions requirement to set $PhpUnitDirectory in
> run-test.php and explains use of new --runall option.
>
> * SearchEngineTest.php: renamed to SearchEngineTest.inc so algorithm in
> run-tests.php that automatically finds all PHPUnit tests works (see below).
>
> * SearchMySQL4Test: renamed 'SearchEngineTest.php' to 'SearchEngineTest.inc'
> in
> require_once() call.
>
This all looks fine too.
> There is an architectural issue related to this patch that needs attention by
> senior developers. I decided to add the --runall option to run-test.php so we
> can run the PHPUnit tests without relying on the make utility. However, in
> order to retain the ability to use the current Makefile, I used an algorithm
> to
> find the PHPUnit tests in the /tests/ directory that isn't very pretty.
> Specifically, the algorithm assumes all files of the form <prefix>Test.php are
> PHPUnit tests (e.g., ArticleTest.php). This follows the convention used by
> PHPUnit tests that the unit test for class <class> is named <class>Test. This
> convention was used by the developer who initially created the PHPUnit tests
> for file names as well.
>
> A better architectural approach would be to create a subdirectory of /tests/,
> say PHPUnitTests, that contains all PHPUnit tests. This, however, requires a
> structural change to the /tests/ directory that is sufficiently major that it
> requires review. If this approach is considered superior, it would be easy to
> modify run-tests.php to run all tests in the PHPUnitTests sub-directory. If
> senior developers give me feedback in bug ticket comments that this strategy
> is
> better, I can provide an updated patch fairly quickly.
>
Moving the tests to a subdirectory would be fine, and pretty trivial as long as
all referencing paths are updated too.
> Another architectural approach is to create a file in /tests/ that lists the
> PHPUnit tests to run. This has the disadvantage that it requires updating
> whenever a new test is added. My view is this is a less desirable option.
>
Ew no. That's the exact thing we're trying to avoid doing...we've got way too
many "lists of" type configs out there, and it's getting ridiculous.
Also, something I noticed: $PhpUnitDirectory is nasty and should be removed. A)
PEAR should already be in the include path B) We don't like telling people to
edit any files other than LocalSettings, as changes can get lost on upgrade.
--
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
