Faster tests by using wait_task

[PLNech]

Today, several of our tests are slowed down by forced sleeping:

def test_save_signal(self):
        Website.objects.create(name='Algolia', url='https://www.algolia.com')
        Website.objects.create(name='Google', url='https://www.google.com')
        Website.objects.create(name='Facebook', url='https://www.facebook.com')

        time.sleep(10)  # FIXME: Expose last task's ID so we can waitTask instead of sleeping
        self.assertEqual(raw_search(Website)['nbHits'], 3)

We could replace the sleep call by leveraging wait_task. This requires:

• exposing it on the Client and not only on the index, see PR refactor(client/index): expose wait_task on the client algoliasearch-client-python#343
• facading it in AlgoliaEngine
• Because the taskID is returned in the __post_save_receiver hook, we need to expose the last taskID somewhere like AlgoliaEngine.last_taskID so you can then refactor the previous code as:

def test_save_signal(self):
        Website.objects.create(name='Algolia', url='https://www.algolia.com')
        Website.objects.create(name='Google', url='https://www.google.com')
        Website.objects.create(name='Facebook', url='https://www.facebook.com')

        algolia_engine.wait_task(algolia_engine.last_taskID)
        self.assertEqual(raw_search(Website)['nbHits'], 3)

[clemfromspace]

A complete different idea, why not just mock the call to the algoliasearch python package ?

Example:

from unittest.mock import patch, call

def test_save_signal(self):
    with patch('algoliasearch.index.Index.save_object') as mocked_save_object:
        Website.objects.create(name='Algolia', url='https://www.algolia.com')
        Website.objects.create(name='Google', url='https://www.google.com')
        Website.objects.create(name='Facebook', url='https://www.facebook.com')

    mocked_save_object.assert_has_calls([
        call({'name': 'Algolia'}),
        ...
    ])

This way, we avoid unnecessary calls to the API and avoid waiting for the answers.

While I do think calling the API and check the responses are necessary in some cases (ie the indexes settings parameters checks), I don't think it's necessary in this kind of tests since the algoliasearch package have already have tests itself.

WDYT?

[PLNech]

@clemfromspace 💯, let's stop duplicating E2E tests for the sake of it. I'm all in for mocking everywhere we can! 🙂

This being said, I still believe exposing wait_task to users of the Django integration would be beneficial. WDYT?

[clemfromspace]

Indeed, as I said above, for some tests we do need to actually call the API, so the wait_task will be useful anyway 👍

[julienbourdeau]

wait_task is now exposed on the client, but note that you need to pass the index name along with th taskID. See commit here: algolia/algoliasearch-client-python@88f9b98

[gregsadetsky]

Would it be a good idea, in registration.py's save_record and delete_record methods, to return the results that those functions get from models.py's save_record and delete_record methods ?

i.e. here replace adapter.save_record(...) by return adapter.save_record(..)? That way, any function calling registration.py's save_record could fetch the taskID and wait for it to complete?

Thanks!

[gregsadetsky]

It'd also be great if models.py's delete_record returned the result it got from delete_object (see here). I did this locally, and then overrode save_record and delete_record in my AlgoliaIndex child class, to add optional calls to self.wait_task (when running in test mode).

Should I open a PR..?

[iamr0b0tx]

I had a similar issue trying to run tests that call the API while waiting synchronously. This was the solution I came up with

1. I created a CustomAlgoliaIndex class. it is a replica of algoliasearch_django models in a file named custom_algolia_index.py in my project directory. I made changes to the save_record and delete_record by adding .wait(). Example below for save_record, delete_record and clear_objects.

from algoliasearch_django import AlgoliaIndex
...

