refactor(tests): Fix flaky ItemizableUpdateService test

[Sergio-Mira]

Resolves #4496

Description

Fix flaky ItemizableUpdateService test. The issue is caused by the use of Timecop and having order dependence on Event entries in the logic that are ordered by event_time, updated_at.
Looks like that given some specific seeds the order of entries into the DB with the exact same timestamp would then sometimes result in out-of-order (vs entry order) events being returned that would be processed in InventoryAggregate:inventory_for in the wrong order, causing unexpected results as seen in the screenshot.

Recommendation is to not freeze time unless you want the time to be consistent across the test execution, I have instead removed the global time freeze and used it as a block on specific tests to mock Time.now similarly to what was intended before.

Also replaced usage of subject with a method definition as subject is normally and object not a method call, but can revert this change.

Type of change

• Tests refactor

How Has This Been Tested?

• Test suite passes
• Test suite tested with broken seed for thousands of runs

Screenshots

How the issue was found

• Make sure we have a fast loop dev time (Disable SimpleCov by commenting it).
• Use a script to run the specific spec for thousands of times until we reach a failure. Note down seed.
• Having found a seed that reproduces the issue (or similar issue in this case) run with --bisect to try and trim down to the least specs possible. In this case surprisingly it was all tests although manual edits and runs later proved that it could be trimmed down a bit.

bundle exec rspec spec/services/itemizable_update_service_spec.rb --seed 38075 --fail-fast

• Start digging back from the point of failure (current_quantity being zero) by debugging the test failure using a spec runner in VSCode
• Add debugging outputs and compare runs with breaking seed and any other seed that made the test pass
• Reached the method in InventoryAggregate:inventory_for where events were being outputted in a different order
• Checked logic around ordering an noted down it was order(:event_time, :updated_at) (note no id in it)
• Read about time precision and Timecop influence on it https://stevenharman.net/on-flaky-tests-time-precision-and-order-dependence
• Disable Timecop for that test and prove it passes with breaking seed
• Refactor tests to use Timecop in a different way

It was pretty hard to find, got lucky by getting a seed that would cause a similar issue consistently.

[dorner]

Unfortunately this doesn't seem to work - it's causing failures on CI right now 😦 https://github.com/rubyforgood/human-essentials/actions/runs/21137813115/job/61316145366?pr=5479

[Sergio-Mira]

Thanks @dorner had a look at the failure and there can be small differences when loading the exact same date from the DB, looked at alternative ways to check the same date and applied them to the assert.

[Sergio-Mira]

Can use be_within(1.second) alternatively as I see it used in other specs

[dorner]

@Sergio-Mira I don't see the purpose in removing the freeze_time call. It can only make things more reliable. The fixes should largely be around the order of execution, not relying on time to be a particular value.