Flaky Itemizable test

[dorner]

Summary

The following test only fails sometimes: https://github.com/rubyforgood/human-essentials/actions/runs/9780748669/job/27003147071?pr=4488

 1) ItemizableUpdateService events with distributions should send an itemizable event if it already exists
     Failure/Error:
       raise InventoryError.new("Could not reduce quantity by #{quantity} - current quantity is #{current_quantity}",
         item_id,
         id)

     InventoryError:
       Could not reduce quantity by 5 - current quantity is 0 for My Item 1 in Smithsonian Conservation Center
     # ./app/events/event_types/event_storage_location.rb:35:in `reduce_inventory'
     # ./app/events/event_types/inventory.rb:45:in `move_item'
     # ./app/events/inventory_aggregate.rb:71:in `block in handle_inventory_event'
     # ./app/events/inventory_aggregate.rb:65:in `each'
     # ./app/events/inventory_aggregate.rb:65:in `handle_inventory_event'
     # ./app/events/inventory_aggregate.rb:114:in `block in <module:InventoryAggregate>'
     # ./app/events/inventory_aggregate.rb:57:in `handle'
     # ./app/events/inventory_aggregate.rb:42:in `block in inventory_for'
     # ./app/events/inventory_aggregate.rb:33:in `each'
     # ./app/events/inventory_aggregate.rb:33:in `inventory_for'
     # ./app/models/event.rb:84:in `validate_inventory'
     # ./app/events/distribution_event.rb:4:in `publish'
     # ./spec/services/itemizable_update_service_spec.rb:234:in `block (4 levels) in <top (required)>'
     # ./spec/services/itemizable_update_service_spec.rb:23:in `block (3 levels) in <top (required)>'
     # ./spec/services/itemizable_update_service_spec.rb:22:in `block (2 levels) in <top (required)>'
     
     Randomized with seed 18466

Things to consider

No response

Criteria for Completion

No response

[coalest]

I took a crack at figuring this out but was unsuccessful. Just commenting in case it helps someone else in the future.

I ran the test suite with --bisect and the given seed on the commit that this occurred on and wasn't able to reproduce the failure.

I also looked through the code a bit. At first I thought it might be flaky because there are two rspec before blocks during setup that create inventory (TestInventory#create_inventory) for the same storage location and that method has a comment saying it "Blows away any current inventory for the storage locations." But it turns out to only wipe current inventory for the given items (in a storage location) passed in. So I'm not sure where the issue is.

Best of luck future contributor.

[McEileen]

Hi, I'd be interested in working on this issue.

I was able to locally reproduce the failure by running the individual spec 400 times. The flake appears to occur when the from_location has no items associated with it, and reduce_inventory method sets current_quantity to equal 0.

I haven't yet figured out why the from_location has no items, but I look forward to investigating!

[cielf]

It's yours -- Good hunting!

[github-actions]

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

[McEileen]

Hi, I am having a tough time making progress. As previously noted, I was able to reproduce the error and note that it occurs in the event_types/inventory’s move_item method. When the event storage_locations[from_location] doesn’t have any items associated with it, the reduce_inventory method sets item current_quantity to 0. and the error seen in the flaky test occurs.

I can’t figure out why storage_locations[from_location] doesn’t have items associated with it. I couldn’t find any bug in test inventory setup. When I added puts statements to see the items in the test inventory, there was no discrepancy between the tests that passed and the tests that flaked. I still tried to combine the two before blocks into one, which can be seen on my branch here. The test continued to flake with this change.

I did notice that the itemizable_update_service method update_storage_location deletes all line items. But it then updates the distribution with the line items attributes in the params. So, I can’t think of how that would contribute to this flake.

I’d appreciate input from anyone who’s more familiar with the code or has an idea what might be causing this flake. If I don’t make progress by this upcoming Monday, I’ll remove myself from the issue.

[cielf]

As an aside -- Not a code familiarity insight per se. Storage locations are initially created with no items.. but you should never get a "from" storage location with no items at all in it in the wild, because you can't select a storage location with no items when entering a distribution.

[cielf]

@dorner -- this is definitely in your bailiwick.

[dorner]

Nothing jumps out at me. Sometimes there needs to be reload statements in judicious places to ensure the data is correct.

[McEileen]

I haven't made progress since posting and I am going to step aside. Could someone please unassign me from this issue? Thanks!