You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by Filipe David Manana <fd...@gmail.com> on 2012/11/01 12:35:13 UTC
Re: git commit: COUCHDB-1424 Fix etap to not consume any message
On Wed, Oct 31, 2012 at 8:17 PM, Bob Dionne <bo...@cloudant.com> wrote:
> 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 <di...@dionne-associates.com> 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
Thanks for looking at it.
Test 140- will fail without the patch as well. Fails often for me (Jan
tried it as well recently, same thing).
>>
>> On Oct 31, 2012, at 3:10 PM, Filipe David Manana <fd...@gmail.com> wrote:
>>
>>> On Wed, Oct 31, 2012 at 8:01 PM, Bob Dionne
>>> <di...@dionne-associates.com> 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 <fd...@gmail.com> wrote:
>>>>
>>>>> On Wed, Oct 31, 2012 at 4:32 PM, Paul Davis <pa...@gmail.com> 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, <fd...@apache.org> 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 <fd...@apache.org>
>>>>>>> Authored: Wed Oct 31 16:19:48 2012 +0100
>>>>>>> Committer: Filipe David Borba Manana <fd...@apache.org>
>>>>>>> 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."
>>
>
--
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."