Closed (duplicate)
Project:
Drupal core
Version:
7.x-dev
Component:
simpletest.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
25 Jun 2008 at 14:07 UTC
Updated:
26 May 2009 at 17:02 UTC
Jump to comment: Most recent file
Comments
Comment #1
chx commentedBatch API IMO needs to be changed into a pluggable system -- it is, after all a glorified for loop: init, do one, go to next step, evaluate whether we are finished, loop. This is the first step towards a new CLI script. There are others.
Comment #2
chx commentedBy the way we already have two: web js and web redirect. This would be a third implementation. I guess a webservice one would also make sense as contrib.
Comment #3
wim leersThis third one, webservice, would e.g. allow for a website to send a batch of files? Or a desktop application to upload a batch of images?
Comment #4
yched commentedInteresting.
Not sure if this relevant to the current CLI/simpletest issue, but the 'progressive' part of the batch API can be disabled using a 'progressive = FALSE' mode where all operations are run in the same request (leaving them exposed to PHP timeouts, of course). It is currently an internal flag controlled only by form.inc to process batches set by drupal_execute'd forms (where we can't have the request hijacked to a progress page).
It's a matter of a simple patch to make it an actual API parameter for batches, thus letting the same code handle both 'single request' and 'multiple request' processing cases (this is actually more of an oversight in the initial batch patch, that I realized only after dealing with a few actual batch API use cases, and it was already quite late in the D6 cycle).
Comment #5
damien tournoud commentedHere is a first attempt at fixing run_tests following the Batch API patch, and at making it multi-threaded to allow parallel execution in multi-core and multi-cpu machines.
This patch introduces a public parameter --concurrency [num] that control the concurrency. Parallel execution by forking is not supported under Windows, so this parameter has no effect there.
Two execution types are supported:
Note that we can't fork while Drupal is bootstrapped (database connection would be lost, etc.).
The "reporting" part still needs work (grouping, etc.), but review is appreciated.
Comment #6
damien tournoud commentedThe same version without tabs or trailing spaces.
Comment #7
damien tournoud commentedSome test results on an Intel(R) Core(TM)2 Duo E6550 @ 2.33GHz:
Comment #8
dries commentedMmm, when I start the tests, I get all sorts of MySQL errors. The tests don't start running. I did a clean install of Drupal and could reproduce this on a clean site. I'll have to investigate this more.
Only did a first light pass over the code but 'Detailled' is spelled 'Detailed'.
Comment #9
boombatower commentedWhen trying to run any tests either via group name of --all I get
PHP Notice: Undefined index: host in /home/jimmy/software/php/drupal/scripts/run-tests.sh on line 84
PHP Notice: Undefined variable: test_id in /home/jimmy/software/php/drupal/scripts/run-tests.sh on line 130
PHP Notice: Undefined index: host in /home/jimmy/software/php/drupal/scripts/run-tests.sh on line 84
PHP Notice: Undefined variable: execute_batch in /home/jimmy/software/php/drupal/scripts/run-tests.sh on line 124
ERROR: No valid tests were specified.
Comment #10
damien tournoud commentedNote that you absolutely have to specify the --url parameter.
Comment #11
dries commentedOf course, I'll be forced to start saving for an eight-core Apple Mac Pro (http://www.apple.com/macpro/) now. And of course, it should come with a 30" Apple Cinema HD display so I can use some screen space to keep a test window open. Thanks guys. ;-)
Or maybe not. Not yet ...
Comment #12
damien tournoud commented@Dries: I have no idea where you issues with mysql.inc (and not even mysqli!!) could come from.
Here is an updated version of the patch, including E_ALL-related issues fixes and spelling and grammar review. We now enforce that --url is set, because I know no development environment where the default $host (localhost) and $path (/) could apply.
Comment #13
boombatower commentedSeems to work great. The only test I noticed that was out of place was the Blog API test. It doesn't pass, although it does it web interface.
Comment #14
damien tournoud commentedIn fact is looks that there is an issue with the upload functionality. I guess it's in fact not really related to that patch but stayed under our radar until now.
From the CLI:
Comment #15
boombatower commentedIn IRC DamZ and I discovered that the tests need to run as the user that the web server runs as for best results.
Comment #16
damien tournoud commentedBecause folders (like files/upload) can be created from the webserver with a restrictive umask, we have to require that the tests are run from the webserver user.
Here are my latest results (from patch in #12):
Note that the search engine failure is specific to my environment (see http://drupal.org/node/276039).
Comment #17
boombatower commentedThis seems to work great...we all agree it still needs cleanup, but separate issue.
This makes it run.
Comment #18
dmitrig01 commentedThis may be something for later, but I have two comments - first, there should be a --verbose option which shows all assertions, and second, you should have highlighting/coloring, as found in my script - http://drupal.org/files/issues/fix-run-tests-243773-129.patch
Comment #19
boombatower commentedThis patch is just to make it work again. Already a thread open for improving it and that is one of the things that will be included.
Comment #20
moshe weitzman commentedInteresting. FYI, I just committed a 'test run' command to drush HEAD so folks who want CLI right now can choose to use that.
Comment #21
dries commentedOdd. I still get the mysql errors -- on both my Macs using MAMP. Need to investigate this more but at first glance it smells like $active_db doesn't get set. Maybe the proper settings file doesn't get loaded.
Comment #22
dries commented(BTW, 'yay' for color coding.)
Comment #23
damien tournoud commented@Dries: can you report extensively on your system characteristics?
Comment #24
damien tournoud commented@dmitrig01#18: colors would be cool, but it should be optional. It is difficult to automatically parse the resulting file with colors.
Comment #25
dries commentedDebugged it and got it to work. The following line did the trick:
root$ ln -s /Applications/MAMP/bin/php5/bin/php /usr/bin/phphttp://www.scripting.com/stories/2008/06/27/classicGeekVideo.html
Comment #26
dries commentedI have a 2.16Ghz Intel Core 2 Duo with 2GB of RAM but it doesn't seem to be very 'dual core'-ish at first glance -- or maybe I don't know enough about the dual core architecture.
Concurrency 1 (old style):
Concurrency 2 (new style):
Comment #27
damien tournoud commented@Dries: can you check that the Process Control Extension is compiled in on MAMP?
If not, the script will silently fall-back to mono-threaded execution.
http://php.net/manual/en/book.pcntl.php
Comment #28
catchback to fixed.
Comment #29
damien tournoud commentedHum. Why fixed? I don't believe this was committed.
Comment #30
dries commentedIt doesn't look like it is available on MAMP:
I think this shows that it would be better if the script didn't silently fail when concurrency > 1. I'd simply exit with an error message advising people to omit the concurrency level or recompiling with the 'Process Control Extension'. I wouldn't have figured this one out otherwise, and I'd rather recompile PHP to take advantage of this than _thinking_ it was working.
Also, I've made some minor edits to the patch and committed it as is. The above suggestion(s) still need to be incorporated but I figured a working script is better than a broken script. Let's follow-up with more patches to get the details right. I'm marking this 'code needs work'.
Thanks for the hard work!
Comment #31
damien tournoud commented@dmitrig01: do you want to reimplement color on that basis? Be my guest.
It also seems that moshe implemented a reporter that is probably better than what we have today (drush_print_table() in drush HEAD), see http://drupal.org/node/275536
Comment #32
damien tournoud commentedDidn't mean to change the status.
Comment #33
boombatower commentedRun-tests works and is multi-processed.