Skip to content

Commit 25524bb

Browse files
committed
Try adding doctor and dialyzer
1 parent fa5dcf1 commit 25524bb

14 files changed

+248
-18
lines changed

.doctor.exs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
%Doctor.Config{
2+
exception_moduledoc_required: true,
3+
failed: false,
4+
ignore_modules: [
5+
~r/^PlaygroundWeb.*$/,
6+
~r/^Playground.Release$/
7+
],
8+
ignore_paths: [],
9+
min_module_doc_coverage: 80,
10+
min_module_spec_coverage: 50,
11+
min_overall_doc_coverage: 80,
12+
min_overall_spec_coverage: 50,
13+
raise: false,
14+
reporter: Doctor.Reporters.Full,
15+
struct_type_spec_required: true,
16+
umbrella: true
17+
}

.github/workflows/check.yml

+115-11
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ name: Checks
44
# Sets the ENV `MIX_ENV` to `test` for running tests
55
env:
66
MIX_ENV: test
7+
# Artificially refresh the cache
8+
DEPENDENCIES_CACHE_VERSION: 1
9+
PLT_CACHE_VERSION: 1
710

811
permissions:
912
contents: read
@@ -56,9 +59,81 @@ jobs:
5659
cache-name: cache-elixir-deps
5760
with:
5861
path: deps
59-
key: ${{ runner.os }}-mix-${{ env.cache-name }}-${{ hashFiles('**/mix.lock') }}
62+
key: ${{ runner.os }}-mix-${{ env.cache-name }-${{ env.DEPENDENCIES_CACHE_VERSION }}}-${{ hashFiles('**/mix.lock') }}
6063
restore-keys: |
61-
${{ runner.os }}-mix-${{ env.cache-name }}-
64+
${{ runner.os }}-mix-${{ env.cache-name }}-${{ env.DEPENDENCIES_CACHE_VERSION }}-
65+
66+
# Step: Define how to cache the `_build` directory. After the first run,
67+
# this speeds up tests runs a lot. This includes not re-compiling our
68+
# project's downloaded deps every run.
69+
- name: Cache compiled build
70+
id: cache-build
71+
uses: actions/cache@v3
72+
env:
73+
cache-name: cache-compiled-build
74+
with:
75+
path: _build
76+
key: ${{ runner.os }}-mix-${{ env.cache-name }}-${{ env.DEPENDENCIES_CACHE_VERSION }}-${{ hashFiles('**/mix.lock') }}
77+
restore-keys: |
78+
${{ runner.os }}-mix-${{ env.cache-name }}-${{ env.DEPENDENCIES_CACHE_VERSION }}-
79+
${{ runner.os }}-mix-
80+
81+
# Step: Conditionally bust the cache when job is re-run. Sometimes, we
82+
# may have issues with incremental builds that are fixed by doing a full
83+
# recompile. In order to not waste dev time on such trivial issues (while
84+
# also reaping the time savings of incremental builds for *most*
85+
# day-to-day development), force a full recompile only on builds that are
86+
# retried.
87+
- name: Clean to rule out incremental build as a source of flakiness
88+
if: github.run_attempt != '1'
89+
run: |
90+
mix deps.clean --all
91+
mix clean
92+
shell: sh
93+
94+
# Step: Download project dependencies. If unchanged, uses the cached
95+
# version.
96+
- name: Install dependencies
97+
run: mix deps.get
98+
99+
# Step: Compile the project treating any warnings as errors.
100+
- name: Compiles without warnings
101+
run: mix compile --warnings-as-errors
102+
103+
# Step: Setup the database for the tests.
104+
- name: Setup db
105+
run: mix ecto.setup
106+
107+
# Step: Execute the tests.
108+
- name: Run tests
109+
run: mix test
110+
111+
ensure_code_consistency:
112+
runs-on: ubuntu-latest
113+
name: Test on OTP ${{matrix.otp}} / Elixir ${{matrix.elixir}}
114+
steps:
115+
# Step: Setup Elixir + Erlang image as the base.
116+
- name: Set up Elixir
117+
uses: erlef/setup-beam@v1
118+
with:
119+
otp-version: ${{matrix.otp}}
120+
elixir-version: ${{matrix.elixir}}
121+
122+
# Step: Check out the code.
123+
- name: Checkout code
124+
uses: actions/checkout@v3
125+
126+
# Step: Define how to cache deps. Restores existing cache if present.
127+
- name: Cache deps
128+
id: cache-deps
129+
uses: actions/cache@v3
130+
env:
131+
cache-name: cache-elixir-deps
132+
with:
133+
path: deps
134+
key: ${{ runner.os }}-mix-${{ env.cache-name }}-${{ env.DEPENDENCIES_CACHE_VERSION }}-${{ hashFiles('**/mix.lock') }}
135+
restore-keys: |
136+
${{ runner.os }}-mix-${{ env.cache-name }}-${{ env.DEPENDENCIES_CACHE_VERSION }}-
62137
63138
# Step: Define how to cache the `_build` directory. After the first run,
64139
# this speeds up tests runs a lot. This includes not re-compiling our
@@ -86,7 +161,7 @@ jobs:
86161
run: |
87162
mix deps.clean --all
88163
mix clean
89-
shell: sh
164+
shell: s
90165

