Like the ProcessorIntegrationTest, but more hardcore: in IntegrationTest, enable all processors (except for "Role filter", since the index does not contain users) and see what blows up (and, ideally, why).

Estimated Value and Story Points

This issue was identified as a Beta Blocker for Drupal 8. We sat down and figured out the value proposition and amount of work (story points) for this issue.

Value and Story points are in the scale of fibonacci. Our minimum is 1, our maximum is 21. The higher, the more value or work a certain issue has.

Value : 1
Story Points: 5

Comments

drunken monkey created an issue. See original summary.

drunken monkey’s picture

Status: Active » Needs review
StatusFileSize
new2.1 KB

Currently blows up horribly.

Status: Needs review » Needs work

The last submitted patch, 2: 2564853-2--test_processors_integration.patch, failed testing.

nick_vh’s picture

Issue summary: View changes
Issue tags: +beta blocker
borisson_’s picture

Status: Needs work » Needs review
StatusFileSize
new1.21 KB

Attached is a patch that enables all the processors.

borisson_’s picture

Locally, this patch fails; I have the most recent drupal core and search_api installed.

Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for search_api.index.webtest_index with the following errors: search_api.index.webtest_index:options.processors.aggregated_field.settings.fields variable type is NULL but applied schema class is Drupal\Core\Config\Schema\Sequence in Drupal\Core\Config\Testing\ConfigSchemaChecker->onConfigSave() (line 98 of core/lib/Drupal/Core/Config/Testing/ConfigSchemaChecker.php).

drunken monkey’s picture

StatusFileSize
new1.05 KB
new2.26 KB

Test bot says the test succeeds with "249 passes", locally I get 267 – and one exception. Seems the patch once again doesn't actually get applied. The test bot really picked a great time to collapse …
Anyways, the attached patch should fix this, too. Great we actually spotted a bug here!

One other thing, though:

+++ b/src/Tests/IntegrationTest.php
@@ -88,6 +88,36 @@ class IntegrationTest extends WebTestBase {
+   * Test all the processors, all the time.

What do you mean with that?
Actually, I'd find it a good idea to keep the processors always enabled. But this will just enable it in a separate test case and then test nothing more, especially not indexing or searching. So I think that should be changed to be a method right at the beginning of the existing test, which enables all processors and just leaves them on for the complete test. Might change some results and thus need more changes to the test case, but we'd just have to review those to see whether they make sense or point to bugs.

borisson_’s picture

Status: Needs review » Needs work

The comment was just a description for the test, phpcs complains when a method doesn't have a description, it can be changed to anything else.

I agree that this might be better to leave them on for the complete test - setting this back to needs work.

borisson_’s picture

Status: Needs work » Needs review
StatusFileSize
new942 bytes
new2.28 KB
drunken monkey’s picture

StatusFileSize
new2.24 KB
new2.34 KB

This needed a re-roll. Also, I'd like to try whether enabling the processors before setting the fields works, too. That way, they have the most time to mess things up.

Status: Needs review » Needs work

The last submitted patch, 10: 2564853-10--test_enabling_all_processors.patch, failed testing.

The last submitted patch, 10: 2564853-10--test_enabling_all_processors.patch, failed testing.

drunken monkey’s picture

Status: Needs work » Needs review
StatusFileSize
new2.42 KB
new4.59 KB

I think that should fix it.

drunken monkey’s picture

StatusFileSize
new4.54 KB

Status: Needs review » Needs work

The last submitted patch, 14: 2564853-14--test_enabling_all_processors.patch, failed testing.

The last submitted patch, 14: 2564853-14--test_enabling_all_processors.patch, failed testing.

drunken monkey’s picture

Status: Needs work » Needs review
StatusFileSize
new3.61 KB
new7.56 KB
borisson_’s picture

Status: Needs review » Reviewed & tested by the community

Looks great!

drunken monkey’s picture

Status: Reviewed & tested by the community » Fixed

Great to hear, thanks for reviewing!

  • drunken monkey committed e7ff5e8 on 8.x-1.x
    Issue #2564853 by drunken monkey, borisson_: Added a test that enables...

Status: Fixed » Closed (fixed)

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