The patch to make tests use the batch APi broke the CLI script: http://drupal.org/node/243773

Here's a link to dimitri's patch that was posted there (needs work): http://drupal.org/files/issues/fix-run-tests-243773-129.patch

Comments

chx’s picture

Title: broken CLI test script » create a CLI version of batch API
Category: bug » task
Status: Needs work » Active

Batch 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.

chx’s picture

By 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.

wim leers’s picture

This 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?

yched’s picture

Interesting.
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).

damien tournoud’s picture

Title: create a CLI version of batch API » Fix run_tests and make it multi-threaded
Status: Active » Needs review
StatusFileSize
new9.37 KB

Here 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:

  • A mono-threaded version: in that version, run_test execute each test in sequence in a separate process space. It does that by executing itself with a special command line parameter (--execute-batch);
  • A multi-threaded version: in which run_test execute itself with the --execute-batch parameter than forks itself several times and execute tests in child copies

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.

damien tournoud’s picture

StatusFileSize
new9.46 KB

The same version without tabs or trailing spaces.

damien tournoud’s picture

Some test results on an Intel(R) Core(TM)2 Duo E6550 @ 2.33GHz:

  • Mono-threaded: 5m27.014s
  • Multi-threaded (concurrency = 2): 3m7.057s
dries’s picture

Mmm, 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.

$ php scripts/run-tests.sh --url http://localhost/drupal-cvs--all
Warning: mysql_real_escape_string() expects parameter 2 to be resource, null given in /Users/dries/Sites/drupal-cvs/includes/database.mysql.inc on line 322
Warning: mysql_query(): supplied argument is not a valid MySQL-Link resource in /Users/dries/Sites/drupal-cvs/includes/database.mysql.inc on line 109
Warning: mysql_errno(): supplied argument is not a valid MySQL-Link resource in /Users/dries/Sites/drupal-cvs/includes/database.mysql.inc on line 123

Only did a first light pass over the code but 'Detailled' is spelled 'Detailed'.

boombatower’s picture

Status: Needs review » Needs work

When 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.

damien tournoud’s picture

Note that you absolutely have to specify the --url parameter.

dries’s picture

Of 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 ...

damien tournoud’s picture

StatusFileSize
new9.95 KB

@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.

boombatower’s picture

Seems 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.

damien tournoud’s picture

In 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:

Upload functionality 109 passes, 22 fails, 0 exceptions

[Pass]  GET to http://master-1/Drupal/HEAD/node/1/edit, response is 6688 bytes. Browser drupal_web_test_case.php   808      curlExec
[Pass]  Valid HTML found on "http://master-1/Drupal/HEAD/node/1/edit"   Browser drupal_web_test_case.php        834parse
[Fail]  Found the requested form at node/1/edit Other   upload.test     171     drupalPost
[Pass]  Found the Save button   Other   upload.test     171     drupalPost
[Fail]  Failed to set field files[upload] to sites/default/files/simpletest/text-1.txt  Other   upload.test     171drupalPost
[Fail]  File attached successfully.     Other   upload.test     46      uploadFile
boombatower’s picture

In IRC DamZ and I discovered that the tests need to run as the user that the web server runs as for best results.

damien tournoud’s picture

Status: Needs work » Needs review

Because 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):

