also +1 on this patch just based on inspection, it looks like the right thing to do
On Oct 31, 2012, at 3:16 PM, Bob Dionne <[email protected]> wrote: > oops, sorry, wrong paste, it is irrelevant. I meant to paste: > > ./test/etap/120-stats-collect.t ........... ok > ./test/etap/121-stats-aggregates.t ........ ok > ./test/etap/130-attachments-md5.t ......... ok > ./test/etap/140-attachment-comp.t ......... Failed 9/85 subtests > ./test/etap/150-invalid-view-seq.t ........ ok > ./test/etap/160-vhosts.t .................. ok > ./test/etap/170-os-daemons.t .............. ok > ./test/etap/171-os-daemons- > > 140-attach .. is the bad one. I'll poke at it some more when I can > > Overall I have to say I seldom run "make check" these days as it's gotten to > long-winded > > On Oct 31, 2012, at 3:10 PM, Filipe David Manana <[email protected]> wrote: > >> On Wed, Oct 31, 2012 at 8:01 PM, Bob Dionne >> <[email protected]> wrote: >>> Hi Filipe, >>> >>> I tested this new branch and it seems to have issues (at least on my >>> machine) : >>> >>> ok 44 reduce_false >>> ok 45 reduce_false_temp >>> not ok 46 replication >>> Reason: expected '"tony"', got 'null' >>> Trace back (most recent call first): >>> >>> 0: >>> Error("expected '\"tony\"', got 'null'") >>> 46: /Users/bitdiddle/emacs/foo/couchdb/test/javascript/cli_runner.js >>> T(false,"expected '\"tony\"', got 'null'",undefined) >>> 324: >>> /Users/bitdiddle/emacs/foo/couchdb/share/www/script/couch_test_runner.js >>> TEquals("tony",null) >>> >>> >>> I also retested the original patch with and without the sleep(100) and >>> without still fails, though it's move around a bit in the etaps, so this >>> could be other issues. >> >> Bob, that failure is irrelevant I think. It's a js test, I only >> modified etap.erl, therefore only etap tests matter. >> >> thanks for testing >> >>> >>> Cheers, >>> Bob >>> >>> On Oct 31, 2012, at 11:35 AM, Filipe David Manana <[email protected]> >>> wrote: >>> >>>> On Wed, Oct 31, 2012 at 4:32 PM, Paul Davis <[email protected]> >>>> wrote: >>>>> Nice find >>>> >>>> Say thanks to Damien :) >>>> Fixed in an older etap version, I just ported it to latest etap. >>>> >>>> Robert (Dionne), wanna test this to see if it fixes the hangs you used >>>> to have with OTP R15Bx? >>>> thanks >>>> >>>>> >>>>> On Wed, Oct 31, 2012 at 11:29 AM, <[email protected]> wrote: >>>>>> Updated Branches: >>>>>> refs/heads/1424-fix-etap-consuming-any-test-message [created] 67d531b02 >>>>>> >>>>>> >>>>>> COUCHDB-1424 Fix etap to not consume any message >>>>>> >>>>>> Turns out that etap consumes any message in the mailbox in >>>>>> some cases. This can make some tests that use message passing >>>>>> hang, as etap itself consumes the messages. >>>>>> >>>>>> >>>>>> Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo >>>>>> Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/67d531b0 >>>>>> Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/67d531b0 >>>>>> Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/67d531b0 >>>>>> >>>>>> Branch: refs/heads/1424-fix-etap-consuming-any-test-message >>>>>> Commit: 67d531b028503b316c9ff8842b8fb34ea75db29d >>>>>> Parents: 8ccf696 >>>>>> Author: Filipe David Borba Manana <[email protected]> >>>>>> Authored: Wed Oct 31 16:19:48 2012 +0100 >>>>>> Committer: Filipe David Borba Manana <[email protected]> >>>>>> Committed: Wed Oct 31 16:19:48 2012 +0100 >>>>>> >>>>>> ---------------------------------------------------------------------- >>>>>> src/etap/etap.erl | 21 +++++++++++---------- >>>>>> 1 files changed, 11 insertions(+), 10 deletions(-) >>>>>> ---------------------------------------------------------------------- >>>>>> >>>>>> >>>>>> http://git-wip-us.apache.org/repos/asf/couchdb/blob/67d531b0/src/etap/etap.erl >>>>>> ---------------------------------------------------------------------- >>>>>> diff --git a/src/etap/etap.erl b/src/etap/etap.erl >>>>>> index 7380013..ae3896c 100644 >>>>>> --- a/src/etap/etap.erl >>>>>> +++ b/src/etap/etap.erl >>>>>> @@ -120,14 +120,14 @@ plan(N) when is_integer(N), N > 0 -> >>>>>> %% @doc End the current test plan and output test results. >>>>>> %% @todo This should probably be done in the test_server process. >>>>>> end_tests() -> >>>>>> - timer:sleep(100), >>>>>> + Ref = make_ref(), >>>>>> case whereis(etap_server) of >>>>>> - undefined -> self() ! true; >>>>>> - _ -> etap_server ! {self(), state} >>>>>> + undefined -> self() ! {Ref, true}; >>>>>> + _ -> etap_server ! {self(), state, Ref} >>>>>> end, >>>>>> - State = receive X -> X end, >>>>>> + State = receive {Ref, X} -> X end, >>>>>> if >>>>>> - State#test_state.planned == -1 -> >>>>>> + is_record(State, test_state) andalso State#test_state.planned >>>>>> == -1 -> >>>>>> io:format("1..~p~n", [State#test_state.count]); >>>>>> true -> >>>>>> ok >>>>>> @@ -564,8 +564,8 @@ test_server(State) -> >>>>>> count = State#test_state.count + 1, >>>>>> fail = State#test_state.fail + 1 >>>>>> }; >>>>>> - {From, state} -> >>>>>> - From ! State, >>>>>> + {From, state, Ref} -> >>>>>> + From ! {Ref, State}, >>>>>> State; >>>>>> {_From, diag, Message} -> >>>>>> io:format("~s~n", [Message]), >>>>>> @@ -573,8 +573,8 @@ test_server(State) -> >>>>>> {From, count} -> >>>>>> From ! State#test_state.count, >>>>>> State; >>>>>> - {From, is_skip} -> >>>>>> - From ! State#test_state.skip, >>>>>> + {From, is_skip, Ref} -> >>>>>> + From ! {Ref, State#test_state.skip}, >>>>>> State; >>>>>> done -> >>>>>> exit(normal) >>>>>> @@ -584,7 +584,8 @@ test_server(State) -> >>>>>> %% @private >>>>>> %% @doc Process the result of a test and send it to the etap_server >>>>>> process. >>>>>> mk_tap(Result, Desc) -> >>>>>> - IsSkip = lib:sendw(etap_server, is_skip), >>>>>> + etap_server ! {self(), is_skip, Ref = make_ref()} , >>>>>> + receive {Ref, IsSkip} -> ok end, >>>>>> case [IsSkip, Result] of >>>>>> [_, true] -> >>>>>> etap_server ! {self(), pass, Desc}, >>>>>> >>>> >>>> >>>> >>>> -- >>>> Filipe David Manana, >>>> >>>> "Reasonable men adapt themselves to the world. >>>> Unreasonable men adapt the world to themselves. >>>> That's why all progress depends on unreasonable men." >>> >> >> >> >> -- >> Filipe David Manana, >> >> "Reasonable men adapt themselves to the world. >> Unreasonable men adapt the world to themselves. >> That's why all progress depends on unreasonable men." >
