Simpletest does not allow assigning actions to triggers

Added tags

This is a test to hit a bug

The last submitted patch failed testing.

The last submitted patch failed testing.

The last submitted patch failed testing.

The last submitted patch failed testing.

Let's test

That's a good one.

merged from other patch, now hook is user_login

trigger_get_assigned_actions returns array of arrays

I have RTBC'd above 'cos I knew that the bot can set to CNW should a test fail. I am still content with the patch and now the bot is too.

Oh my... english
This should be like

Check that action assigned to the right hook of the trigger

Change

+ // Verify that the action assigned to correct hook.
+ $actions = trigger_get_assigned_actions($hook);

to

+ // Verify that the action has been assigned to the correct hook.
+ $actions = trigger_get_assigned_actions($hook);

OK then back to the green queue.

I have no idea what this is doing. Please explain and add comments.

This still needs documentation, and I'm not really sure that the solution makes sense. I would expect that hidden fields need to be treated like text, textarea and password. We need to pass them to post anyway.

Just explain what you just explained in an inline comment for that case statement and be done with it.

uhm, replace that with maybe: "Normally, hidden fields are considered read-only. However, to allow to submit multiple forms on a single page that only differ in a hidden value, we allow their value to be overridden."

I'm on crack. Are you, too?

uhm, I just wanted to fix the wrapping of that inline comment, but after looking at the actual code and how the processed values of handleForm() are used afterwards, that inline comment is utterly wrong.

Anyway, since I did it, you get it.

that inline comment is utterly wrong.

Hidden fields are not read only. Like, not read only at all.

@Damien Tournoud in this context hidden fields are read-only because you can't set them (within this function).

I leave this issue because this small and useful fix eats a lot of attention and all about comment...

A write a lot to explain what it all about :( sorry...

deekayen requested that failed test be re-tested.

IMO, we should address this in a more generic fashion. The problem with the triggers page is that each form has the same Submit and the same values in the select. The problem is that the only way we have to choose among them is the value of the button, which is pretty weak. If we could just select with an xpath expression, I think that would do it.

I opened #709852: Impossible to write a test where duplicate buttons/forms exist as a more generic statement of the same problem (encountered in the same place). I'm marking it as a dup of this one.

I don't know if this causes us pain in other areas, but the assignment of actions to triggers is basically not being tested because of this limitation. You can only assign the first item on the page.

I spent quite a lot of time with this tonight, and andypost's approach will work. There were a couple of problems due to the passage of time, but this patch works.

I took such delight in the fact that we can now test triggers and actions that this fleshes out Andypost's original test, adds another one that triggers and advanced (email) action - which we couldn't do before.

I also took the liberty of adding two tiny email helper functions to drupal_web_test_case(). One allows assertion on a *pattern* in an email message, and the other allows verbose output of a mail message. Mail hasn't been supported as well as page views, but when it's better supported, we'll have better tests.

Finally, the trigger_test mock module for trigger had not been properly updated to D7, so was creating warnings about a missing label field. I took the liberty of fixing that, since it's in the line of trigger test and all.

The bot will find 2 exceptions (hopefully only two): The new test uncovered #714190: system_send_email_action uses $account without initializing it , which had already been reported.

Well, unfortunately these test failures demonstrate that this approach does *not* work.

The idea was a workaround to try to get simpletest to find the correct form based on a passed-in hidden field. If the hidden field matches what's there, it helps select the correct form on the page.

The problem is that forms like imagefield's forms submit hidden fields, which are *different* from what's already in the form. So I don't think we're going to be successful piggy-backing on the hidden field idea. This was my original reservation in #45.

IMO we have to change the interface to drupalPost() somehow to give it the ID or name of the form we want to submit.

I will roll a proposed alternate. I hope an interface change will fly, because we really need this testing capability.

The last submitted patch, drupal.simpletest_triggers_601398_50.patch, failed testing.

The test using system_send_email_action moved to #721086: Create tests for system actions, clean up token replacement, clean up triggers

If anything this should be just $form_id. It doesn't have anything to do with CSS.

Fails coding standards.
- Needs newline between function summary and @params
- We aren't documenting types in @param or @returns yet.

Fails multiple coding standards.
- See above points, same apply here.
- Why not just use $edit instead of $action_configure_edit?

Wow. Just wow. This is going to be a fun chunk to maintain. Is there any better way to accomplish this? Can we just load the action from the database and use that data to find the specific link instead of this voodoo? :)

Powered by Dreditor.

I think it's fantastic. Much improved. I really like what you did with the form id. That really helps bridge the gap between the Drupal and the HTML world.

Quick re-roll: there was a hunk from modules/trigger/tests/trigger_test.module which already fixed

Also about code-style - docs said that @param could have type and it is recommended for complex types of parameters

Edit: #60 is the same patch as #56, rerolled. The two pieces removed belonged with #721086: Create tests for system actions, clean up token replacement, clean up triggers so I moved them over there.

This should go in.

md5() staff used to separate actionIDs that are code-defined and user-defined.
Suppose rules would grow older for D8 and will replace triggers

Working on #403446: Increase default weight of trigger module
I found that drupal_html_id() uses drupal_static() and there's no ability to post the same form.

Here's a patch to fix this:

1) Logic to clean $form_id copied from drupal_html_id() with comment (please check my grammar)
2) Tests for triggers extended with real form names (pointing actual trigger to assign action to)
3) TriggerContentTestCase a bit optimized - there's no need to create new users in loop

@sun do you propose to change tests to pass a clean $form_id?

Added comment and tests are changed to actial form IDs

user-login formID missed

Since we're going back to the HTML ID, I'd like to make the signature really clear about that. This reroll addresses that.

Dang. I loved $form_id in that function. :(

Also this?

Ugh. So very, very ugly. And an API change to boot.

Since the reason for changing this according to #65 is "I found that drupal_html_id() uses drupal_static() and there's no ability to post the same form," can we not fix this with #684568: drupal_html_id() should use $drupal_static_fast pattern instead?

Sorry. I meant the fact that test writers who used to be able to do:

now have to do:

This was not the case in the original patch that was committed.

Sorry, quite some confusion here.

1. The optional form id was already introduced in a (committed) patch earlier in this issue. It is entirely optional, and only required in cases you have the same form more than once on the page.
2. The earlier patch introduced $form_id as parameter, specifying the internal form_id. This is wrong for two reasons:
   
   1. When using the internal $form_id, we need to invoke drupal_html_id() to translate the $form_id into a HTML id, and, assume that we get the translated form_id we want. Since the sole purpose of drupal_html_id() is to ensure that ids in Drupal's HTML output are unique, it's easily possible that drupal_html_id() will not return what you expected, due to other tests/assertions running before yours.
   2. The entire SimpleTest functionality and helper functions work with values from the HTML output. We do not specify an multi-dimensional array of form values to post, but instead input names like foo[bar], as visible in the output. We also specify form button names using t('Save'), as visible in the output. Therefore, it is wrong to specify an internal $form_id, because this is not what we see in the output.

Actually, I just got bit by the fact that drupal_html_id() is clearly the wrong tool for this job, as it generated an incorrect HTML ID for me (with a -2 on it). I think we probably have to use the HTML ID to make this work right.

Another +1 from me for this as is.

Automatically closed -- issue fixed for 2 weeks with no activity.