exercism / exercism/elixir

Too forgiving test in `rpn-calculator-inspection`

オープン
#1,133 コメント 2 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Elixir
スター
664
フォーク
416
PR マージ指標
30日以内にマージされた PR はありません

説明

Tests for `RPNCalculatorInspection.reliability_check/2` are too forgiving for calculations that take longer than 100ms. If the solution starts the checks in parallel and awaits the results iteratively all the tests pass, but they should not. When the solution awaits the first check and it does not complete in 100ms, it adds `:timeout` to results. But when awaiting the second check, it already has that 100ms "bonus" from the previous await. So if the second check takes 150ms to finish, the await will receive the response after 50ms and add `:ok` to results, whereas it should add `:timeout`.

Here is a test which covers this case:
```elixir
test "returns :timeout for all calculations lasting longer than 100ms" do
inputs = [200, 150, 0]
calculator = &:timer.sleep(&1)

outputs = %{
200 => :timeout,
150 => :timeout,
0 => :ok
}

assert RPNCalculatorInspection.reliability_check(calculator, inputs) == outputs
end
```

And here is a solution which passes the existing tests, but not the above one:
```elixir
def reliability_check(calculator, inputs) do
trap_orig = Process.flag(:trap_exit, true)

results =
inputs
|> Enum.map(&start_reliability_check(calculator, &1))
|> Enum.reduce(%{}, &await_reliability_check_result/2)

Process.flag(:trap_exit, trap_orig)
results
end
```

Here's a solution which will pass the above test as well.
```elixir
def reliability_check(calculator, inputs) do
trap_orig = Process.flag(:trap_exit, true)
caller = self()

results =
inputs
|> Enum.map(fn input ->
await_pid =
spawn_link(fn ->
check = start_reliability_check(calculator, input)
results = await_reliability_check_result(check, %{})
[{^input, result}] = Map.to_list(results)
send(caller, {self(), result})
end)

{input, await_pid}
end)
|> Enum.reduce(%{}, fn {input, await_pid}, results ->
receive do
{^await_pid, result} -> Map.put(results, input, result)
after
200 -> raise "Timeout"
end
end)

Process.flag(:trap_exit, trap_orig)
results
end
```

Hardening the test would lead to more complicated solutions which are probably beyond the educational scope of this exercise. But the existing tests do not sufficiently cover the goals set in the exercise. So I'm not sure what exactly to do here.

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。