Add Elixir and OTP CI matrix and stabilize distributed tests

Author: jeregrine

.github/workflows/ci.yml

@@ -0,0 +1,62 @@
+name: CI
+
+on:
+  push:
+  pull_request:
+
+permissions:
+  contents: read
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  test:
+    name: ${{ matrix.latest && 'Latest stable Elixir / OTP' || format('Elixir {0} / OTP {1}', matrix.elixir, matrix.otp) }}
+    runs-on: ubuntu-24.04
+    timeout-minutes: 15
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - elixir: "1.19"
+            otp: "26"
+          - elixir: "1.19"
+            otp: "27"
+          - elixir: "1.19"
+            otp: "28"
+          - elixir: "1.20"
+            otp: "27"
+          - elixir: "1.20"
+            otp: "28"
+          # Unlike "latest", these ranges exclude prereleases.
+          - elixir: "> 0"
+            otp: "> 0"
+            latest: true
+    env:
+      MIX_ENV: test
+      # Inherited by :peer nodes; bound scheduler usage for distributed tests.
+      ERL_FLAGS: "+S 4:4"
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+
+      - uses: erlef/setup-beam@54075bcc5e249e4758d363f27d099f55d843f124 # v1.24.1
+        with:
+          elixir-version: ${{ matrix.elixir }}
+          otp-version: ${{ matrix.otp }}
+
+      - name: Install dependencies
+        run: mix deps.get
+
+      - name: Check formatting
+        if: matrix.latest
+        run: mix format --check-formatted
+
+      - name: Compile with warnings as errors
+        run: mix compile --warnings-as-errors
+
+      - name: Run local and distributed tests
+        run: mix test --warnings-as-errors

README.md

@@ -400,6 +400,24 @@ mix test
 See [`test/README.md`](test/README.md) for details on the distributed test
 infrastructure.
 
+GitHub Actions runs the full local and distributed suite on pushes and pull
+requests using Elixir 1.19 / OTP 26–28, Elixir 1.20 / OTP 27–28, and the latest
+stable Elixir / OTP pair. Version ranges pick up new patch releases automatically;
+the latest-stable job also picks up new minor and major releases, excluding
+prereleases. Every job treats compilation and test warnings as errors, and the
+latest-stable job checks formatting.
+
+To run the same checks locally:
+
+```bash
+export MIX_ENV=test
+export ERL_FLAGS="+S 4:4"
+mix deps.get
+mix format --check-formatted
+mix compile --warnings-as-errors
+mix test --warnings-as-errors
+```
+
 ## Benchmarks
 
 ```bash

test/README.md

@@ -104,8 +104,10 @@ TestCluster.spawn_register_update_unregister(node_a, :test, "user/1", %{v: 1}, %
 
 `spawn_register` accepts `flush_shards: num_shards` which calls
 `:sys.get_state` on the target shard's GenServer after registration. This
-blocks until all pending messages (nodedown, replicate, etc.) are processed
-on that shard — useful in partition tests where you need to guarantee ordering.
+synchronizes with that shard after the write; it is not a pre-write barrier
+and does not flush buffered replication. Use `TestCluster.flush_shards/2` to
+flush replication and `TestCluster.assert_group_nodes/3` to wait for peer
+discovery or nodedown cleanup on every shard.
 
 ```elixir
 TestCluster.spawn_register(node_a, :test, "key", %{}, flush_shards: 4)
@@ -136,6 +138,9 @@ TestCluster.monitor_nodes_on(node_a, self())
 assert_receive {:nodedown_on_remote, ^node_b}, 5000
 ```
 
+`monitor_nodes_on/2` returns only after the remote monitor is installed.
+Receiving its notification does not mean Group's shards have processed nodedown.
+
 #### Network partitions
 
 ```elixir
@@ -148,6 +153,24 @@ partitions don't work reliably because the test node bridges them — Erlang
 distribution is fully meshed, so if the test node can reach both peers, they
 can reach each other through it.
 
