Skip to content

refactor(tests): Fix flaky ItemizableUpdateService test #5479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sergio-Mira wants to merge 1 commit into
base: main
Choose a base branch
from
+39 −40
Open

refactor(tests): Fix flaky ItemizableUpdateService test #5479

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 39 additions & 40 deletions spec/services/itemizable_update_service_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
let(:item1) { create(:item, organization: organization, name: "My Item 1") }
let(:item2) { create(:item, organization: organization, name: "My Item 2") }
let(:item3) { create(:item, organization: organization, name: "My Item 3") }
let(:two_days_ago) { 2.days.ago } # Store exact timestamp to be able to assert later
before(:each) do
TestInventory.create_inventory(storage_location.organization, {
storage_location.id => {
Expand All @@ -18,12 +19,6 @@
})
end

around(:each) do |ex|
freeze_time do
ex.run
end
end

describe "increases" do
let(:itemizable) do
line_items = [
Expand All @@ -39,12 +34,12 @@

let(:attributes) do
{
issued_at: 2.days.ago,
issued_at: two_days_ago,
line_items_attributes: {"0": {item_id: item1.id, quantity: 2}, "1": {item_id: item2.id, quantity: 2}}
}
end

subject do
def call_update_service
described_class.call(itemizable: itemizable,
params: attributes,
event_class: DonationEvent)
Expand All @@ -53,19 +48,19 @@
it "should update quantity in same storage location" do
expect(storage_location.size).to eq(20)
expect(new_storage_location.size).to eq(20)
subject
call_update_service
expect(itemizable.reload.line_items.count).to eq(2)
expect(itemizable.line_items.sum(&:quantity)).to eq(4)
expect(storage_location.size).to eq(14)
expect(new_storage_location.size).to eq(20)
expect(itemizable.issued_at).to eq(2.days.ago)
expect(itemizable.issued_at.to_fs(:db)).to eq(two_days_ago.to_fs(:db)) # Use to_fs to ignore nanosecond differences
expect(UpdateExistingEvent.count).to eq(1)
end

it "fails with a validation message for donation events with invalid issued_at" do
attributes[:issued_at] = ""

expect { subject }.to raise_error do |e|
expect { call_update_service }.to raise_error do |e|
expect(e).to be_a(ActiveRecord::RecordInvalid)
expect(e.message).to eq("Validation failed: Issue date can't be blank")
end
Expand All @@ -75,7 +70,7 @@
context "when there is no intervening audit" do
it "should update quantity in different locations" do
attributes[:storage_location_id] = new_storage_location.id
subject
call_update_service
expect(itemizable.reload.line_items.count).to eq(2)
expect(itemizable.line_items.sum(&:quantity)).to eq(4)
expect(storage_location.size).to eq(10)
Expand All @@ -89,15 +84,15 @@
"If you need to change the storage location, please delete this donation and create a new donation with the new storage location."
create(:audit, :with_items, item: itemizable.items.first, organization: organization, storage_location: storage_location, status: "finalized")
attributes[:storage_location_id] = new_storage_location.id
expect { subject }.to raise_error(msg)
expect { call_update_service }.to raise_error(msg)
end
end
end

it "should raise an error if any item is inactive" do
item1.update!(active: false)
msg = "Update failed: The following items are currently inactive: My Item 1. Please reactivate them before continuing."
expect { subject }.to raise_error(msg)
expect { call_update_service }.to raise_error(msg)
end
end

Expand All @@ -116,30 +111,30 @@

let(:attributes) do
{
issued_at: 2.days.ago,
issued_at: two_days_ago,
line_items_attributes: {"0": {item_id: item1.id, quantity: 2}, "1": {item_id: item2.id, quantity: 2}}
}
end

subject do
def call_update_service
described_class.call(itemizable: itemizable, params: attributes, event_class: DistributionEvent)
end

it "should update quantity in same storage location" do
expect(storage_location.size).to eq(20)
expect(new_storage_location.size).to eq(20)
subject
call_update_service
expect(itemizable.reload.line_items.count).to eq(2)
expect(itemizable.line_items.sum(&:quantity)).to eq(4)
expect(storage_location.size).to eq(26)
expect(new_storage_location.size).to eq(20)
expect(itemizable.issued_at).to eq(2.days.ago)
expect(itemizable.issued_at.to_fs(:db)).to eq(two_days_ago.to_fs(:db)) # Use to_fs to ignore nanosecond differences
Copy link
Author

@Sergio-Mira Sergio-Mira Jan 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

end

it "fails with a validation message for distribution events with invalid issued_at" do
attributes[:issued_at] = ""

expect { subject }.to raise_error do |e|
expect { call_update_service }.to raise_error do |e|
expect(e).to be_a(ActiveRecord::RecordInvalid)
expect(e.message).to eq("Validation failed: Distribution date and time can't be blank")
end
Expand All @@ -149,7 +144,7 @@
context "when there is no intervening audit" do
it "should update quantity in different locations" do
attributes[:storage_location_id] = new_storage_location.id
subject
call_update_service
expect(itemizable.reload.line_items.count).to eq(2)
expect(itemizable.line_items.sum(&:quantity)).to eq(4)
expect(storage_location.size).to eq(30)
Expand All @@ -163,15 +158,15 @@
"If you need to change the storage location, please reclaim this distribution and create a new distribution from the new storage location."
create(:audit, :with_items, item: itemizable.items.first, organization: organization, storage_location: storage_location, status: "finalized")
attributes[:storage_location_id] = new_storage_location.id
expect { subject }.to raise_error(msg)
expect { call_update_service }.to raise_error(msg)
end
end
end

it "should raise an error if any item is inactive" do
item1.update!(active: false)
msg = "Update failed: The following items are currently inactive: My Item 1. Please reactivate them before continuing."
expect { subject }.to raise_error(msg)
expect { call_update_service }.to raise_error(msg)
end
end

Expand All @@ -188,12 +183,14 @@
line_items: line_items,
issued_at: 1.day.ago)
end

let(:attributes) do
{
issued_at: 2.days.ago,
issued_at: two_days_ago,
line_items_attributes: {"0": {item_id: item1.id, quantity: 5}, "1": {item_id: item3.id, quantity: 50}}
}
end

it "should send an itemizable event if it already exists" do
DonationEvent.publish(itemizable)
expect(DonationEvent.count).to eq(1)
Expand All @@ -216,14 +213,8 @@
expect(View::Inventory.total_inventory(organization.id)).to eq(75) # 40 - 5 (item1) - 10 (item2) + 50 (item3)
end
end

describe "with distributions" do
before(:each) do
TestInventory.create_inventory(storage_location.organization, {
storage_location.id => {
item3.id => 10
}
})
end
let(:itemizable) do
line_items = [
create(:line_item, item_id: item1.id, quantity: 5),
Expand All @@ -235,12 +226,22 @@
line_items: line_items,
issued_at: 1.day.ago)
end

let(:attributes) do
{
issued_at: 2.days.ago,
issued_at: two_days_ago,
line_items_attributes: {"0": {item_id: item1.id, quantity: 2}, "1": {item_id: item3.id, quantity: 6}}
}
end

before(:each) do
TestInventory.create_inventory(storage_location.organization, {
storage_location.id => {
item3.id => 10
}
})
end

it "should send an itemizable event if it already exists" do
DistributionEvent.publish(itemizable)
expect(DistributionEvent.count).to eq(1)
Expand All @@ -264,10 +265,9 @@
end

it "should raise an error if there is an intervening snapshot" do
itemizable.save!
travel(-1.week)
SnapshotEvent.publish(organization)
travel 1.week
travel(-1.week) do
SnapshotEvent.publish(organization)
end
itemizable.update!(created_at: 2.weeks.ago)
expect do
described_class.call(itemizable: itemizable, params: attributes, event_class: DistributionEvent)
Expand All @@ -276,16 +276,15 @@

it "should not raise an error if no inventory was changed" do
no_change_attrs = {
issued_at: 2.days.ago,
issued_at: two_days_ago,
line_items_attributes: {
"0": {item_id: item1.id, quantity: 10},
"1": {item_id: item2.id, quantity: 10}
}
}
itemizable.save!
travel(-1.week)
SnapshotEvent.publish(organization)
travel 1.week
travel(-1.week) do
SnapshotEvent.publish(organization)
end
itemizable.update!(created_at: 2.weeks.ago)
expect do
described_class.call(itemizable: itemizable, params: no_change_attrs, event_class: DistributionEvent)
Expand Down