Double test a patch for a single request

The only way I can see this happening is if pifr/review is called twice while file data is loaded thus invoking pifr_review_run().

A possible fix would be to change the pifr_reviewing to only be true once pifr_review_run() has been called, and use file information for check.

I'm going to rework status/pifr_reviewing stuff in #343100: Stop any PHP process when slave reset command issued & report slave status checking.

Once complete I'll check and see if this is still an issue.

Appears to have occurred on: http://testing.drupal.org/pifr/file/1/2510.

12/16/2008 - 16:40	Results sent to project server.
12/16/2008 - 16:38	Result received from slave #4 (Failed: 7600 passes, 40 fails, 433 exceptions).
12/16/2008 - 16:35	Results sent to project server.
12/16/2008 - 16:31	Result received from slave #4 (Passed: 7660 passes, 0 fails, 0 exceptions).

Different results the second time interestingly.

The only function to invoke the pifr.file.review xmlrpc command on a slave is pifr_send_to_slave().

The function is called in the following locations:

• pifr_send_free_slaves() - pifr.cron.inc
• pifr_result() - pifr.module (which sends another file to a slave right after it reports back)
• pifr_check_project_server() - pifr.cron.inc (related to checking drupal core, which is disabled)

pifr_send_to_slave() should call pifr_file_mark_sent() upon success which records the event.

I see two reasons this may occur given the log:

• Sending patch to slave fails and is thus not recorded (which is a different issue entirely, it would be on testing master server end of things)
• Testing slave is getting stuck and sends results twice...somehow messing them up.

This is a correction to the logic that attempts to prevent tests from running twice.

This does not explain why the tests are being run twice. Either some accidental call to pifr/review/ which also has the key!?!? or somehow the process does not terminate after sending results?

Those are my current ideas.

This patch will fix the logic issue and may indirectly fix the issue. It does not explain why the results differ.

The other problem is how to test this, since it is one of those "fun" issues that only occurs some of the time it is very hard to debug. (possibly cron related? - although I'm not sure how as it should still show up in log)

Patch applied to #4 and PIFR debugging option enabled. The log looks good and test still appear to run so this at least this doesn't break anything (as expected).

pifr_debug	12/17/2008 - 04:00	Results sent.	Anonymous	
pifr_debug	12/17/2008 - 04:00	Testing completed.
pifr_debug	12/17/2008 - 03:52	HEAD installation complete.	Anonymous	
pifr_debug	12/17/2008 - 03:52	PHP syntax checked.	Anonymous	
pifr_debug	12/17/2008 - 03:52	Patch applied.	Anonymous	
pifr_debug	12/17/2008 - 03:52	CVS checkout complete.	Anonymous	
pifr_debug	12/17/2008 - 03:52	File fetched.	Anonymous	
pifr_debug	12/17/2008 - 03:52	Starting file review.	Anonymous	
pifr_debug	12/17/2008 - 03:52	Background process started.	Anonymous	
pifr_debug	12/17/2008 - 03:52	File information variables set.	Anonymous

Test results match the ones before.

Bart suggested this may be memory related as he had an issue with a slave with the same memory limit as #4.

Committed.

I'll wait for memory limit to be raised and run a number of tests through without this occurring before I close issue.

I'll mark this as fixed and re-open if discovered again.