From f43fa83bcc84fd1d77eb57827ced8bd14be580b6 Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Mon, 31 Oct 2011 23:40:38 -0400 Subject: [PATCH 1/2] Fix a bug with how the remaining free space is calculated --- src/lager_format.erl | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lager_format.erl b/src/lager_format.erl index b3fa86e..543ac07 100644 --- a/src/lager_format.erl +++ b/src/lager_format.erl @@ -145,7 +145,7 @@ build([], Acc, MaxLen, _O) -> build2([{C,As,F,Ad,P,Pad,Enc}|Cs], Count, MaxLen) -> {S, Len} = control2(C, As, F, Ad, P, Pad, Enc, MaxLen div Count), - [S|build2(Cs, Count - 1, MaxLen - abs(Len))]; + [S|build2(Cs, Count - 1, MaxLen - Len)]; build2([C|Cs], Count, MaxLen) -> [C|build2(Cs, Count, MaxLen)]; build2([], _, _) -> []. @@ -221,27 +221,27 @@ control(C, A, F, Adj, P, Pad, Enc, L) -> control2($w, [A], F, Adj, P, Pad, _Enc, L) -> Term = lager_trunc_io:fprint(A, L, [{lists_as_strings, false}]), Res = term(Term, F, Adj, P, Pad), - {Res, L - lists:flatlength(Res)}; + {Res, lists:flatlength(Res)}; control2($p, [A], F, Adj, P, Pad, _Enc, L) -> Term = lager_trunc_io:fprint(A, L, [{lists_as_strings, true}]), Res = term(Term, F, Adj, P, Pad), - {Res, L - lists:flatlength(Res)}; + {Res, lists:flatlength(Res)}; control2($W, [A,Depth], F, Adj, P, Pad, _Enc, L) when is_integer(Depth) -> Term = lager_trunc_io:fprint(A, L, [{depth, Depth}, {lists_as_strings, false}]), Res = term(Term, F, Adj, P, Pad), - {Res, L - lists:flatlength(Res)}; + {Res, lists:flatlength(Res)}; control2($P, [A,Depth], F, Adj, P, Pad, _Enc, L) when is_integer(Depth) -> Term = lager_trunc_io:fprint(A, L, [{depth, Depth}, {lists_as_strings, true}]), Res = term(Term, F, Adj, P, Pad), - {Res, L - lists:flatlength(Res)}; + {Res, lists:flatlength(Res)}; control2($s, [L0], F, Adj, P, Pad, latin1, L) -> List = lager_trunc_io:fprint(maybe_flatten(L0), L, [{force_strings, true}]), Res = string(List, F, Adj, P, Pad), - {Res, L - lists:flatlength(Res)}; + {Res, lists:flatlength(Res)}; control2($s, [L0], F, Adj, P, Pad, unicode, L) -> List = lager_trunc_io:fprint(unicode:characters_to_list(L0), L, [{force_strings, true}]), Res = uniconv(string(List, F, Adj, P, Pad)), - {Res, L - lists:flatlength(Res)}. + {Res, lists:flatlength(Res)}. maybe_flatten(X) when is_list(X) -> lists:flatten(X); From 5068e293838062c7af8d5b1a44848c59ddb1d7f2 Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Tue, 1 Nov 2011 11:33:40 -0400 Subject: [PATCH 2/2] Fix a couple minor issues with length calculation, add tests --- src/lager_trunc_io.erl | 39 ++++++++++++++++++++++++++++++++----- test/lager_test_backend.erl | 4 ++-- test/trunc_io_eqc.erl | 12 ++++++++++-- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/lager_trunc_io.erl b/src/lager_trunc_io.erl index 0a11447..9eef041 100644 --- a/src/lager_trunc_io.erl +++ b/src/lager_trunc_io.erl @@ -193,7 +193,7 @@ print(List, Max, Options) when is_list(List) -> true -> alist_start(List, Max, dec_depth(Options)); _ -> - {R, Len} = list_body(List, Max, dec_depth(Options), false), + {R, Len} = list_body(List, Max - 2, dec_depth(Options), false), {[$[, R, $]], Len + 2} end. @@ -216,7 +216,7 @@ list_body(X, Max, Options, _Tuple) -> %% improper list {[$|,List], Len + 1}. list_bodyc([], _Max, _Options, _Tuple) -> {[], 0}; -list_bodyc(_, Max, _Options, _Tuple) when Max < 4 -> {",...", 3}; +list_bodyc(_, Max, _Options, _Tuple) when Max < 5 -> {",...", 4}; list_bodyc([H|T], Max, #print_options{depth=Depth} = Options, Tuple) -> {List, Len} = print(H, Max, dec_depth(Options)), {Final, FLen} = list_bodyc(T, Max - Len - 1, Options, Tuple), @@ -239,7 +239,7 @@ list_bodyc(X, Max, Options, _Tuple) -> %% improper list alist_start([], _Max, #print_options{force_strings=true}) -> {"", 0}; alist_start([], _Max, _Options) -> {"[]", 2}; alist_start(_, Max, _Options) when Max < 4 -> {"...", 3}; -alist_start(_, _Max, #print_options{depth=0}) -> {"[...]", 3}; +alist_start(_, _Max, #print_options{depth=0}) -> {"[...]", 5}; alist_start(L, Max, #print_options{force_strings=true} = Options) -> alist(L, Max, Options); alist_start([H|T], Max, Options) when is_integer(H), H >= 16#20, H =< 16#7e -> % definitely printable @@ -427,6 +427,7 @@ quote_strip_test() -> binary_printing_test() -> ?assertEqual("<<>>", lists:flatten(format("~p", [<<>>], 50))), ?assertEqual("<<..>>", lists:flatten(format("~p", [<<"hi">>], 0))), + ?assertEqual("<<...>>", lists:flatten(format("~p", [<<"hi">>], 1))), ?assertEqual("<<\"hello\">>", lists:flatten(format("~p", [<<$h, $e, $l, $l, $o>>], 50))), ?assertEqual("<<\"hello\">>", lists:flatten(format("~p", [<<"hello">>], 50))), ?assertEqual("<<104,101,108,108,111>>", lists:flatten(format("~w", [<<"hello">>], 50))), @@ -448,7 +449,9 @@ list_printing_test() -> ?assertEqual("\"\\rabc\"", lists:flatten(format("~p", [[13,$a, $b, $c]], 50))), ?assertEqual("[1,2,3|4]", lists:flatten(format("~p", [[1, 2, 3|4]], 50))), ?assertEqual("[...]", lists:flatten(format("~p", [[1, 2, 3,4]], 4))), - ?assertEqual("[1,...]", lists:flatten(format("~p", [[1, 2, 3,4]], 6))), + ?assertEqual("[1,...]", lists:flatten(format("~p", [[1, 2, 3, 4]], 6))), + ?assertEqual("[1,...]", lists:flatten(format("~p", [[1, 2, 3, 4]], 7))), + ?assertEqual("[1,2,...]", lists:flatten(format("~p", [[1, 2, 3, 4]], 8))), ?assertEqual("[1|4]", lists:flatten(format("~p", [[1|4]], 50))), ?assertEqual("[1]", lists:flatten(format("~p", [[1]], 50))), ?assertError(badarg, lists:flatten(format("~s", [[1|4]], 50))), @@ -459,15 +462,41 @@ list_printing_test() -> ?assertEqual("\"\\rhello world\\r\\n\"", lists:flatten(format("~p", ["\rhello world\r\n"], 50))), ?assertEqual("[13,104,101,108,108,111,32,119,111,114,108,100,13,10]", lists:flatten(format("~w", ["\rhello world\r\n"], 60))), ?assertEqual("...", lists:flatten(format("~s", ["\rhello world\r\n"], 3))), + ?assertEqual("[22835963083295358096932575511191922182123945984,...]", + lists:flatten(format("~p", [ + [22835963083295358096932575511191922182123945984, + 22835963083295358096932575511191922182123945984]], 9))), + ?assertEqual("[22835963083295358096932575511191922182123945984,...]", + lists:flatten(format("~p", [ + [22835963083295358096932575511191922182123945984, + 22835963083295358096932575511191922182123945984]], 53))), ok. tuple_printing_test() -> ?assertEqual("{}", lists:flatten(format("~p", [{}], 50))), ?assertEqual("{}", lists:flatten(format("~w", [{}], 50))), ?assertError(badarg, lists:flatten(format("~s", [{}], 50))), + ?assertEqual("{...}", lists:flatten(format("~p", [{foo}], 1))), + ?assertEqual("{...}", lists:flatten(format("~p", [{foo}], 2))), ?assertEqual("{...}", lists:flatten(format("~p", [{foo}], 3))), + ?assertEqual("{...}", lists:flatten(format("~p", [{foo}], 4))), + ?assertEqual("{...}", lists:flatten(format("~p", [{foo}], 5))), ?assertEqual("{foo,...}", lists:flatten(format("~p", [{foo,bar}], 6))), - ?assertEqual("{foo,bar}", lists:flatten(format("~p", [{foo,bar}], 9))), + ?assertEqual("{foo,...}", lists:flatten(format("~p", [{foo,bar}], 7))), + ?assertEqual("{foo,...}", lists:flatten(format("~p", [{foo,bar}], 9))), + ?assertEqual("{foo,bar}", lists:flatten(format("~p", [{foo,bar}], 10))), + ?assertEqual("{22835963083295358096932575511191922182123945984,...}", + lists:flatten(format("~w", [ + {22835963083295358096932575511191922182123945984, + 22835963083295358096932575511191922182123945984}], 10))), + ?assertEqual("{22835963083295358096932575511191922182123945984,...}", + lists:flatten(format("~w", [ + {22835963083295358096932575511191922182123945984, + bar}], 10))), + ?assertEqual("{22835963083295358096932575511191922182123945984,...}", + lists:flatten(format("~w", [ + {22835963083295358096932575511191922182123945984, + 22835963083295358096932575511191922182123945984}], 53))), ok. unicode_test() -> diff --git a/test/lager_test_backend.erl b/test/lager_test_backend.erl index 85a6ee2..725aa76 100644 --- a/test/lager_test_backend.erl +++ b/test/lager_test_backend.erl @@ -632,8 +632,8 @@ error_logger_redirect_test_() -> sync_error_logger:error_report(crash_report, [[{pid, self()}, {error_info, {error, {system_limit,[{wtf,boom,[string:copies("aaaa", 4096)]}]}, []}}], []]), _ = gen_event:which_handlers(error_logger), {_, _, Msg} = pop(), - ?assert(length(lists:flatten(Msg)) > 500), - ?assert(length(lists:flatten(Msg)) < 700) + ?assert(length(lists:flatten(Msg)) > 600), + ?assert(length(lists:flatten(Msg)) < 650) end }, {"messages should not be generated if they don't satisfy the threshold", diff --git a/test/trunc_io_eqc.erl b/test/trunc_io_eqc.erl index 13f8e6a..b363640 100644 --- a/test/trunc_io_eqc.erl +++ b/test/trunc_io_eqc.erl @@ -139,7 +139,13 @@ gen_char() -> prop_format() -> ?FORALL({FmtArgs, MaxLen}, {gen_fmt_args(), gen_max_len()}, begin - %% trunc_io does not correctly calc safe size of pid/port/numbers/funs + %% Because trunc_io will print '...' when its running out of + %% space, even if the remaining space is less than 3, it + %% doesn't *exactly* stick to the specified limit. + + %% Also, since we don't truncate terms not printed with + %% ~p/~P/~w/~W/~s, we also need to calculate the wiggle room + %% for those. Hence the fudge factor calculated below. FudgeLen = calculate_fudge(FmtArgs, 50), {FmtStr, Args} = build_fmt_args(FmtArgs), try @@ -191,7 +197,9 @@ calculate_fudge([{Fmt, Arg}|T], Acc) when Fmt == "~f"; Fmt == "~10.f"; Fmt == "~g"; Fmt == "~10.g"; Fmt == "~e"; Fmt == "~10.e"; - Fmt == "~x"; Fmt == "~X" -> + Fmt == "~x"; Fmt == "~X"; + Fmt == "~B"; Fmt == "~b"; Fmt == "~36B"; + Fmt == "~.10#"; Fmt == "~10+" -> calculate_fudge(T, Acc + length(lists:flatten(io_lib:format(Fmt, [Arg])))); calculate_fudge([_|T], Acc) -> calculate_fudge(T, Acc).