If we move the parts that are independent from the backend technology into the base class we have a good starting point for implementing tests for different backends like Solr or elastic search.

In addition the regression tests should be split into separate functions to support easy backend specific overwrites.

Comments

mkalkbrenner created an issue. See original summary.

mkalkbrenner’s picture

Status: Active » Needs review
StatusFileSize
new100.22 KB
mkalkbrenner’s picture

StatusFileSize
new835 bytes
new100.49 KB

a small update to clean up the module list.

mkalkbrenner’s picture

Issue summary: View changes
mkalkbrenner’s picture

StatusFileSize
new12.34 KB
new117.42 KB

I moved the entity field definitions that are required for the example content from search_api_test_db to a separate test module called search_api_test_example_content.
BackendTestBase::setUp() now installs that field definitions instead of the database specific BackendTest.

drunken monkey’s picture

Title: move generic backend tests from database BackendTest into abstract base class » Add a generic base for backend tests
Priority: Major » Normal
Status: Needs review » Needs work
StatusFileSize
new8.85 KB
new114.71 KB

Thanks, great work so far!
Attached is a re-roll with the regression test for #2616804: An overlong word (more than 50 characters) was encountered while indexing. I also added a few missing comments and did some rewrite of test entity creation so it's more stable if some methods are overridden (and don't add any entities anymore). Could probably still be improved (i.e., not hard-code the entity IDs – should actually be pretty easy now, come to think of it).

However, the major problem is that the base class is still much too DB-centric. A lot of its methods (e.g., checkMultiValuedInfo(), editServerPartial(), editServerMinChars()) don't make any sense for Solr or other backends.
Instead, the class should cover all the base functionality of backends and then have some clearly defined extension points where child classes can just call whatever methods they want. There, the DB backend would test changes to its specific backend options, and things like the internal DB info, while other backends might do entirely different things.

Also, the doc blocks for the regression tests don't comply to the coding standards. In the first line, I'd instead really describe what is tested (e.g., "Tests indexing of long multi-byte words.") and then only reference the issues in the extended description (or even with @see tags and proper URLs – not sure).

mkalkbrenner’s picture

The interdiff looks good and from my point of view the the doc blocks of the regression tests are the last remaining tasks.

However, the major problem is that the base class is still much too DB-centric. A lot of its methods (e.g., checkMultiValuedInfo(), editServerPartial(), editServerMinChars()) don't make any sense for Solr or other backends.

I already thought about this and decided to keep them in the base class in my first patch. I currently started to implement the first tests for the search_api_solr_multilingual backend. There it is possible to configure something like min chars. And if you use a managed index these values can be modified on the fly. That's why I kept these functions in the base class.

As already stated above I recommend to improve the doc blocks and to commit the patch.

drunken monkey’s picture

They are still backend-specific, even if one of them is also available in one other backend. Please either correct this, or wait until I get the time to do it myself.

mkalkbrenner’s picture

I can work on this.
Could you name all functions you would like to be moved?

mkalkbrenner’s picture

Assigned: Unassigned » mkalkbrenner
mkalkbrenner’s picture

Status: Needs work » Needs review
StatusFileSize
new100.01 KB
new41.01 KB

I reviewed all the tests and moved db specific ones to the db BackendTest.

mkalkbrenner’s picture

StatusFileSize
new354 bytes
new99.98 KB

Just a small fix. Forgot to remove the call to regressionTest2511860() from the unspecific regression tests. It has been moved to the db backend specific ones already.

mkalkbrenner’s picture

Using the patch in #12, the Solr backend tests pass as well:
https://travis-ci.org/mkalkbrenner/search_api_solr/builds/127272521

mkalkbrenner’s picture

search_api_solr_multilingual passes, too
https://travis-ci.org/mkalkbrenner/search_api_solr_multilingual/builds/1...

RTBC ;-)

borisson_’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new1.92 KB
new89.08 KB

Patch is not very easy to review, but I think I it looks great, the testbot agrees here and in solr so let's get this in.

drunken monkey’s picture

Status: Reviewed & tested by the community » Fixed

Unbelievably, Joris missed a few comment style issues, but apart from that this indeed looks good. Thanks!
After confirming with Markus, I also renamed regressionTestSecondServer() to checkSecondServer().
Otherwise, looks good. Committed.

mkalkbrenner’s picture

Thank you!

Status: Fixed » Closed (fixed)

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