91166
# Step: Download project dependencies. If unchanged, uses the cached
92167
# version.
@@ -97,18 +172,47 @@ jobs:
97172
- name: Compiles without warnings
98173
run: mix compile --warnings-as-errors
99174

100-
# Step: Check that the checked in code has already been formatted.
175+
# Step: Check that the code has already been formatted.
101176
- name: Check Formatting
102177
run: mix format --check-formatted
103178

104-
# Step: Check that the checked in code complies with the credo rules.
179+
# Step: Check that the code complies with the credo rules.
105180
- name: Check Credo
106181
run: mix credo -A
107182

108-
# Step: Setup the database for the tests.
109-
- name: Setup db
110-
run: mix ecto.setup
183+
# Step: Check that the documentation and specs complies with the doctor rules.
184+
- name: Check Doctor
185+
run: mix doctor
111186

112-
# Step: Execute the tests.
113-
- name: Run tests
114-
run: mix test
187+
# Don't cache PLTs based on mix.lock hash, as Dialyzer can incrementally update even old ones
188+
# Cache key based on Elixir & Erlang version (also useful when running in matrix)
189+
- name: Restore PLT cache
190+
uses: actions/cache/restore@v4
191+
id: plt_cache
192+
with:
193+
key: |
194+
plt-${{ runner.os }}-${{ env.PLT_CACHE_VERSION }}
195+
restore-keys: |
196+
plt-${{ runner.os }}-${{ env.PLT_CACHE_VERSION }}
197+
path: |
198+
priv/plts
199+
200+
# Create PLTs if no cache was found
201+
- name: Create PLTs
202+
if: steps.plt_cache.outputs.cache-hit != 'true'
203+
run: mix dialyzer --plt
204+
205+
# By default, the GitHub Cache action will only save the cache if all steps in the job succeed,
206+
# so we separate the cache restore and save steps in case running dialyzer fails.
207+
- name: Save PLT cache
208+
uses: actions/cache/save@v4
209+
if: steps.plt_cache.outputs.cache-hit != 'true'
210+
id: plt_cache_save
211+
with:
212+
key: |
213+
plt-${{ runner.os }}-${{ env.PLT_CACHE_VERSION }}
214+
path: |
215+
priv/plts
216+
217+
- name: Run dialyzer
218+
run: mix dialyzer --format raw

lib/playground/db/game.ex

+4-2
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,17 @@ defmodule Playground.DB.Game do
1414
timestamps(type: :utc_datetime)
1515
end
1616

17-
@doc false
17+
@doc "Creates a new game"
18+
@spec create_changeset(Playground.DB.Game.t(), map) :: Ecto.Changeset.t()
1819
def create_changeset(game, attrs) do
1920
game
2021
|> cast(attrs, [:state, :type, :room_id])
2122
|> cast(%{status: :active}, [:status])
2223
|> validate_required([:state, :status, :type, :room_id])
2324
end
2425

