The \Drupal\search_api\Tests\ProcessorIntegrationTest currently has tests for all our processors (as far as I can see), except for "Role filter". (There is also a @todo comment for this.)
Should be easy to add, based on all the existing methods.

CommentFileSizeAuthor
#11 2823985-11--role_filter_integration_test.patch2.06 KBdrunken monkey
#9 interdiff-9.txt837 bytesdermario
#9 add_integration_test-2823985-9.patch2.18 KBdermario
#8 add_integration_test-2823985-8.patch1.85 KBdermario
#2 edit_add_integration-2823985-1.patch1.32 KBfelribeiro

Comments

drunken monkey created an issue. See original summary.

felribeiro’s picture

felribeiro commented
Status: Active » Needs review
StatusFileSize
new edit_add_integration-2823985-1.patch1.32 KB
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

This tests only tests if the processor is enabled, but not the actual behavior, not sure if want to test that as well.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Needs work

Thanks a lot for the patch, good start!
As Joris says, though, this should also test filling out the configuration form with editSettingsForm(). (See, e.g., checkStopWordsIntegration().)
Also, the patch should remove the @todo about this issue from the top of the file.

cebasqueira’s picture

cebasqueira commented
Assigned: Unassigned » cebasqueira
cebasqueira’s picture

cebasqueira commented
Assigned: cebasqueira » Unassigned
dermario’s picture

dermario
Primary language German
Location Zürich
commented
Assigned: Unassigned » dermario

I will have a look at it the upcoming week.

dermario’s picture

dermario
Primary language German
Location Zürich
commented
Status: Needs work » Needs review
StatusFileSize
new add_integration_test-2823985-8.patch1.85 KB

This is a manual reroll of #2 as the test class was moved to Functional tests meanwhile. I also fixed the incorrect namespace. After doing so, i was able to run these tests on my local. Lets see what the testbot says. If the test is green, i would like to test if the role filter does what it is supposed to do.

dermario’s picture

dermario
Primary language German
Location Zürich
commented
StatusFileSize
new add_integration_test-2823985-9.patch2.18 KB
new interdiff-9.txt837 bytes

Ok i did not check if the role filter actually filters users by roles, because no check* Method does that in ProcessorIntegrationTest. Maybe that should be handled in a different way?
As suggested in #4 by @drunken monkey the configuration form is filled out and one @todo was removed.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Novice

Looks good, thanks @dermario!

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
StatusFileSize
new 2823985-11--role_filter_integration_test.patch2.06 KB

Re-roll.

Testing of the actual functionality is already done in RoleFilterTest, so that's covered. In the integration test, we really just want to make sure things do not go BOOM in the UI for some reason.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Reviewed & tested by the community » Fixed

Sorry, my bad. Got ahead of myself and had #2656052: Remove the "plugin_id"/"settings" sub-level of index plugin settings already committed locally. (Would be great if you could take a look at that, Joris!)
The patch in #9 is already great – thanks a lot!
Committed.
Thanks again, everyone!

Status: Fixed » Closed (fixed)

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