Speed up tests

[ellnix]

In preparation for adding more tests, which itself is in preparation for some serious refactoring.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.54%. Comparing base (5450df2) to head (54e8edc).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #408   +/-   ##
=======================================
  Coverage   89.54%   89.54%           
=======================================
  Files          13       13           
  Lines         775      775           
=======================================
  Hits          694      694           
  Misses         81       81           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[wesharper]

Overall, looks good! I've had to go through and make similar changes a handful of times for our own tests. Little stuff like this can make all the difference.

[wesharper]

What exactly did the true do? Is this just a style cleanup or are there perf implications?

[ellnix]

clear_index! and a lot of other methods in this gem have a single argument that is a synchronous flag, i. e. whether to await the resulting task. This absolutely has performance implications (although not all that many since awaiting is not really avoidable).

I often write descriptions in git commits, here's the one on the commit of this change:

Unnecessary awaits should be avoided, generally these methods are meant
to be called at the beginning of the test and for best performance we
should wait on tasks right before an assertion.

So the performance implication is being lazy and putting off the await for as long as possible.

[ellnix]

Also confusion like this is why I really don't like flags, consider this method signature:

      def ms_reindex!(batch_size = MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, synchronous = false)

Not only is the first argument just a magic number (in most cases), but the second argument is a flag. And because we do not encourage the use of await, we have the flag come second so specifying only the flag takes this form:

Color.ms_reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true)

I would argue this does not look great.

In concert with a specific batch size, you get this borderline incomprehensible method call:

Color.ms_reindex!(50, true)

In conclusion, I would really like to change these methods :)

[wesharper]

Makes sense, thanks for the writeup. Agreed on the method signature. kwargs are definitely preferred IMO as well.