Merge pull request #17 from inaka/elbrujohalcon.17.let_players_skip_t…

…asks

Let players skip tasks

src/bo_server.erl

@@ -34,46 +34,36 @@ init(noargs) -> {ok, #{}}.
   ({signup, bo_players:name()}, {pid(), term()}, state()) ->
     {reply, {ok, bo_task:task()} | {error, conflict}, state()};
   ({task, bo_players:name()}, {pid(), term()}, state()) ->
-    {reply, {ok, bo_task:task()} | {error, forbidden | notfound}, state()};
+    {reply, {ok, bo_task:task()}
+          | {error, ended | forbidden | notfound}, state()};
   ({submit, bo_players:name(), term()}, {pid(), term()}, state()) ->
     {reply, {ok, bo_task:task()} | the_end
-          | {error, invalid | timeout | forbidden | notfound}
-          | {failures, [term()]}, state()}.
+          | {error, invalid | timeout | ended | forbidden | notfound}
+          | {failures, [term()]}, state()};
+  ({skip, bo_players:name()}, {pid(), term()}, state()) ->
+    {reply, {ok, bo_task:task()} | the_end
+          | {error, ended | forbidden | notfound}, state()}.
 handle_call({signup, PlayerName}, {From, _}, State) ->
   Node = node(From),
   try bo_players_repo:signup(PlayerName, Node) of
-    Player -> {reply, {ok, task(Player)}, State}
+    Player -> {reply, task(Player), State}
   catch
     _:conflict -> {reply, {error, conflict}, State}
   end;
 handle_call({task, PlayerName}, {From, _}, State) ->
-  Node = node(From),
-  case bo_players_repo:fetch(PlayerName) of
-    notfound -> {reply, {error, notfound}, State};
-    Player ->
-      case bo_players:node(Player) of
-        Node -> {reply, {ok, task(Player)}, State};
-        NotNode ->
-          error_logger:warning_msg(
-            "~p trying to access from ~p but registered at ~p",
-            [PlayerName, Node, NotNode]),
-          {reply, {error, forbidden}, State}
-      end
+  case check_player(PlayerName, From) of
+    {error, Error} -> {reply, {error, Error}, State};
+    Player -> {reply, task(Player), State}
   end;
 handle_call({submit, PlayerName, Solution}, {From, _}, State) ->
-  Node = node(From),
-  case bo_players_repo:fetch(PlayerName) of
-    notfound -> {reply, {error, notfound}, State};
-    Player ->
-      case bo_players:node(Player) of
-        Node ->
-          {reply, test(Player, Solution), State};
-        NotNode ->
-          error_logger:warning_msg(
-            "~p trying to access from ~p but registered at ~p",
-            [PlayerName, Node, NotNode]),
-          {reply, {error, forbidden}, State}
-      end
+  case check_player(PlayerName, From) of
+    {error, Error} -> {reply, {error, Error}, State};
+    Player -> {reply, test(Player, Solution), State}
+  end;
+handle_call({skip, PlayerName}, {From, _}, State) ->
+  case check_player(PlayerName, From) of
+    {error, Error} -> {reply, {error, Error}, State};
+    Player -> {reply, advance(Player), State}
   end.
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@@ -91,15 +81,36 @@ handle_info(_, State) -> {noreply, State}.
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %%% Internals
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
-task(Player) -> bo_task:describe(bo_players:task(Player)).
+check_player(PlayerName, From) ->
+  Node = node(From),
+  case bo_players_repo:fetch(PlayerName) of
+    notfound -> {error, notfound};
+    Player ->
+      case bo_players:node(Player) of
+        Node -> check_task(Player);
+        NotNode ->
+          error_logger:warning_msg(
+            "~p trying to access from ~p but registered at ~p",
+            [PlayerName, Node, NotNode]),
+          {error, forbidden}
+      end
+  end.
+
+check_task(Player) ->
+  case bo_players:task(Player) of
+    undefined -> {error, ended};
+    _Task -> Player
+  end.
+
+task(Player) -> {ok, bo_task:describe(bo_players:task(Player))}.
 
 test(Player, Solution) ->
   case bo_task:test(bo_players:task(Player), Solution) of
