exercism / exercism/elixir

Too forgiving test in `rpn-calculator-inspection`

Aperta
#1,133 2 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Elixir
Stelle
664
Fork
416
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

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.

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.