25-
@doc false
26+
@doc "Updates a game status and/or state"
27+
@spec update(Playground.DB.Game.t(), map) :: {:ok, Playground.DB.Game.t()}
2628
def update(game, attr) do
2729
game
2830
|> cast(attr, [:status, :state])

lib/playground/db/player.ex

+4-2
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,16 @@ defmodule Playground.DB.Player do
1717
timestamps(type: :utc_datetime)
1818
end
1919

20-
@doc false
20+
@doc "Creates a new player"
21+
@spec changeset(Playground.DB.Player.t(), map) :: Ecto.Changeset.t()
2122
def changeset(player, attrs) do
2223
player
2324
|> cast(attrs, [:name, :room_id])
2425
|> validate_required([:name, :room_id])
2526
end
2627

27-
@doc false
28+
@doc "Get a player by the given id"
29+
@spec get_player_by_id(integer) :: Playground.DB.Player.t() | nil
2830
def get_player_by_id(id) do
2931
Repo.one(Query.from(p in __MODULE__, where: p.id == ^id))
3032
end

lib/playground/db/room.ex

+6
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ defmodule Playground.DB.Room do
2121
Change set to create a room.
2222
Validate the code is 4 characters long.
2323
"""
24+
@spec create_changeset(Playground.DB.Room.t(), map) :: Ecto.Changeset.t()
2425
def create_changeset(room, attrs) do
2526
room
2627
|> cast(attrs, [:code, :status])
@@ -31,6 +32,7 @@ defmodule Playground.DB.Room do
3132
@doc """
3233
Set the host of the room
3334
"""
35+
@spec set_host_changeset(Playground.DB.Room.t(), map) :: Ecto.Changeset.t()
3436
def set_host_changeset(room, attrs) do
3537
room
3638
|> cast(attrs, [:host_id])
@@ -41,6 +43,7 @@ defmodule Playground.DB.Room do
4143
@doc """
4244
Update the status of the room
4345
"""
46+
@spec set_status_changeset(Playground.DB.Room.t(), map) :: Ecto.Changeset.t()
4447
def set_status_changeset(room, attrs) do
4548
room
4649
|> cast(attrs, [:status])
@@ -50,20 +53,23 @@ defmodule Playground.DB.Room do
5053
@doc """
5154
Returns a query on Playground.DB.Room, with :room as a named binding.
5255
"""
56+
@spec base_query() :: Ecto.Query.t()
5357
def base_query do
5458
Query.from(r in __MODULE__, as: :room)
5559
end
5660

5761
@doc """
5862
Returns a query on Playground.DB.Room, where the room has the given code.
5963
"""
64+
@spec where_code(Ecto.Query.t(), String.t()) :: Ecto.Query.t()
6065
def where_code(query, code) do
6166
Query.where(query, [room: r], r.code == ^code)
6267
end
6368

6469
@doc """
6570
Returns a query on Playground.DB.Room, where the room is active.
6671
"""
72+
@spec where_is_active(Ecto.Query.t()) :: Ecto.Query.t()
6773
def where_is_active(query) do
6874
Query.where(query, [room: r], r.status in [:open, :playing])
6975
end

lib/playground/engine.ex

+16
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,32 @@ defmodule Playground.Engine do
77
alias Playground.RoomProcess
88
alias Playground.Rooms
99

10+
@doc """
11+
Starts the playground engine that drives all room
12+
"""
13+
@spec start_link(map) :: {:ok, pid} | {:error, term}
1014
def start_link(init_arg) do
1115
case DynamicSupervisor.start_link(__MODULE__, init_arg, name: __MODULE__) do
1216
{:ok, pid} -> {:ok, pid}
1317
{:error, {:already_started, pid}} -> {:ok, pid}
1418
end
1519
end
1620

21+
@doc """
22+
Create a room given the host name
23+
"""
24+
@spec create_room(%{host_name: String.t()}) :: {:ok, Playground.DB.Room.t()} | {:error, term}
1725
def create_room(%{host_name: host_name} = opts) when is_binary(host_name) do
1826
case Rooms.create_room(opts) do
1927
{:ok, room} -> start_room_process(room)
2028
{:error, reason} -> {:error, reason}
2129
end
2230
end
2331

32+
@doc """
33+
Start the process that manages a room
34+
"""
35+
@spec start_room_process(Playground.DB.Room.t()) :: {:ok, Playground.DB.Room.t()}
2436
def start_room_process(room) do
2537
case DynamicSupervisor.start_child(__MODULE__, {RoomProcess, room}) do
2638
{:ok, pid} -> {:ok, pid}
@@ -31,6 +43,10 @@ defmodule Playground.Engine do
3143
end
3244

3345
@impl DynamicSupervisor
46+
@doc """
47+
Initializes the playground engine, set the strategy to :one_for_one
48+
"""
49+
@spec init(map) :: :ignore | {:ok, map()}
3450
def init(_init_arg) do
3551
DynamicSupervisor.init(strategy: :one_for_one)
3652
end

lib/playground/games.ex

+34
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,41 @@ defmodule Playground.Games do
2222
iex> list_games()
2323
[%Playground.DB.GameType{}]
2424
"""
25+
@spec list_games() :: [GameType.t()]
2526
def list_games do
2627
@games
2728
|> Map.keys()
2829
|> Enum.map(fn game_id -> get_game_details(game_id) end)
2930
end
3031