Remove feed functionality 22 passes, 0 fails, 0 exceptions
Update feed functionality 28 passes, 0 fails, 0 exceptions
Add feed functionality 24 passes, 0 fails, 0 exceptions
Update feed item functionality 27 passes, 0 fails, 0 exceptions
Remove feed item functionality 28 passes, 0 fails, 0 exceptions
Categorize feed item functionality 23 passes, 0 fails, 0 exceptions
Block functionality 48 passes, 0 fails, 0 exceptions
Blog API functionality 47 passes, 0 fails, 0 exceptions
Blog functionality 191 passes, 0 fails, 0 exceptions
Book functionality 97 passes, 0 fails, 0 exceptions
Site-wide contact form 120 passes, 4 fails, 0 exceptions
Personal contact form 70 passes, 0 fails, 0 exceptions
Comment functionality 410 passes, 0 fails, 0 exceptions
Filter administration functionality 95 passes, 0 fails, 0 exceptions
Core filters 48 passes, 12 fails, 0 exceptions
DBLog functionality 309 passes, 0 fails, 0 exceptions
Help functionality 112 passes, 0 fails, 0 exceptions
String translate 87 passes, 0 fails, 0 exceptions
Forum functionality 386 passes, 0 fails, 0 exceptions
Node revisions 26 passes, 0 fails, 0 exceptions
Node teaser 116 passes, 0 fails, 0 exceptions
Page edit test 26 passes, 0 fails, 0 exceptions
Page preview test 18 passes, 0 fails, 0 exceptions
Menu item creation/deletion 309 passes, 0 fails, 0 exceptions
Page node creation 15 passes, 0 fails, 0 exceptions
Unauthorized node view 18 passes, 0 fails, 0 exceptions
PHP filter functionality 40 passes, 0 fails, 0 exceptions
PHP filter access check 34 passes, 0 fails, 0 exceptions
Path alias functionality 70 passes, 0 fails, 0 exceptions
Poll create 27 passes, 7 fails, 0 exceptions
Poll vote 40 passes, 0 fails, 0 exceptions
Test select field 33 passes, 0 fails, 0 exceptions
Test single fields 112 passes, 0 fails, 0 exceptions
Test date field 35 passes, 0 fails, 0 exceptions
Test field weights 47 passes, 0 fails, 0 exceptions
Search engine queries 105 passes, 0 fails, 0 exceptions
Bike shed 16 passes, 0 fails, 0 exceptions
Search engine ranking 25 passes, 1 fail, 0 exceptions
Top visitor blocking 32 passes, 0 fails, 0 exceptions
SimpleTest functionality 21 passes, 0 fails, 0 exceptions
Syslog functionality 17 passes, 0 fails, 0 exceptions
IP address blocking 35 passes, 1 fail, 0 exceptions
Cron run 8 passes, 0 fails, 0 exceptions
Fingerprinting meta tag 3 passes, 0 fails, 0 exceptions
Admin overview 40 passes, 0 fails, 0 exceptions
Module list functionality 47 passes, 0 fails, 0 exceptions
Vocabulary functions 13 passes, 0 fails, 0 exceptions
Term functions 11 passes, 0 fails, 0 exceptions
Taxonomy nodeapi 38 passes, 0 fails, 0 exceptions
Tracker 106 passes, 0 fails, 0 exceptions
Translation functionality 60 passes, 0 fails, 0 exceptions
Upload functionality 133 passes, 0 fails, 0 exceptions
User registration 45 passes, 0 fails, 0 exceptions
Trigger content (node) actions 368 passes, 0 fails, 0 exceptions
Username/e-mail validation 19 passes, 0 fails, 0 exceptions
User delete 23 passes, 0 fails, 0 exceptions
Upload user picture 62 passes, 2 fails, 0 exceptions
Role permissions 22 passes, 0 fails, 0 exceptions
IP address test 5 passes, 0 fails, 0 exceptions
Registry parse file test 3 passes, 0 fails, 0 exceptions
Format size test 14 passes, 0 fails, 0 exceptions
Registry parse files test 8 passes, 0 fails, 0 exceptions
XML-RPC validator 0 passes, 0 fails, 0 exceptions

Note that the search engine failure is specific to my environment (see http://drupal.org/node/276039).

boombatower’s picture

Status: Needs review » Reviewed & tested by the community

This seems to work great...we all agree it still needs cleanup, but separate issue.

This makes it run.

dmitrig01’s picture

This 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

// Pass:
echo("\033[42;37;1m" . $text . "\033[0m\n");
// Fail:
echo("\033[41;37;1m" . $text . "\033[0m\n");

boombatower’s picture

This 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.

moshe weitzman’s picture

Interesting. FYI, I just committed a 'test run' command to drush HEAD so folks who want CLI right now can choose to use that.

dries’s picture

Odd. 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.

dries’s picture

(BTW, 'yay' for color coding.)

damien tournoud’s picture

@Dries: can you report extensively on your system characteristics?

  • what user do you run the tests with? what is the webserver user?
  • I guess you are using sites/default/settings.php, what is the permission of that file (and on the directory)?
  • which PHP interpreter do you use (the one shipped by default on MacOS X or the one from MAMP?). Does changing the $php variable in run_tests.sh change anything?
damien tournoud’s picture

@dmitrig01#18: colors would be cool, but it should be optional. It is difficult to automatically parse the resulting file with colors.

dries’s picture

Debugged it and got it to work. The following line did the trick:

root$ ln -s /Applications/MAMP/bin/php5/bin/php /usr/bin/php

http://www.scripting.com/stories/2008/06/27/classicGeekVideo.html

dries’s picture

Status: Reviewed & tested by the community » Fixed

I 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):

real	8m38.706s
user	1m49.775s
sys	0m15.136s

Concurrency 2 (new style):

real	8m33.538s
user	1m49.363s
sys	0m14.697s
damien tournoud’s picture

Status: Fixed » Reviewed & tested by the community

@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

catch’s picture

Status: Reviewed & tested by the community » Fixed

back to fixed.

damien tournoud’s picture

Hum. Why fixed? I don't believe this was committed.

dries’s picture

Status: Fixed » Needs work

It doesn't look like it is available on MAMP:

$ php -r "if (function_exists('pcntl_fork') === FALSE) { print 'not installed'; } else { print 'installed'; }"
not installed

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!

damien tournoud’s picture

Status: Needs work » Fixed

@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

damien tournoud’s picture

Status: Fixed » Needs work

Didn't mean to change the status.

boombatower’s picture

Status: Needs work » Closed (duplicate)

Run-tests works and is multi-processed.