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:

• [x] exposing it on the Client and not only on the index, see PR algolia/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 :100:, let's stop duplicating E2E tests for the sake of it. I'm all in for mocking everywhere we can! :slightly_smiling_face:

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: https://github.com/algolia/algoliasearch-client-python/pull/373/commits/88f9b98c9b4d5b383f0f138a2a8d74cf0b23d8b6

[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='test@test.com', 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.__