Skip to content

Commit 18e9a7a

Browse files
authored
chore(sync-service): Fix flakey Publisher tests (#2851)
These tests can occasionally fail with: ``` Assertion failed, no matching message after 5000ms The process mailbox is empty. code: assert_receive :publish_finished ``` The race condition occurs if the TestSubscriber receives the `:finish_processing_message` message before it receives the publisher call, in which case it swallows the message since `handle_info` is not implemented on TestSubscriber. A GenServer would log that a message had been received and not handled, but as `use GenServer` was omitted from the TestSubscriber the message was swallowed silently. This PR: - Adds `use GenServer` to the TestSubscriber as this is best practice (the race condition would have been obvious had it been included in the first place) - Waits for `:message_received` from each of the subscribers before sending `:finish_processing_message`. This eliminates the race condition. - Renames `event` to `message` for consistency Credit to ChatGPT which worked out what the race condition was. It's solution was terrible, but the hard part was knowing what the race condition was, so that was super helpful.
1 parent 970431a commit 18e9a7a

File tree

1 file changed

+29
-15
lines changed

1 file changed

+29
-15
lines changed

packages/sync-service/test/electric/publisher_test.exs

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ defmodule Electric.PublisherTest do
55
alias Electric.PublisherTest.TestSubscriber
66

77
defmodule TestSubscriber do
8+
use GenServer
9+
810
def start_link(on_message) do
911
GenServer.start_link(__MODULE__, on_message)
1012
end
@@ -32,38 +34,47 @@ defmodule Electric.PublisherTest do
3234
end
3335

3436
test "does not return until all subscibers have processed the message" do
35-
on_event = fn :test_message ->
37+
pid = self()
38+
39+
on_message = fn :test_message ->
40+
send(pid, :message_received)
41+
3642
receive do
37-
:finish_processing_event -> :ok
43+
:finish_processing_message -> :ok
3844
end
3945
end
4046

41-
{:ok, sub1} = TestSubscriber.start_link(on_event)
42-
{:ok, sub2} = TestSubscriber.start_link(on_event)
43-
44-
pid = self()
47+
{:ok, sub1} = TestSubscriber.start_link(on_message)
48+
{:ok, sub2} = TestSubscriber.start_link(on_message)
4549

4650
Task.async(fn ->
4751
Publisher.publish([sub1, sub2], :test_message)
4852
send(pid, :publish_finished)
4953
end)
5054

55+
assert_receive :message_received
56+
assert_receive :message_received
57+
5158
refute_receive :publish_finished, 10
52-
send(sub2, :finish_processing_event)
59+
send(sub2, :finish_processing_message)
5360
refute_receive :publish_finished, 10
54-
send(sub1, :finish_processing_event)
55-
assert_receive :publish_finished, 5000
61+
send(sub1, :finish_processing_message)
62+
assert_receive :publish_finished
5663
end
5764

5865
test "does not return until all subscibers have processed the message or died" do
59-
on_event = fn :test_message ->
66+
pid = self()
67+
68+
on_message = fn :test_message ->
69+
send(pid, :message_received)
70+
6071
receive do
61-
:finish_processing_event -> :ok
72+
:finish_processing_message -> :ok
6273
end
6374
end
6475

65-
{:ok, sub1} = TestSubscriber.start_link(on_event)
66-
{:ok, sub2} = TestSubscriber.start_link(on_event)
76+
{:ok, sub1} = TestSubscriber.start_link(on_message)
77+
{:ok, sub2} = TestSubscriber.start_link(on_message)
6778

6879
pid = self()
6980

@@ -72,12 +83,15 @@ defmodule Electric.PublisherTest do
7283
send(pid, :publish_finished)
7384
end)
7485

86+
assert_receive :message_received
87+
assert_receive :message_received
88+
7589
refute_receive :publish_finished, 10
7690
Process.unlink(sub2)
7791
Process.exit(sub2, :kill)
7892
refute_receive :publish_finished, 10
79-
send(sub1, :finish_processing_event)
80-
assert_receive :publish_finished, 5000
93+
send(sub1, :finish_processing_message)
94+
assert_receive :publish_finished
8195
end
8296
end
8397
end

0 commit comments

Comments
 (0)