32+
@doc """
33+
Return the details of a game
34+
"""
3135
@callback get_game_details() :: GameType.t()
36+
37+
@doc """
38+
Return the name of a game
39+
"""
3240
@callback get_name() :: String.t()
41+
42+
@doc """
43+
Return the minimum number of players for a game
44+
"""
3345
@callback get_min_players() :: integer
46+
47+
@doc """
48+
Return the maximum number of players for a game
49+
"""
3450
@callback get_max_players() :: integer
51+
52+
@doc """
53+
Create a game changeset for the game with the initial state of the game
54+
"""
3555
@callback create_game_changeset(room :: Room.t()) :: Ecto.Changeset.t()
56+
57+
@doc """
58+
Apply a move to a game, alter the game state and return the new state
59+
"""
3660
@callback move(move :: map) :: map()
3761

3862
defp game_module(game_id), do: @games[game_id]
@@ -45,6 +69,7 @@ defmodule Playground.Games do
4569
iex> create_game_changeset(%Room{}, "game_id")
4670
%Ecto.Changeset{}
4771
"""
72+
@spec create_game_changeset(Room.t(), String.t()) :: Ecto.Changeset.t()
4873
def create_game_changeset(%Room{} = room, game_id) do
4974
game_module(game_id).create_game_changeset(room)
5075
end
@@ -57,6 +82,7 @@ defmodule Playground.Games do
5782
iex> get_game_details("game_id")
5883
%Playground.DB.GameType{}
5984
"""
85+
@spec get_game_details(String.t()) :: GameType.t()
6086
def get_game_details(game_id) do
6187
game_module(game_id).get_game_details()
6288
end
@@ -69,6 +95,7 @@ defmodule Playground.Games do
6995
iex> get_name("game_id")
7096
"Tic Tac Toe"
7197
"""
98+
@spec get_name(String.t()) :: String.t()
7299
def get_name(game_id) do
73100
game_module(game_id).get_name()
74101
end
@@ -81,6 +108,12 @@ defmodule Playground.Games do
81108
iex> move(%{game: game, player_id: _player_id, move: _move, support: _support})
82109
%Playground.DB.Game{}
83110
"""
111+
@spec move(%{
112+
game: Game.t(),
113+
player_id: integer | String.t(),
114+
move: map(),
115+
support: map()
116+
}) :: {:ok, Game.t()}
84117
def move(
85118
%{
86119
game: game,
@@ -103,6 +136,7 @@ defmodule Playground.Games do
103136
iex> get_player_name([%Playground.DB.Player{id: 1, name: "Alice"}, %Playground.DB.Player{id: 2, name: "Bob"}], 1)
104137
"Alice"
105138
"""
139+
@spec get_player_name([Playground.DB.Player.t()], integer | String.t()) :: String.t()
106140
def get_player_name(players, player_id) do
107141
players |> Enum.find(&("#{&1.id}" == "#{player_id}")) |> Map.get(:name, "")
108142
end

0 commit comments

Comments
 (0)