class CustomAlgoliaIndex(AlgoliaIndex):
    ...
    def save_record(self, instance, update_fields=None, **kwargs):
        """Saves the record.

        If `update_fields` is set, this method will use partial_update_object()
        and will update only the given fields (never `_geoloc` and `_tags`).

        For more information about partial_update_object:
        https://github.com/algolia/algoliasearch-client-python#update-an-existing-object-in-the-index
        """
        if not self._should_index(instance):
            # Should not index, but since we don't now the state of the
            # instance, we need to send a DELETE request to ensure that if
            # the instance was previously indexed, it will be removed.
            self.delete_record(instance)
            return

        obj = None

        try:
            if update_fields:
                obj = self.get_raw_record(instance,
                                          update_fields=update_fields)
                result = self.__index.partial_update_object(obj).wait()
            else:
                obj = self.get_raw_record(instance)
                result = self.__index.save_object(obj).wait()
            logger.info('SAVE %s FROM %s', obj['objectID'], self.model)
            return result
        except AlgoliaException as e:
            if DEBUG:
                raise e
            else:
                logger.warning('%s FROM %s NOT SAVED: %s', obj['objectID'],
                               self.model, e)

    def delete_record(self, instance):
        """Deletes the record."""
        objectID = self.objectID(instance)
        try:
            self.__index.delete_object(objectID).wait()
            logger.info('DELETE %s FROM %s', objectID, self.model)
        except AlgoliaException as e:
            if DEBUG:
                raise e
            else:
                logger.warning('%s FROM %s NOT DELETED: %s', objectID,
                               self.model, e)

    def clear_objects(self):
        """Clears all objects of an index."""
        try:
            self.__index.clear_objects().wait()
            logger.info('CLEAR INDEX %s', self.index_name)
        except AlgoliaException as e:
            if DEBUG:
                raise e
            else:
                logger.warning('%s NOT CLEARED: %s', self.model, e)
...

2. I Used the CustomAlgoliaIndex to create my model Index in index.py

from algoliasearch_django.decorators import register

from .models import Company, User
from core.custom_algolia_index import CustomAlgoliaIndex

@register(User)
class UserIndex(CustomAlgoliaIndex):
    fields = ('first_name', 'last_name', 'community', 'company_name')
    settings = {'searchableAttributes': ['first_name', 'last_name', 'community', 'company_name']}
    index_name = 'users'

3. my tests.py file

from algoliasearch_django import algolia_engine, raw_search
from django.db.models.signals import post_save, post_delete
from django.test import TestCase

from core import settings
from users.index import UserIndex, CompanyIndex
from users.models import User, Company, index_object, delete_object_index


# TODO: test company indexing, mock the is_async_available lib
class UsersTestCase(TestCase):
    def setUp(self):
        algolia_settings = dict(settings.ALGOLIA)
        algolia_settings['INDEX_PREFIX'] = 'test'
        algolia_engine.reset(algolia_settings)

        self.model_index_pairs = ((User, UserIndex), (Company, CompanyIndex))
        for model, model_index in self.model_index_pairs:
            algolia_engine.register(model, model_index, False)

            post_save.disconnect(receiver=index_object, sender=model)
            post_delete.disconnect(receiver=delete_object_index, sender=model)

        self.user = User.objects.create(email='[email protected]', first_name='Angelina', last_name='Jolie')

    def tearDown(self):
        for model, model_index in self.model_index_pairs:
            algolia_engine.clear_objects(model)

        algolia_engine.reset(settings.ALGOLIA)

    def test_save_user_index(self):
        algolia_engine.save_record(self.user)

        response = raw_search(User)
        self.assertTrue(any(self.user.last_name == user_dict['last_name'] for user_dict in response['hits']))

    def test_delete_user_index(self):
        algolia_engine.save_record(self.user)
        algolia_engine.delete_record(self.user)

        response = raw_search(User)
        self.assertTrue(not any(self.user.last_name == user_dict['last_name'] for user_dict in response['hits']))

I hope this helps someone!

Note: I am running with AUTO_INDEX=False and my post_save and post_delete signals call a celery task.