Fixed tests in lager_crash_log on win32

src/lager_crash_log.erl

@@ -243,30 +243,30 @@ do_log({log, Event}, #state{name=Name, fd=FD, inode=Inode, ctime=Ctime, flap=Fla
 filesystem_test_() ->
     {foreach,
         fun() ->
-            CrashLog = filename:join(lager_util:create_test_dir(), "crash_test.log"),
-            file:write_file(CrashLog, []),
-
-            error_logger:tty(false),
-            application:load(lager),
-            application:set_env(lager, handlers, [{lager_test_backend, info}]),
-            application:set_env(lager, error_logger_redirect, true),
-            application:unset_env(lager, crash_log),
-            lager:start(),
-            timer:sleep(1000),
-            lager_test_backend:flush(),
+            {ok, TestDir} = lager_util:get_test_dir(),
+            CrashLog = filename:join(TestDir, "crash_test.log"),
+            ok = lager_test_util:safe_write_file(CrashLog, []),
+            ok = error_logger:tty(false),
+            ok = lager_test_util:safe_application_load(lager),
+            ok = application:set_env(lager, handlers, [{lager_test_backend, info}]),
+            ok = application:set_env(lager, error_logger_redirect, true),
+            ok = application:unset_env(lager, crash_log),
+            ok = lager:start(),
+            ok = timer:sleep(1000),
+            ok = lager_test_backend:flush(),
             CrashLog
         end,
-        fun(CrashLog) ->
+        fun(_CrashLog) ->
             case whereis(lager_crash_log) of
                 P when is_pid(P) ->
-                    exit(P, kill);
+                    gen_server:stop(P);
                 _ ->
                     ok
             end,
-            application:stop(lager),
-            application:stop(goldrush),
-            lager_util:delete_test_dir(filename:dirname(CrashLog)),
-            error_logger:tty(true)
+            ok = application:stop(lager),
+            ok = application:stop(goldrush),
+            ok = lager_util:delete_test_dir(),
+            ok = error_logger:tty(true)
         end, [
         fun(CrashLog) ->
             {"under normal circumstances, file should be opened",
@@ -281,9 +281,13 @@ filesystem_test_() ->
         fun(CrashLog) ->
             {"file can't be opened on startup triggers an error message",
             fun() ->
-                {ok, FInfo} = file:read_file_info(CrashLog, [raw]),
-                file:write_file_info(CrashLog, FInfo#file_info{mode = 0}),
+                {ok, FInfo0} = file:read_file_info(CrashLog, [raw]),
+                FInfo1 = FInfo0#file_info{mode = 0},
+                ?assertEqual(ok, file:write_file_info(CrashLog, FInfo1)),
                 {ok, _} = ?MODULE:start_link(CrashLog, 65535, 0, undefined, 0, lager_rotator_default),
+                % Note: required on win32, do this early to prevent subsequent failures
+                % from preventing cleanup
+                ?assertEqual(ok, file:write_file_info(CrashLog, FInfo0)),
                 ?assertEqual(1, lager_test_backend:count()),
                 {_Level, _Time, Message,_Metadata} = lager_test_backend:pop(),
                 ?assertEqual(
@@ -300,11 +304,15 @@ filesystem_test_() ->
                 _ = gen_event:which_handlers(error_logger),
                 ?assertEqual(1, lager_test_backend:count()),
                 file:delete(CrashLog),
-                file:write_file(CrashLog, ""),
-                {ok, FInfo} = file:read_file_info(CrashLog, [raw]),
-                file:write_file_info(CrashLog, FInfo#file_info{mode = 0}),
+                ok = lager_test_util:safe_write_file(CrashLog, ""),
+                {ok, FInfo0} = file:read_file_info(CrashLog, [raw]),
+                FInfo1 = FInfo0#file_info{mode = 0},
+                ?assertEqual(ok, file:write_file_info(CrashLog, FInfo1)),
                 sync_error_logger:error_msg("Test message\n"),
                 _ = gen_event:which_handlers(error_logger),
+                % Note: required on win32, do this early to prevent subsequent failures
+                % from preventing cleanup
+                ?assertEqual(ok, file:write_file_info(CrashLog, FInfo0)),
                 ?assertEqual(3, lager_test_backend:count()),
                 lager_test_backend:pop(),
                 {_Level, _Time, Message,_Metadata} = lager_test_backend:pop(),
@@ -341,8 +349,8 @@ filesystem_test_() ->
                 _ = gen_event:which_handlers(error_logger),
                 {ok, Bin} = file:read_file(CrashLog),
                 ?assertMatch([_, "Test message\n"], re:split(Bin, "\n", [{return, list}, {parts, 2}])),
-                file:delete(CrashLog),
-                file:write_file(CrashLog, ""),
+                ?assertEqual(ok, file:delete(CrashLog)),
+                ?assertEqual(ok, lager_test_util:safe_write_file(CrashLog, "")),
                 sync_error_logger:error_msg("Test message1~n"),
                 _ = gen_event:which_handlers(error_logger),
                 {ok, Bin1} = file:read_file(CrashLog),

src/lager_file_backend.erl

@@ -594,7 +594,7 @@ filesystem_test_() ->
     {foreach,
         fun() ->
             ok = error_logger:tty(false),
-            ok = safe_application_load(lager),
+            ok = lager_test_util: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),
@@ -603,8 +603,8 @@ filesystem_test_() ->
             %% race condition where lager logs its own start up
             %% makes several tests fail. See test/lager_test_backend
             %% around line 800 for more information.
-            timer:sleep(5),
-            lager_test_backend:flush()
+            ok = timer:sleep(5),
+            ok = lager_test_backend:flush()
         end,
         fun(_) ->
             ok = application:stop(lager),
@@ -658,7 +658,7 @@ filesystem_test_() ->
         fun() ->
             {ok, TestDir} = lager_util:get_test_dir(),
             TestLog = filename:join(TestDir, "test.log"),
-            ?assertEqual(ok, safe_write_file(TestLog, [])),
+            ?assertEqual(ok, lager_test_util:safe_write_file(TestLog, [])),
 
             {ok, FInfo0} = file:read_file_info(TestLog, [raw]),
             FInfo1 = FInfo0#file_info{mode = 0},
@@ -689,7 +689,7 @@ filesystem_test_() ->
             lager:log(error, self(), "Test message"),
             ?assertEqual(1, lager_test_backend:count()),
             ?assertEqual(ok, file:delete(TestLog)),
-            ?assertEqual(ok, safe_write_file(TestLog, "")),
+            ?assertEqual(ok, lager_test_util:safe_write_file(TestLog, "")),
             {ok, FInfo0} = file:read_file_info(TestLog, [raw]),
             FInfo1 = FInfo0#file_info{mode = 0},
             ?assertEqual(ok, file:write_file_info(TestLog, FInfo1)),
@@ -708,7 +708,7 @@ filesystem_test_() ->
         fun() ->
             {ok, TestDir} = lager_util:get_test_dir(),
             TestLog = filename:join(TestDir, "test.log"),
-            ?assertEqual(ok, safe_write_file(TestLog, [])),
+            ?assertEqual(ok, lager_test_util:safe_write_file(TestLog, [])),
 
             {ok, FInfo} = file:read_file_info(TestLog, [raw]),
             OldPerms = FInfo#file_info.mode,
@@ -739,7 +739,7 @@ filesystem_test_() ->
             lager:log(error, self(), "Test message1"),
             ?assertEqual(1, lager_test_backend:count()),
             ?assertEqual(ok, file:delete(TestLog)),
-            ?assertEqual(ok, safe_write_file(TestLog, "")),
+            ?assertEqual(ok, lager_test_util:safe_write_file(TestLog, "")),
             lager:log(error, self(), "Test message2"),
             ?assertEqual(2, lager_test_backend:count()),
             {ok, Bin} = file:read_file(TestLog),
@@ -997,7 +997,7 @@ trace_files_test_() ->
             Events  = filename:join(TestDir, "events.log"),
 
             ok = error_logger:tty(false),
-            ok = safe_application_load(lager),
+            ok = lager_test_util:safe_application_load(lager),
             ok = application:set_env(lager, handlers, [
                      {lager_file_backend, [
                          {file, Log},
@@ -1068,10 +1068,10 @@ formatting_test_() ->
             {ok, TestDir} = lager_util:get_test_dir(),
             Log1 = filename:join(TestDir, "test.log"),
             Log2 = filename:join(TestDir, "test2.log"),
-            ?assertEqual(ok, safe_write_file(Log1, [])),
-            ?assertEqual(ok, safe_write_file(Log2, [])),
+            ?assertEqual(ok, lager_test_util:safe_write_file(Log1, [])),
+            ?assertEqual(ok, lager_test_util:safe_write_file(Log2, [])),
             ok = error_logger:tty(false),
-            ok = safe_application_load(lager),
+            ok = lager_test_util: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(),
@@ -1159,26 +1159,4 @@ config_validation_test_() ->
         }
     ].
 
-safe_application_load(App) ->
-    case application:load(App) of
-        ok ->
-            ok;
-        {error, {already_loaded, App}} ->
-            ok;
-        Error ->
-            ?assertEqual(ok, Error)
-    end.
-
-safe_write_file(File, Content) ->
-    % Note: ensures that the new creation time is at least one second
-    % in the future
-    ?assertEqual(ok, file:write_file(File, Content)),
-    Ctime0 = calendar:local_time(),
-    Ctime0Sec = calendar:datetime_to_gregorian_seconds(Ctime0),
-    Ctime1Sec = Ctime0Sec + 1,
-    Ctime1 = calendar:gregorian_seconds_to_datetime(Ctime1Sec),
-    {ok, FInfo0} = file:read_file_info(File, [raw]),
-    FInfo1 = FInfo0#file_info{ctime = Ctime1},
-    ?assertEqual(ok, file:write_file_info(File, FInfo1, [raw])).
-
 -endif.

src/lager_util.erl

@@ -597,7 +597,7 @@ maybe_flush(Flag, #lager_shaper{} = S) when is_boolean(Flag) ->
 
 -spec has_file_changed(Name :: file:name_all(),
                        Inode0 :: pos_integer(),
-                       Ctime0 :: file:date_time()) -> {boolean(), file:file_info()}.
+                       Ctime0 :: file:date_time()) -> {boolean(), file:file_info() | undefined}.
 has_file_changed(Name, Inode0, Ctime0) ->
     {OsType, _} = os:type(),
     F = file:read_file_info(Name, [raw]),
@@ -868,7 +868,7 @@ create_test_dir() ->
 get_test_dir() ->
     case application:get_env(lager, test_dir) of
         undefined ->
-            create_test_dir(),
+            ok = create_test_dir(),
             get_test_dir();
         {ok, _}=Res ->
             Res
@@ -891,6 +891,7 @@ delete_test_dir() ->
     ok = delete_test_dir(TestDir).
 
 delete_test_dir(TestDir) ->
+    ok = application:unset_env(lager, test_dir),
     {OsType, _} = os:type(),
     ok = case {OsType, otp_version()} of
              {win32, _} ->
@@ -900,8 +901,7 @@ delete_test_dir(TestDir) ->
                  os:cmd("rm -rf " ++ TestDir);
              {unix, _} ->
                  do_delete_test_dir(TestDir)
-         end,
-    ok = application:unset_env(lager, test_dir).
+         end.
 
 do_delete_test_dir(Dir) ->
     ListRet = file:list_dir_all(Dir),

test/lager_test_util.erl

@@ -0,0 +1,52 @@
+%% -------------------------------------------------------------------
+%%
+%% Copyright (c) 2011-2017 Basho Technologies, Inc.
+%%
+%% This file is provided to you under the Apache License,
+%% Version 2.0 (the "License"); you may not use this file
+%% except in compliance with the License.  You may obtain
+%% a copy of the License at
+%%
+%%   http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing,
+%% software distributed under the License is distributed on an
+%% "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+%% KIND, either express or implied.  See the License for the
+%% specific language governing permissions and limitations
+%% under the License.
+%%
+%% -------------------------------------------------------------------
+
+-module(lager_test_util).
+
+-ifdef(TEST).
+
+-export([safe_application_load/1, safe_write_file/2]).
+
+-include_lib("eunit/include/eunit.hrl").
+-include_lib("kernel/include/file.hrl").
+
+safe_application_load(App) ->
+    case application:load(App) of
+        ok ->
+            ok;
+        {error, {already_loaded, App}} ->
+            ok;
+        Error ->
+            ?assertEqual(ok, Error)
+    end.
+
+safe_write_file(File, Content) ->
+    % Note: ensures that the new creation time is at least one second
+    % in the future
+    ?assertEqual(ok, file:write_file(File, Content)),
+    Ctime0 = calendar:local_time(),
+    Ctime0Sec = calendar:datetime_to_gregorian_seconds(Ctime0),
+    Ctime1Sec = Ctime0Sec + 1,
+    Ctime1 = calendar:gregorian_seconds_to_datetime(Ctime1Sec),
+    {ok, FInfo0} = file:read_file_info(File, [raw]),
+    FInfo1 = FInfo0#file_info{ctime = Ctime1},
+    ?assertEqual(ok, file:write_file_info(File, FInfo1, [raw])).
+
+-endif.