From 2f4f2531e96cab233f848cb0dcdf7f1d97930885 Mon Sep 17 00:00:00 2001 From: David Norton Date: Sat, 28 Sep 2024 13:54:24 -0500 Subject: [PATCH 1/3] partial fix for flaky renameuser test using polling-until-expected-result relax RENAMEACCOUNT test since apparently more information is returned from the server now formatted code --- .../protocols/spring/spring_auth_test.exs | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/test/teiserver/protocols/spring/spring_auth_test.exs b/test/teiserver/protocols/spring/spring_auth_test.exs index a13bdcff8..fee41837a 100644 --- a/test/teiserver/protocols/spring/spring_auth_test.exs +++ b/test/teiserver/protocols/spring/spring_auth_test.exs @@ -18,11 +18,25 @@ defmodule Teiserver.SpringAuthTest do new_user: 2 ] + import Teiserver.Support.Tachyon, only: [poll_until: 2] + setup do %{socket: socket, user: user} = auth_setup() {:ok, socket: socket, user: user} end + # wrapped login attempt in a function to make polling for expected output easier + def _send_login_attempt_and_parse_reply_msg_from_server_socket(socket, login_name) do + _send_raw( + socket, + "LOGIN #{login_name} X03MO1qnZdYdgyfeuILPmQ== 0 * LuaLobby Chobby\t1993717506 0d04a635e200f308\tb sp\n" + ) + + reply = _recv_until(socket) + [server_response | _remainder] = String.split(reply, "\n") + server_response + end + test "Test bad match", %{socket: socket} do # Previously a bad match on the data could cause a failure on the next # command as it corrupted state. This checks for that regression @@ -442,9 +456,6 @@ CLIENTS test_room #{user.name}\n" assert reply =~ "s.battles.id_list " end - # this is a flakey test, but only when other tests are running around. - # you may be able to reproduce with the seed 846266 - @tag :needs_attention test "RENAMEACCOUNT", %{socket: socket, user: user} do old_name = user.name new_name = "test_user_rename" @@ -461,7 +472,9 @@ CLIENTS test_room #{user.name}\n" # Rename with an invalid name _send_raw(socket, "RENAMEACCOUNT Y--Y\n") reply = _recv_raw(socket) - assert reply == "SERVERMSG Invalid characters in name (only a-z, A-Z, 0-9, [, ] allowed)\n" + assert String.starts_with?(reply, "SERVERMSG") + assert String.contains?(reply, "Invalid characters in name") + assert String.contains?(reply, "(only a-z, A-Z, 0-9, [, ] allowed)") # Rename with existng name _send_raw(socket, "RENAMEACCOUNT #{watcher_user.name}\n") @@ -478,9 +491,6 @@ CLIENTS test_room #{user.name}\n" wreply = _recv_raw(watcher) assert wreply == "REMOVEUSER #{user.name}\n" - # Give it a chance to perform some of the actions - :timer.sleep(250) - assert Client.get_client_by_id(userid) == nil # Lets be REAL sure @@ -497,14 +507,15 @@ CLIENTS test_room #{user.name}\n" _ = _recv_raw(socket) # Now we get flood protection after the rename - _send_raw( - socket, - "LOGIN #{new_name} X03MO1qnZdYdgyfeuILPmQ== 0 * LuaLobby Chobby\t1993717506 0d04a635e200f308\tb sp\n" - ) + expected_server_response = "DENIED Flood protection - Please wait 20 seconds and try again" - reply = _recv_until(socket) - [accepted | _remainder] = String.split(reply, "\n") - assert accepted == "DENIED Flood protection - Please wait 20 seconds and try again" + server_response = + Teiserver.Support.Tachyon.poll_until( + fn -> _send_login_attempt_and_parse_reply_msg_from_server_socket(socket, new_name) end, + &(&1 == expected_server_response) + ) + + assert server_response == expected_server_response # Un-flood them CacheUser.set_flood_level(userid, 0) From 28eb33e89ab8e632d59ea36a1220f2ee1e283ac6 Mon Sep 17 00:00:00 2001 From: David Norton Date: Wed, 2 Oct 2024 20:51:41 -0500 Subject: [PATCH 2/3] fixup yet another assert statement to work under slightly different server responses --- test/teiserver/protocols/spring/spring_auth_test.exs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/teiserver/protocols/spring/spring_auth_test.exs b/test/teiserver/protocols/spring/spring_auth_test.exs index fee41837a..d10dde7ca 100644 --- a/test/teiserver/protocols/spring/spring_auth_test.exs +++ b/test/teiserver/protocols/spring/spring_auth_test.exs @@ -479,7 +479,8 @@ CLIENTS test_room #{user.name}\n" # Rename with existng name _send_raw(socket, "RENAMEACCOUNT #{watcher_user.name}\n") reply = _recv_raw(socket) - assert reply == "SERVERMSG Username already taken\n" + assert String.starts_with?(reply, "SERVERMSG") + assert String.contains?(reply, "Username already taken") # Perform rename _send_raw(socket, "RENAMEACCOUNT test_user_rename\n") From e1befdeb301ea6adab04fee7acc67c9ba1219d70 Mon Sep 17 00:00:00 2001 From: David Norton Date: Mon, 7 Oct 2024 07:17:01 -0500 Subject: [PATCH 3/3] switched to anon function, set timeouts --- .../protocols/spring/spring_auth_test.exs | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/test/teiserver/protocols/spring/spring_auth_test.exs b/test/teiserver/protocols/spring/spring_auth_test.exs index d10dde7ca..eafa56cd5 100644 --- a/test/teiserver/protocols/spring/spring_auth_test.exs +++ b/test/teiserver/protocols/spring/spring_auth_test.exs @@ -25,18 +25,6 @@ defmodule Teiserver.SpringAuthTest do {:ok, socket: socket, user: user} end - # wrapped login attempt in a function to make polling for expected output easier - def _send_login_attempt_and_parse_reply_msg_from_server_socket(socket, login_name) do - _send_raw( - socket, - "LOGIN #{login_name} X03MO1qnZdYdgyfeuILPmQ== 0 * LuaLobby Chobby\t1993717506 0d04a635e200f308\tb sp\n" - ) - - reply = _recv_until(socket) - [server_response | _remainder] = String.split(reply, "\n") - server_response - end - test "Test bad match", %{socket: socket} do # Previously a bad match on the data could cause a failure on the next # command as it corrupted state. This checks for that regression @@ -492,7 +480,12 @@ CLIENTS test_room #{user.name}\n" wreply = _recv_raw(watcher) assert wreply == "REMOVEUSER #{user.name}\n" - assert Client.get_client_by_id(userid) == nil + Teiserver.Support.Tachyon.poll_until( + fn -> Client.get_client_by_id(userid) end, + &(&1 == nil), + limit: 32, + wait: 250 + ) # Lets be REAL sure client_ids = Client.list_client_ids() @@ -510,13 +503,21 @@ CLIENTS test_room #{user.name}\n" # Now we get flood protection after the rename expected_server_response = "DENIED Flood protection - Please wait 20 seconds and try again" - server_response = - Teiserver.Support.Tachyon.poll_until( - fn -> _send_login_attempt_and_parse_reply_msg_from_server_socket(socket, new_name) end, - &(&1 == expected_server_response) - ) - - assert server_response == expected_server_response + Teiserver.Support.Tachyon.poll_until( + fn -> + _send_raw( + socket, + "LOGIN #{new_name} X03MO1qnZdYdgyfeuILPmQ== 0 * LuaLobby Chobby\t1993717506 0d04a635e200f308\tb sp\n" + ) + + reply = _recv_until(socket) + [accepted | _remainder] = String.split(reply, "\n") + accepted + end, + &(&1 == expected_server_response), + limit: 32, + wait: 250 + ) # Un-flood them CacheUser.set_flood_level(userid, 0)