Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Not all fields from the index config make it to the index.
I currently only have a test to prove this theory. I have no idea what is actually going on. Some facet tests use "keywords" as a field, and that doesn't make it to index. When I run ::getFields on the index I don't see the keywords field, so there don't seem to be any keywords and I can't get them to pass. Weirdly enough this doesn't seem to happen all the time.
The testcase here does seem to prove that the fields are not all the same.
This might be because of missing dependencies - but if that's the case, we should clearly state those dependencies in the `search_api_test_db` module.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 2839142-15--test_index_setup.patch | 14.54 KB | drunken monkey |
Comments
Comment #2
borisson_Comment #4
borisson_Comment #5
borisson_More explicit assertion in the test.
Comment #7
borisson_Comment #9
borisson_The patch in #7 fixes the problem that we're testing for, but because it's a sledgehammer-type solution it breaks other tests.
Trying to figure out how to actually fix the underlying problem. I think this is a very valuable test though.
Comment #10
borisson_I have no idea how to fix this.
Comment #12
drunken monkeyI think that's the same problem that forced us to add
\Drupal\Tests\search_api\Functional\ViewsTest::installDrupal()(see lines 855–865 and #2612104-22: Cascade properly when processors providing new properties are disabled): we add the "Keywords" field for the "article" and "item" bundles, but those don't exist by default so we need to hack pretty deep into the test setup to enable them early enough.Not actually sure why we don't just add the fields for the default (
entity_test_mulrev_changed) bundle instead/additionally, though? Probably because the fix was relatively easy back then and just got vastly inflated by the move toBrowserTestBase.Comment #13
borisson_Fixes the test, thanks so much for helping out @drunken monkey , this makes so much more sense now that I more or less understand what's going on.
Actually, couldn't we remove them from our own bundles now? They should get them from the default bundle, no? Trying that now.
Comment #14
borisson_Looks like that doesn't work. The test doess pass though, this should avoid confusion for anyone who adds a new test and should increase the sturdiness for anyone useing this example content.
Comment #15
drunken monkeyGood to hear this worked, thanks!
However, more interesting would be to see whether that also fixes the
ViewsTestwithout theinstallDrupal()override.If so, we also don't need the new test, since
ViewsTestwould cover this anyways.Removing the fields from the other two bundles can't work since we're creating articles/items in the tests with these fields – if those bundles don't have these fields, this can't work.
Also, removing the unrelated
Indexchanges.Comment #17
borisson_Look like that doesn't work, I do like this as it makes my tests easier, with this patch (#13) applied, I can make my own tests easier #2839629: Remove ::installModulesFromClassProperty from test.
Comment #18
drunken monkeyHm, weird. Unfortunately, debugging doesn't seem to work for me anymore since the move to BTB tests, so this'll have to wait.
Anyways, should we just commit the additional test field config files? And would you then say we should also add the new test, just to be safe?
Comment #19
borisson_The additional config files make sense yeah. I'd say the new test makes sense as well. Even though it might test something that's implicitly tested somewhere else I don't really mind that tbh.
Comment #21
drunken monkeyOK, then let's do that.
Committed #15 minus the
ViewsTestchanges.Thanks again!