+Before disconnecting, wait for discovery on **every shard**, not just
+`Node.list/0` connectivity. In-flight discovery sends can otherwise trigger
+reconnect retries. After disconnecting, wait for every shard's peer state
+before writing partition-local data:
+
+```elixir
+TestCluster.assert_group_nodes(node_a, :test, [node_b, node_c])
+TestCluster.assert_group_nodes(node_b, :test, [node_a, node_c])
+TestCluster.assert_group_nodes(node_c, :test, [node_a, node_b])
+
+TestCluster.disconnect_nodes(node_c, node_a)
+TestCluster.disconnect_nodes(node_c, node_b)
+
+TestCluster.assert_group_nodes(node_a, :test, [node_b])
+TestCluster.assert_group_nodes(node_b, :test, [node_a])
+TestCluster.assert_group_nodes(node_c, :test, [])
+```
+
 #### Polling for eventual consistency
 
 ```elixir

test/distributed_test.exs

@@ -574,14 +574,11 @@ defmodule Group.DistributedTest do
 
       start_group_on_peers(peers, opts)
 
-      # Wait for Erlang-level connectivity so disconnect_nodes actually works
-      TestCluster.assert_eventually(
-        fn ->
-          c_nodes = TestCluster.rpc!(node_c, Node, :list, [])
-          node_a in c_nodes and node_b in c_nodes
-        end,
-        timeout: 5000
-      )
+      # Finish discovery on every shard before cutting links. Erlang connectivity
+      # alone can leave handshake sends in flight that trigger reconnect retries.
+      TestCluster.assert_group_nodes(node_a, name, [node_b, node_c])
+      TestCluster.assert_group_nodes(node_b, name, [node_a, node_c])
+      TestCluster.assert_group_nodes(node_c, name, [node_a, node_b])
 
       # Set up nodedown monitor on A
       TestCluster.monitor_nodes_on(node_a, self())
@@ -593,8 +590,12 @@ defmodule Group.DistributedTest do
       # Wait for A to confirm it saw C go down
       assert_receive {:nodedown_on_remote, ^node_c}, 5000
 
+      # A separate node monitor does not establish that every shard handled DOWN.
+      TestCluster.assert_group_nodes(node_a, name, [node_b])
+      TestCluster.assert_group_nodes(node_b, name, [node_a])
+      TestCluster.assert_group_nodes(node_c, name, [])
+
       # While partitioned: register keys on A, join groups on C
-      # flush_shards ensures nodedown is processed before registering
       TestCluster.spawn_register(node_a, name, "user/from_a", %{origin: :a}, flush_shards: 2)
       TestCluster.spawn_join(node_c, name, "room/from_c", %{origin: :c})
 
@@ -1970,14 +1971,10 @@ defmodule Group.DistributedTest do
         length(nodes) >= 1
       end)
 
-      # Wait for Erlang-level connectivity so disconnect_nodes actually works
-      TestCluster.assert_eventually(
-        fn ->
-          c_nodes = TestCluster.rpc!(node_c, Node, :list, [])
-          node_a in c_nodes and node_b in c_nodes
-        end,
-        timeout: 5000
-      )
+      # Wait for Group discovery, not just Erlang connectivity, before partitioning.
+      TestCluster.assert_group_nodes(node_a, name, [node_b, node_c])
+      TestCluster.assert_group_nodes(node_b, name, [node_a, node_c])
+      TestCluster.assert_group_nodes(node_c, name, [node_a, node_b])
 
       # Set up nodedown monitors on A before partitioning
       TestCluster.monitor_nodes_on(node_a, self())
@@ -1990,9 +1987,12 @@ defmodule Group.DistributedTest do
       assert_receive {:nodedown_on_remote, ^node_c}, 5000
 
       # Wait for Group's own peer tables to reflect the partition before writing.
+      TestCluster.assert_group_nodes(node_a, name, [node_b])
+      TestCluster.assert_group_nodes(node_b, name, [node_a])
+      TestCluster.assert_group_nodes(node_c, name, [])
+
       TestCluster.assert_eventually(fn ->
-        node_c not in TestCluster.rpc!(node_a, Group, :nodes, [name]) and
-          node_c not in TestCluster.rpc!(node_a, Group, :nodes, [name, "game"])
+        node_c not in TestCluster.rpc!(node_a, Group, :nodes, [name, "game"])
       end)
 
       # Register data during partition

test/support/test_cluster.ex

@@ -11,7 +11,7 @@ defmodule Group.TestCluster do
         Enum.flat_map(code_paths, fn p -> [~c"-pa", p] end)
 
     for _i <- 1..count do
-      name = :"peer#{System.unique_integer([:positive])}"
+      name = :"peer_#{System.pid()}_#{System.unique_integer([:positive])}"
 
       {:ok, pid, node} =
         :peer.start(%{name: name, host: ~c"127.0.0.1", longnames: true, args: args})
@@ -302,10 +302,20 @@ defmodule Group.TestCluster do
   @doc "Monitor nodedown events from a remote node, forwarding to caller"
   def monitor_nodes_on(node, target_pid) do
     :erpc.call(node, fn ->
-      spawn(fn ->
-        :net_kernel.monitor_nodes(true)
-        forward_nodedown(target_pid)
-      end)
+      parent = self()
+
+      pid =
+        spawn(fn ->
+          :net_kernel.monitor_nodes(true)
+          send(parent, {:monitor_ready, self()})
+          forward_nodedown(target_pid)
+        end)
+
+      receive do
+        {:monitor_ready, ^pid} -> pid
+      after
+        5000 -> raise "monitor_nodes_on timed out"
+      end
     end)
   end
 
@@ -397,6 +407,23 @@ defmodule Group.TestCluster do
     end)
   end
 
+  @doc "Wait for every shard and the shared peer table to agree on the expected Group peers."
+  def assert_group_nodes(node, name, expected_nodes) do
+    expected_nodes = Enum.sort(expected_nodes)
+
+    assert_eventually(fn ->
+      :erpc.call(node, fn ->
+        num_shards = Group.get_config(name).num_shards
+
+        Enum.sort(Group.nodes(name)) == expected_nodes and
+          Enum.all?(0..(num_shards - 1), fn shard ->
+            state = :sys.get_state(:"#{name}_replica_#{shard}")
+            Enum.sort(Map.keys(state.remote_shards)) == expected_nodes
+          end)
+      end)
+    end)
+  end
+
   @doc "Returns the current message_queue_len for a shard on a remote node."
   def shard_message_queue_len(node, name, shard) do
     :erpc.call(node, __MODULE__, :do_shard_message_queue_len, [name, shard])

test/test_helper.exs

@@ -14,7 +14,9 @@ unless epmd_running?.() do
 end
 
 unless Node.alive?() do
-  {:ok, _} = Node.start(:"test_#{System.unique_integer([:positive])}@127.0.0.1", :longnames)
+  # unique_integer is VM-local; include the OS pid to allow concurrent test runs.
+  name = :"test_#{System.pid()}_#{System.unique_integer([:positive])}@127.0.0.1"
+  {:ok, _} = Node.start(name, :longnames)
   Node.set_cookie(:group_test)
 end