-    ok -> next_task(Player);
+    ok -> advance(Player);
     NOK -> NOK
   end.
 
-next_task(Player) ->
+advance(Player) ->
   NewPlayer = bo_players_repo:advance(Player),
   case bo_players:task(NewPlayer) of
     undefined -> the_end;

src/tasks/bo_task.erl

@@ -4,7 +4,6 @@
 
 -type task() :: #{ name := module()
                  , desc := binary()
-                 , time := pos_integer()
                  }.
 
 -type test() :: fun((fun()) -> ok | {error, term()}).
@@ -22,7 +21,6 @@
 describe(Task) ->
   #{ name => Task
    , desc => Task:description()
-   , time => Task:timeout()
    }.
 
 -spec test(module(), fun()) -> result().

test/bo_skip_SUITE.erl

@@ -0,0 +1,107 @@
+-module(bo_skip_SUITE).
+-author('elbrujohalcon@inaka.net').
+
+-export([all/0]).
+-export([init_per_suite/1, end_per_suite/1]).
+-export([ next_task/1
+        , next_task_is_random/1
+        , no_more_tasks/1
+        ]).
+
+-type config() :: proplists:proplist().
+
+-spec all() -> [atom()].
+all() -> [next_task, next_task_is_random, no_more_tasks].
+
+-spec init_per_suite(config()) -> config().
+init_per_suite(Config) ->
+  case net_kernel:start(['bo_test@127.0.0.1']) of
+    {ok, _} -> ok;
+    {error, {already_started, _}} -> ok;
+    {error, Error} -> throw(Error)
+  end,
+  _ = application:load(beam_olympics),
+  application:set_env(
+    beam_olympics, all_tasks, [bo_first_task, simple_task1, simple_task2]),
+  {ok, _} = bo:start(),
+  _ = sumo:delete_all(bo_players),
+  {ok, Client} = bo_test_client:start(skip_suite),
+  [{client, Client} | Config].
+
+-spec end_per_suite(config()) -> config().
+end_per_suite(Config) ->
+  {client, Client} = lists:keyfind(client, 1, Config),
+  ok = bo_test_client:stop(Client),
+  _ = sumo:delete_all(bo_players),
+  application:unset_env(beam_olympics, all_tasks),
+  ok = bo:stop(),
+  Config.
+
+-spec next_task(config()) -> {comment, string()}.
+next_task(Config) ->
+  {client, Client} = lists:keyfind(client, 1, Config),
+  {ok, FirstTask} = bo_test_client:signup(Client, <<"next_task">>),
+
+  ct:comment("The initial task can be skipped"),
+  #{name := bo_first_task} = FirstTask,
+  {ok, NextTask} = bo_test_client:skip(Client, <<"next_task">>),
+
+  ct:comment("A new task is provided"),
+  case NextTask of
+    FirstTask -> ct:fail("Different task expected");
+    NextTask -> ok
+  end,
+
+  {comment, ""}.
+
+-spec next_task_is_random(config()) -> {comment, string()}.
+next_task_is_random(Config) ->
+  Players = [<<Char>> || Char <- lists:seq($a, $z)],
+  {client, Client} = lists:keyfind(client, 1, Config),
+
+  ct:comment("The initial task is the same for all players"),
+  [{ok, FirstTask}] =
+    lists:usort([bo_test_client:signup(Client, Player) || Player <- Players]),
+  #{name := bo_first_task} = FirstTask,
+
+  ct:comment("Every player can skip that task"),
+  [{ok, NextTask1}, {ok, NextTask2}] =
+    lists:usort([bo_test_client:skip(Client, Player) || Player <- Players]),
+
+  ct:comment("Tasks come from the list"),
+  case {NextTask1, NextTask2} of
+    {#{name := simple_task1}, #{name := simple_task2}} -> ok;
+    {#{name := simple_task2}, #{name := simple_task1}} -> ok;
+    {NextTask1, NextTask2} ->
+      ct:fail("Unexpected tasks: ~p", [{NextTask1, NextTask2}])
+  end,
+
+  {comment, ""}.
+
+-spec no_more_tasks(config()) -> {comment, string()}.
+no_more_tasks(Config) ->
+  {client, Client} = lists:keyfind(client, 1, Config),
+  {ok, FirstTask} = bo_test_client:signup(Client, <<"nmt">>),
+
+  ct:comment("The initial task can be skipped"),
+  #{name := bo_first_task} = FirstTask,
+  {ok, #{name := NextTask}} = bo_test_client:skip(Client, <<"nmt">>),
+
+  ct:comment("The next task can be skipped"),
+  ExpectedTask =
+    case NextTask of
+      simple_task1 -> simple_task2;
+      simple_task2 -> simple_task1
+    end,
+  {ok, #{name := ExpectedTask}} = bo_test_client:skip(Client, <<"nmt">>),
+
+  ct:comment("Solving the final task user gets the end message"),
+  the_end = bo_test_client:skip(Client, <<"nmt">>),
+
+  ct:comment("Once finished, the player task is undefined"),
+  {error, ended} = bo_test_client:task(Client, <<"nmt">>),
+
+  ct:comment("Once finished, the player can't skip tasks"),
+  {error, ended} = bo_test_client:skip(Client, <<"nmt">>),
+
+  {comment, ""}.

test/bo_test_client.erl

@@ -1,7 +1,7 @@
 -module(bo_test_client).
 
 -export([start/1, stop/1]).
--export([signup/2, task/2, submit/3]).
+-export([signup/2, task/2, submit/3, skip/2]).
 -export([gen_call/2]).
 
 -type task() :: bo_task:task().
@@ -29,11 +29,13 @@ signup(Node, Player) -> call(Node, {signup, Player}).
 task(Node, Player) -> call(Node, {task, Player}).
 
 -spec submit(node(), player_name(), term()) ->
-    {ok, bo_task:task()} | the_end
-  | {error, invalid | timeout | forbidden | notfound}
-  | {failures, [term()]}.
+    {ok, bo_task:task()} | the_end | {error, term()} | {failures, [term()]}.
 submit(Node, Player, Solution) -> call(Node, {submit, Player, Solution}).
 
+-spec skip(node(), player_name()) ->
+    {ok, bo_task:task()} | the_end | {error, term()}.
+skip(Node, Player) -> call(Node, {skip, Player}).
+
 call(Node, Msg) ->
   Caller = self(),
   Pid = proc_lib:spawn(Node, ?MODULE, gen_call, [Caller, Msg]),

test/bo_valid_SUITE.erl

@@ -25,7 +25,7 @@ init_per_suite(Config) ->
     beam_olympics, all_tasks, [bo_first_task, simple_task1, simple_task2]),
   {ok, _} = bo:start(),
   _ = sumo:delete_all(bo_players),
-  {ok, Client} = bo_test_client:start(invalid_suite),
+  {ok, Client} = bo_test_client:start(valid_suite),
   [{client, Client} | Config].
 
 -spec end_per_suite(config()) -> config().
@@ -102,6 +102,12 @@ no_more_tasks(Config) ->
   the_end =
     bo_test_client:submit(Client, <<"nmt">>, fun ExpectedTask:solution/1),
 
+  ct:comment("Once finished, the player task is undefined"),
+  {error, ended} = bo_test_client:task(Client, <<"nmt">>),
+
+  ct:comment("Once finished, the player can't submit new solutions"),
+  {error, ended} = bo_test_client:submit(Client, <<"nmt">>, fun id/1),
+
   {comment, ""}.
 
 id(X) ->