From 3267b9861dff7768da3392b6e9d9a388258e96d1 Mon Sep 17 00:00:00 2001 From: Luke Bakken Date: Tue, 30 Jul 2019 18:23:10 -0700 Subject: [PATCH] All but one lager_file_backend test passes on win32 --- src/lager_file_backend.erl | 187 +++++++++++++++++++------------------ src/lager_util.erl | 31 +++--- 2 files changed, 114 insertions(+), 104 deletions(-) diff --git a/src/lager_file_backend.erl b/src/lager_file_backend.erl index 4781ae0..d4957b6 100644 --- a/src/lager_file_backend.erl +++ b/src/lager_file_backend.erl @@ -475,16 +475,14 @@ rotation_test_() -> SyncInterval = ?DEFAULT_SYNC_INTERVAL, Rotator = ?DEFAULT_ROTATION_MOD, CheckInterval = 0, %% hard to test delayed mode - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), - #state{name=TestLog, level=?DEBUG, sync_on=SyncLevel, sync_size=SyncSize, sync_interval=SyncInterval, check_interval=CheckInterval, rotator=Rotator} - end, - fun(#state{name=TestLog}) -> - lager_util:delete_test_dir(filename:dirname(TestLog)) + fun(#state{}) -> + ok = lager_util:delete_test_dir() end, [ fun(DefaultState = #state{name=TestLog, sync_size=SyncSize, sync_interval = SyncInterval, rotator = Rotator}) -> {"External rotation should work", @@ -506,7 +504,7 @@ rotation_test_() -> ok end} end, - fun(DefaultState = #state{name=TestLog, sync_size=SyncSize, sync_interval = SyncInterval, rotator = Rotator}) -> + fun(DefaultState = #state{name=TestLog, sync_size=SyncSize, sync_interval=SyncInterval, rotator=Rotator}) -> {"Internal rotation and delayed write", fun() -> TestLog0 = TestLog ++ ".0", @@ -561,14 +559,7 @@ filesystem_test_() -> {foreach, fun() -> ok = error_logger:tty(false), - case application:load(lager) of - ok -> ok; - {error,{already_loaded,lager}} -> ok; - Error -> - % TODO how to report EUnit errors? - io:format(standard_error, "ERROR: application:load(lager) returned ~p~n", [Error]), - ?assert(false) - end, + ok = safe_application_load(lager), ok = application:set_env(lager, handlers, [{lager_test_backend, info}]), ok = application:set_env(lager, error_logger_redirect, false), ok = application:set_env(lager, async_threshold, undefined), @@ -588,7 +579,7 @@ filesystem_test_() -> end, [ {"under normal circumstances, file should be opened", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), gen_event:add_handler(lager_event, lager_file_backend, @@ -601,7 +592,7 @@ filesystem_test_() -> end}, {"don't choke on unicode", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), gen_event:add_handler(lager_event, lager_file_backend, @@ -615,7 +606,7 @@ filesystem_test_() -> end}, {"don't choke on latin-1", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), %% XXX if this test fails, check that this file is encoded latin-1, not utf-8! @@ -630,23 +621,26 @@ filesystem_test_() -> end}, {"file can't be opened on startup triggers an error message", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), ?assertEqual(ok, file:write_file(TestLog, [])), - {ok, FInfo} = file:read_file_info(TestLog), - file:write_file_info(TestLog, FInfo#file_info{mode = 0}), + {ok, FInfo0} = file:read_file_info(TestLog), + FInfo1 = FInfo0#file_info{mode = 0}, + ?assertEqual(ok, file:write_file_info(TestLog, FInfo1)), gen_event:add_handler(lager_event, lager_file_backend, {TestLog, info, 10*1024*1024, "$D0", 5}), ?assertEqual(1, lager_test_backend:count()), - {_Level, _Time,Message, _Metadata} = lager_test_backend:pop(), + {_Level, _Time, Message, _Metadata} = lager_test_backend:pop(), + MessageFlat = lists:flatten(Message), ?assertEqual( "Failed to open log file " ++ TestLog ++ " with error permission denied", - lists:flatten(Message)) + MessageFlat), + ?assertEqual(ok, file:write_file_info(TestLog, FInfo0)) end}, {"file that becomes unavailable at runtime should trigger an error message", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), gen_event:add_handler(lager_event, lager_file_backend, @@ -661,7 +655,7 @@ filesystem_test_() -> {unix, _} -> ?assertEqual(ok, file:write_file(TestLog, "")), {ok, FInfo} = file:read_file_info(TestLog), - file:write_file_info(TestLog, FInfo#file_info{mode = 0}), + ?assertEqual(ok, file:write_file_info(TestLog, FInfo#file_info{mode = 0})), lager:log(error, self(), "Test message"), ?assertEqual(3, lager_test_backend:count()), lager_test_backend:pop(), @@ -674,13 +668,13 @@ filesystem_test_() -> end}, {"unavailable files that are fixed at runtime should start having log messages written", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), ?assertEqual(ok, file:write_file(TestLog, [])), {ok, FInfo} = file:read_file_info(TestLog), OldPerms = FInfo#file_info.mode, - file:write_file_info(TestLog, FInfo#file_info{mode = 0}), + ?assertEqual(ok, file:write_file_info(TestLog, FInfo#file_info{mode = 0})), gen_event:add_handler(lager_event, lager_file_backend, [{file, TestLog},{check_interval, 0}]), ?assertEqual(1, lager_test_backend:count()), @@ -688,7 +682,7 @@ filesystem_test_() -> ?assertEqual( "Failed to open log file " ++ TestLog ++ " with error permission denied", lists:flatten(Message)), - file:write_file_info(TestLog, FInfo#file_info{mode = OldPerms}), + ?assertEqual(ok, file:write_file_info(TestLog, FInfo#file_info{mode = OldPerms})), lager:log(error, self(), "Test message"), {ok, Bin} = file:read_file(TestLog), Pid = pid_to_list(self()), @@ -697,7 +691,7 @@ filesystem_test_() -> end}, {"external logfile rotation/deletion should be handled", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), TestLog0 = TestLog ++ ".0", @@ -721,7 +715,7 @@ filesystem_test_() -> end}, {"internal size rotation should work", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), TestLog0 = TestLog ++ ".0", @@ -733,7 +727,7 @@ filesystem_test_() -> end}, {"internal time rotation should work", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), TestLog0 = TestLog ++ ".0", @@ -747,7 +741,7 @@ filesystem_test_() -> end}, {"rotation call should work", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), TestLog0 = TestLog ++ ".0", @@ -761,7 +755,7 @@ filesystem_test_() -> end}, {"sync_on option should work", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), gen_event:add_handler(lager_event, lager_file_backend, [{file, TestLog}, @@ -775,7 +769,7 @@ filesystem_test_() -> end}, {"sync_on none option should work (also tests sync_interval)", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), gen_event:add_handler(lager_event, lager_file_backend, [{file, TestLog}, @@ -791,7 +785,7 @@ filesystem_test_() -> end}, {"sync_size option should work", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), gen_event:add_handler(lager_event, lager_file_backend, [{file, TestLog}, {level, info}, @@ -814,7 +808,7 @@ filesystem_test_() -> end}, {"runtime level changes", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), gen_event:add_handler(lager_event, {lager_file_backend, TestLog}, {TestLog, info}), @@ -833,7 +827,7 @@ filesystem_test_() -> end}, {"invalid runtime level changes", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), TestLog3 = filename:join(TestDir, "test3.log"), @@ -844,7 +838,7 @@ filesystem_test_() -> end}, {"tracing should work", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), gen_event:add_handler(lager_event, lager_file_backend, {TestLog, critical}), @@ -861,7 +855,7 @@ filesystem_test_() -> end}, {"tracing should not duplicate messages", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), gen_event:add_handler(lager_event, lager_file_backend, @@ -887,7 +881,7 @@ filesystem_test_() -> end}, {"tracing to a dedicated file should work", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "foo.log"), {ok, _} = lager:trace_file(TestLog, [{module, ?MODULE}]), @@ -900,7 +894,7 @@ filesystem_test_() -> end}, {"tracing to a dedicated file should work even if root_log is set", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), LogName = "foo.log", LogPath = filename:join(TestDir, LogName), @@ -916,7 +910,7 @@ filesystem_test_() -> end}, {"tracing with options should work", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "foo.log"), TestLog0 = TestLog ++ ".0", @@ -935,7 +929,7 @@ filesystem_test_() -> end}, {"no silent hwm drops", fun() -> - TestDir = lager_util:get_test_dir(), + {ok, TestDir} = lager_util:get_test_dir(), TestLog = filename:join(TestDir, "test.log"), gen_event:add_handler(lager_event, lager_file_backend, [{file, TestLog}, {level, info}, {high_water_mark, 5}, {flush_queue, false}, {sync_on, "=warning"}]), @@ -954,42 +948,41 @@ filesystem_test_() -> trace_files_test_() -> {foreach, fun() -> - Dir = lager_util:get_test_dir(), - Log = filename:join(Dir, "test.log"), - Debug = filename:join(Dir, "debug.log"), - Events = filename:join(Dir, "events.log"), - - error_logger:tty(false), - application:load(lager), - application:set_env(lager, handlers, [ - {lager_file_backend, [ - {file, Log}, - {level, error}, - {formatter, lager_default_formatter}, - {formatter_config, [message, "\n"]} - ]} - ]), - application:set_env(lager, traces, [ - { % get default level of debug - {lager_file_backend, Debug}, [{module, ?MODULE}] - }, - { % Handler Filters Level - {lager_file_backend, Events}, [{module, ?MODULE}], notice - } - ]), - application:set_env(lager, async_threshold, undefined), - lager:start(), - {Dir, Log, Debug, Events} + {ok, TestDir} = lager_util:get_test_dir(), + Log = filename:join(TestDir, "test.log"), + Debug = filename:join(TestDir, "debug.log"), + Events = filename:join(TestDir, "events.log"), + + ok = error_logger:tty(false), + ok = safe_application_load(lager), + ok = application:set_env(lager, handlers, [ + {lager_file_backend, [ + {file, Log}, + {level, error}, + {formatter, lager_default_formatter}, + {formatter_config, [message, "\n"]} + ]} + ]), + ok = application:set_env(lager, traces, [ + { % get default level of debug + {lager_file_backend, Debug}, [{module, ?MODULE}] + }, + { % Handler Filters Level + {lager_file_backend, Events}, [{module, ?MODULE}], notice + } + ]), + ok = application:set_env(lager, async_threshold, undefined), + ok = lager:start(), + {Log, Debug, Events} end, - fun({Dir, _, _, _}) -> + fun({_, _, _}) -> catch ets:delete(lager_config), - application:unset_env(lager, traces), - application:stop(lager), - - lager_util:delete_test_dir(Dir), - error_logger:tty(true) + ok = application:unset_env(lager, traces), + ok = application:stop(lager), + ok = lager_util:delete_test_dir(), + ok = error_logger:tty(true) end, [ - fun({_, Log, Debug, Events}) -> + fun({Log, Debug, Events}) -> {"a trace using file backend set up in configuration should work", fun() -> lager:error("trace test error message"), @@ -1029,28 +1022,27 @@ count_lines_until(Lines, Timeout, File, Last) -> formatting_test_() -> {foreach, fun() -> - Dir = lager_util:get_test_dir(), - Log1 = filename:join(Dir, "test.log"), - Log2 = filename:join(Dir, "test2.log"), + {ok, TestDir} = lager_util:get_test_dir(), + Log1 = filename:join(TestDir, "test.log"), + Log2 = filename:join(TestDir, "test2.log"), ?assertEqual(ok, file:write_file(Log1, [])), ?assertEqual(ok, file:write_file(Log2, [])), - - error_logger:tty(false), - application:load(lager), - application:set_env(lager, handlers, [{lager_test_backend, info}]), - application:set_env(lager, error_logger_redirect, false), - lager:start(), + ok = error_logger:tty(false), + ok = safe_application_load(lager), + ok = application:set_env(lager, handlers, [{lager_test_backend, info}]), + ok = application:set_env(lager, error_logger_redirect, false), + ok = lager:start(), %% same race condition issue - timer:sleep(5), - {Dir, Log1, Log2} + ok = timer:sleep(5), + {ok, Log1, Log2} end, - fun({Dir, _, _}) -> - application:stop(lager), - application:stop(goldrush), - lager_util:delete_test_dir(Dir), - error_logger:tty(true) + fun({ok, _, _}) -> + ok = application:stop(lager), + ok = application:stop(goldrush), + ok = lager_util:delete_test_dir(), + ok = error_logger:tty(true) end, [ - fun({_, Log1, Log2}) -> + fun({ok, Log1, Log2}) -> {"Should have two log files, the second prefixed with 2>", fun() -> gen_event:add_handler(lager_event, lager_file_backend, @@ -1124,5 +1116,16 @@ config_validation_test_() -> } ]. +safe_application_load(App) -> + case application:load(App) of + ok -> + ok; + {error, {already_loaded, App}} -> + ok; + Error -> + % TODO how to report EUnit errors? + io:format(standard_error, "ERROR: application:load(~p) returned ~p~n", [App, Error]), + ?assert(false) + end. -endif. diff --git a/src/lager_util.erl b/src/lager_util.erl index 8d716a8..5d8fd0f 100644 --- a/src/lager_util.erl +++ b/src/lager_util.erl @@ -843,8 +843,13 @@ create_test_dir() -> ok = application:set_env(lager, test_dir, TestDir). get_test_dir() -> - {ok, TestDir} = application:get_env(lager, test_dir), - TestDir. + case application:get_env(lager, test_dir) of + undefined -> + create_test_dir(), + get_test_dir(); + {ok, _}=Res -> + Res + end. get_temp_dir() -> Tmp = case os:getenv("TEMP") of @@ -859,19 +864,21 @@ get_temp_dir() -> {ok, Tmp}. delete_test_dir() -> - delete_test_dir(get_test_dir()). + {ok, TestDir} = get_test_dir(), + ok = delete_test_dir(TestDir). delete_test_dir(TestDir) -> {OsType, _} = os:type(), - case {OsType, otp_version()} of - {win32, _} -> - application:stop(lager), - do_delete_test_dir(TestDir); - {unix, 15} -> - os:cmd("rm -rf " ++ TestDir); - {unix, _} -> - do_delete_test_dir(TestDir) - end. + ok = case {OsType, otp_version()} of + {win32, _} -> + application:stop(lager), + do_delete_test_dir(TestDir); + {unix, 15} -> + os:cmd("rm -rf " ++ TestDir); + {unix, _} -> + do_delete_test_dir(TestDir) + end, + ok = application:unset_env(lager, test_dir). do_delete_test_dir(Dir) -> ListRet = file:list_dir_all(Dir),