Actions.inc: fatal error after DBTNG conversion

It is quite standard to use $result for the result of the query. Maybe we can rename the other result?

Tests pass, new variable name seems fine.

That's a good point, we should probably add tests here.

To write test-case here should be an action so where to put it? maybe actions_test.module against actions.inc

Another test was added #246096: Cron triggers are not executed

But testing bot still say "tests pass" ... because error happens only if $action_ids is array and Ids are numbers.

So trying to write test...

So here complex patch.

1) actions.inc fixed from #2
2) added tests to configurable actions, provided as part of trigger_test

Logic for test:
1) create non configurable action and assign on trigger cron run (already was)

2) create 2 implementations of configurable actions (same as "Send mail" but just with subject, increment variable)
3) Assign both to cron run trigger

4) run cron

5) check variables (should be changed in actions)

Patch needs corrections in code comments...

A few minor comment improvements..

+   * Assign an actions to a trigger, then pull the trigger, and make sure the actions fire.

The "an" isn't necessary anymore..

+    // Select this test action and assign it to a cron run trigger.

Hm.. I'd say, Select *the* test action....

+    // Make a copy of action with different description.

Now action is singular again, so... ".. of *an* action with.."

+    // Make sure the non-configurable action fire.

singular again, so... "action fire*s*".

+ * Implementation of a configurable Drupal action.

I think these should simply be "Implement a configurable..." now.

I can also confirm that the tests fail without the changes in actions.inc, with the following fatal error:

An error occurred.
Path: /batch?id=4&op=do
Message:
Fatal error: Cannot use object of type DatabaseStatementBase as array in /home/berdir/workspace/drupal7/includes/actions.inc on line 89

Berdir, many thanks!
Reroll

plural *have fired*

Comments fixed, tnx mr.baileys

About concern:

As i research how to make a test I found that we need case when actions_do() receive array of aids and minimum one of them should be configured. All current _trigger_user() _trigger_node() _trigger_comment() executes with ONLY one aid so using this triggers is not possible to catch this bug.

New cron trigger use actions_do with array - so this trigger is Only possible to test actions_do() with array of aids

And because tests are using only one possible trigger (cron) they placed near trigger module

reroll to track HEAD

fixed comment

- Return a form definition so the cron test configurable action can be configured.
+ Return a form so the cron test configurable action can be configured.

I worked with andypost to revise some of the comments in the test function. New patch coming shortly.

Many thanks to webchick!!!

reroll with laconic comments

removed useless 254 limit at testing action

Great job. :) Committed to HEAD!

sqlite does not have md5() function so this breaks sqlite tests #503794: Tesing results for pgsql and sqlite

Fixing that is not hard.

Well that was easy. :)