From #2664280-102: Select lists in action & condition configuration forms Assigned to: jonathan1055:

ActionsFormTest and ConditionsFormTest now have a sequential number added into the test data key. This has two benefits (a) it makes it easier to identify which test case has failed and shows how many others passed, and (b) makes it easier to filter and run just one or two of the full set of test cases locally without editing the file. For example phpunit modules/rules --filter='ActionsForm.*(32|33)' will run just test cases 32 and 33 from the ActionsForm test.

CommentFileSizeAuthor
#10 3254675-10.interdiff-2-10.txt526 bytesjonathan1055
#10 3254675-10.form-tests.patch24.32 KBjonathan1055
#2 3254675-2-form-tests.patch24.36 KBtr

Comments

TR created an issue. See original summary.

tr’s picture

tr commented
Status: Active » Needs review
StatusFileSize
new 3254675-2-form-tests.patch24.36 KB

Here is just the above changes, extracted from the patch in #2664280-102: Select lists in action & condition configuration forms

tr’s picture

tr commented
Parent issue: » #2664280: Select lists in action & condition configuration forms

Status: Needs review » Needs work

The last submitted patch, 2: 3254675-2-form-tests.patch, failed testing. View results

TR credited jonathan1055.

tr’s picture

tr commented
Status: Needs work » Needs review
tr’s picture

tr commented
tr’s picture

tr commented

I think this is because rules_user_role_remove has assignment_restriction = selector (part of the upcasting patch) so there's no "switch to selector" button to push.

jonathan1055’s picture

jonathan1055 commented
Issue summary: View changes

It would be good to get this whole test expansion committed. I added it as part of #2664280: Select lists in action & condition configuration forms when I realised we were lacking test coverage but it is a generic improvement not just for that issue.

I think this is because rules_user_role_remove has assignment_restriction = selector (part of the upcasting patch) so there's no "switch to selector" button to push.

Yes it it. I found that discrepancy yesterday and reported it on #2800749-102: Support upcasting entity IDs to full entity contexts.

I'll provide a patch to match the current committed code, so that it passes the tests and can be committed, to give a better base to work from. Then I think we should make a decision on whether all of the inputs that specify a user should be consistent or not, and make those changes and the accompanying small test corrections.

jonathan1055’s picture

jonathan1055 commented
StatusFileSize
new 3254675-10.form-tests.patch24.32 KB
new 3254675-10.interdiff-2-10.txt526 bytes

Patch #10 removes 'user' from the array of switch buttons to press in test case 33. This passes locally, and should pass here with the current -dev codebase.

jonathan1055’s picture

jonathan1055 commented
Status: Needs review » Reviewed & tested by the community

This is RTBC for me. Then we'll have improved test coverage for all the other issues.

  • TR committed f2cd2ad on 8.x-3.x authored by jonathan1055 
    Issue #3254675 by jonathan1055, TR: Improve ActionsFormTest and...
tr’s picture

tr commented
Status: Reviewed & tested by the community » Fixed

Committed #10.

jonathan1055’s picture

jonathan1055 commented

Excellent, thanks for the quick response. I will now work on the other new issues.

Status: Fixed » Closed (fixed)

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