From 538c559ac7968e85d03c1f9b2b155fa28635c821 Mon Sep 17 00:00:00 2001 From: Sean Cribbs Date: Thu, 15 Nov 2012 20:03:55 -0500 Subject: [PATCH 1/2] Refactor basic_command_line test. * Try to do as much as possible while the node is up/down, without changing its state. Rapidly starting and stopping the node has strange effects. * We should consider removing the restart functionality from the `riak` script, as it leaves things in undetermined states. --- tests/basic_command_line.erl | 124 ++++++++++++++++------------------- 1 file changed, 57 insertions(+), 67 deletions(-) diff --git a/tests/basic_command_line.erl b/tests/basic_command_line.erl index b46ed8dc..3b0e5178 100644 --- a/tests/basic_command_line.erl +++ b/tests/basic_command_line.erl @@ -26,44 +26,50 @@ confirm() -> %% Deploy a node to test against lager:info("Deploy node to test command line"), - Nodes = rt:deploy_nodes(1), - [Node1] = Nodes, - ?assertEqual(ok, rt:wait_until_nodes_ready([Node1])), + [Node] = rt:deploy_nodes(1), + ?assertEqual(ok, rt:wait_until_nodes_ready([Node])), - %% It is possible to save some time grouping tests into whether riak - %% should be up or down when the test runs, but it is my opinion that the - %% the individual tests should handle riak in their own context at the - %% expense of testing time - console_test(Node1), + %% Verify node-up behavior + ping_up_test(Node), + attach_up_test(Node), + status_up_test(Node), + console_up_test(Node), + start_up_test(Node), - start_test(Node1), - - ping_test(Node1), - - restart_test(Node1), - - attach_test(Node1), - - status_test(Node1), + %% Stop the node, Verify node-down behavior + stop_test(Node), + ping_down_test(Node), + attach_down_test(Node), + status_down_test(Node), + console_test(Node), + start_test(Node), pass. +console_up_test(Node) -> + lager:info("Node is already up, `riak console` should fail"), + {ok, ConsoleFail} = rt:riak(Node, ["console"]), + ?assert(rt:str(ConsoleFail, "Node is already running")), + ok. + console_test(Node) -> %% Make sure the cluster will start up with /usr/sbin/riak console, then quit lager:info("Testing riak console on ~s", [Node]), - %% Ensure node is up to start with - rt:start_and_wait(Node), - lager:info("Node is already up, should fail"), - {ok, ConsoleFail} = rt:riak(Node, ["console"]), - ?assert(rt:str(ConsoleFail, "Node is already running")), - %% Stop node, to test console working - rt:stop_and_wait(Node), rt:console(Node, [{expect, "\(abort with ^G\)"}, - {send, "riak_core_ring_manager:get_my_ring()."}, - {expect, "dict,"}]), + {send, "riak_core_ring_manager:get_my_ring()."}, + {expect, "dict,"}, + {send, "q()."}, + {expect, "ok"}]), + rt:wait_until_unpingable(Node), + ok. +start_up_test(Node) -> + %% Try starting again and check you get the node is already running message + lager:info("Testing riak start now will return 'already running'"), + {ok, StartOut} = rt:riak(Node, ["start"]), + ?assert(rt:str(StartOut, "Node is already running!")), ok. @@ -71,39 +77,39 @@ start_test(Node) -> %% Test starting with /bin/riak start lager:info("Testing riak start works on ~s", [Node]), - %% First stop riak - rt:stop_and_wait(Node), - {ok, StartPass} = rt:riak(Node, ["start"]), ?assertMatch(StartPass, ""), - - %% Try starting again and check you get the node is already running message - lager:info("Testing riak start now will return 'already running'"), - {ok, StartOut} = rt:riak(Node, ["start"]), - ?assert(rt:str(StartOut, "Node is already running!")), - + rt:stop_and_wait(Node), ok. -ping_test(Node) -> +stop_test(Node) -> + ?assert(rt:is_pingable(Node)), + + ok = rt:stop(Node), + + ?assertNot(rt:is_pingable(Node)), + ok. + +ping_up_test(Node) -> %% check /usr/sbin/riak ping lager:info("Testing riak ping on ~s", [Node]), %% ping / pong - rt:start_and_wait(Node), + %% rt:start_and_wait(Node), lager:info("Node up, should ping"), {ok, PongOut} = rt:riak(Node, ["ping"]), ?assert(rt:str(PongOut, "pong")), + ok. +ping_down_test(Node) -> %% ping / pang - lager:info("Stopping Node"), - rt:stop_and_wait(Node), lager:info("Node down, should pang"), {ok, PangOut} = rt:riak(Node, ["ping"]), ?assert(rt:str(PangOut, "not responding to pings")), ok. -attach_test(Node) -> +attach_up_test(Node) -> %% check /usr/sbin/riak attach') %% Sort of a punt on this test, it tests that attach @@ -111,9 +117,6 @@ attach_test(Node) -> %% This is probably okay for a basic cmd line test lager:info("Testing riak attach"), - rt:start_and_wait(Node), - %{ok, AttachOut} = rt:riak(Node, ["attach"]), - %?assert(rt:str(AttachOut, "erlang.pipe.1 \(^D to exit\)")), rt:attach(Node, [{expect, "\(^D to exit\)"}, {send, "riak_core_ring_manager:get_my_ring()."}, @@ -122,38 +125,25 @@ attach_test(Node) -> ok. -restart_test(Node) -> - lager:info("Testing riak restart on ~s", [Node]), - - %% Riak should be running - rt:start_and_wait(Node), - - %% Issue restart - {ok, RestartOut} = rt:riak(Node, ["restart"]), - ?assert(rt:str(RestartOut, "ok")), - - %% Its not that we don't trust you 'ok' - %% but make sure riak is really up - ?assert(rt:is_pingable(Node)), - +attach_down_test(Node) -> + lager:info("Testing riak attach while down"), + {ok, AttachOut} = rt:riak(Node, ["attach"]), + ?assert(rt:str(AttachOut, "Node is not running!")), ok. -status_test(Node) -> +status_up_test(Node) -> lager:info("Test riak-admin status on ~s", [Node]), - % riak-admin status needs things completely started - % to work, so are more careful to wait - rt:start_and_wait(Node), - - lager:info("Waiting for status from riak_kv"), - rt:wait_until_status_ready(Node), - - lager:info("Now testing 'riak-admin status'"), {ok, StatusOut} = rt:admin(Node, ["status"]), io:format("Result of status: ~s", [StatusOut]), - ?assert(rt:str(StatusOut, "1-minute stats")), ?assert(rt:str(StatusOut, "kernel_version")), ok. + +status_down_test(Node) -> + lager:info("Test riak-admin status while down"), + {ok, StatusOut} = rt:admin(Node, ["status"]), + ?assert(rt:str(StatusOut, "Node is not running!")), + ok. From a68d9e2e479b7bc82d3af84916c1c2650324049b Mon Sep 17 00:00:00 2001 From: Joe DeVivo Date: Fri, 16 Nov 2012 08:02:38 -0700 Subject: [PATCH 2/2] Removed comment from basic_command_line questioning the validity of the test. It's super valid! --- tests/basic_command_line.erl | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/basic_command_line.erl b/tests/basic_command_line.erl index 3b0e5178..e5ffe065 100644 --- a/tests/basic_command_line.erl +++ b/tests/basic_command_line.erl @@ -110,12 +110,6 @@ ping_down_test(Node) -> ok. attach_up_test(Node) -> - - %% check /usr/sbin/riak attach') - %% Sort of a punt on this test, it tests that attach - %% connects to the pipe, but doesn't run any commands. - %% This is probably okay for a basic cmd line test - lager:info("Testing riak attach"), rt:attach(Node, [{expect, "\(^D to exit\)"},