#2574969: Add a Views-like UI for adding fields added the preIndexSave() method for processors.
However, even though we have otherwise great test coverage for our processors, this method seems to be completely untested.
We should make sure it works correctly for all processors that use it (and maybe the processor plugin base, which provides the ensureField() and findField() helper methods).

Comments

drunken monkey created an issue. See original summary.

drunken monkey’s picture

Issue tags: +Release blocker
niko-’s picture

StatusFileSize
new5.36 KB
niko-’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: preIndexSave-tests-2734897-3.patch, failed testing.

niko-’s picture

Status: Needs work » Needs review
StatusFileSize
new6.52 KB
borisson_’s picture

LGTM at first glance. Haven't looked in detail.

drunken monkey’s picture

StatusFileSize
new7.53 KB
new1.97 KB

Thanks a lot for the patch!
However, I see that in the meantime tests were added for field renaming, which tests the functionality used in the highlight processor and the field processor base class. So only the content access processor remains (though I guess we can say that that is being tested implicitly by ContentAccessTest, which doesn't manually add those necessary fields for indexing).

Adding a test for that to the processor integration test is easy enough, so let's just do that. I also refactored your code a bit, since I don't think it's necessary to test that the fields aren't there before the processor is added.

drunken monkey’s picture

StatusFileSize
new1.2 KB
new1.99 KB
drunken monkey’s picture

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me!

  • drunken monkey committed 04ab514 on 8.x-1.x authored by niko-
    Issue #2734897 by niko-, drunken monkey: Added tests for preIndexSave()...
drunken monkey’s picture

Status: Reviewed & tested by the community » Fixed

Great, thanks for reviewing!
Committed.
Thanks again, niko-!

Status: Fixed » Closed (fixed)

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