Re: svn commit: r925497 - /couchdb/trunk/src/couchdb/couch_rep.erl

2010-03-21 Thread Filipe David Manana
Thanks Nikolai for reporting the issue.

That was a case that somehow didn't get properly tested when I implemented
the feature.

cheers

On Sun, Mar 21, 2010 at 8:59 PM, Nikolai Teofilov wrote:

> Thanks Filipe!
>
> This is very nice and important contribution!
>
> Cheers
> Nikolai
>
> On 21.03.2010, at 21:21, Jan Lehnardt wrote:
>
> > Done and done.
> >
> > Thanks again Filipe.
> >
> > Cheers
> > Jan
> > --
> >
> > On 21 Mar 2010, at 07:10, Filipe David Manana wrote:
> >
> >> Jan,
> >>
> >> It would also be a good idea to do the same with the filtered
> replication specific parameters ("filter" and "query_params"). The patch
> attached to this email does it.
> >>
> >> Also, don't forget the regression test attached to
> https://issues.apache.org/jira/browse/COUCHDB-703
> >>
> >> cheers
> >>
> >> On Sat, Mar 20, 2010 at 12:22 AM,  wrote:
> >> Author: jan
> >> Date: Sat Mar 20 00:22:04 2010
> >> New Revision: 925497
> >>
> >> URL: http://svn.apache.org/viewvc?rev=925497&view=rev
> >> Log:
> >> backwards compatible ids for non-docid replications
> >>
> >> Modified:
> >>   couchdb/trunk/src/couchdb/couch_rep.erl
> >>
> >> Modified: couchdb/trunk/src/couchdb/couch_rep.erl
> >> URL:
> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_rep.erl?rev=925497&r1=925496&r2=925497&view=diff
> >>
> ==
> >> --- couchdb/trunk/src/couchdb/couch_rep.erl (original)
> >> +++ couchdb/trunk/src/couchdb/couch_rep.erl Sat Mar 20 00:22:04 2010
> >> @@ -455,7 +455,12 @@ make_replication_id({Props}, UserCtx) ->
> >>QueryParams = proplists:get_value(<<"query_params">>, Props),
> >>DocIds = proplists:get_value(<<"doc_ids">>, Props),
> >>Base = couch_util:to_hex(erlang:md5(
> >> -term_to_binary([HostName, Src, Tgt, Filter, QueryParams,
> DocIds])
> >> +case DocIds of
> >> +undefined ->
> >> +term_to_binary([HostName, Src, Tgt, Filter, QueryParams]);
> >> +DocIds ->
> >> +term_to_binary([HostName, Src, Tgt, Filter, QueryParams,
> DocIds])
> >> +end
> >>)),
> >>Extension = maybe_append_options(
> >>[<<"continuous">>, <<"create_target">>], Props),
> >>
> >>
> >>
> >>
> >>
> >> --
> >> Filipe David Manana,
> >> fdman...@gmail.com
> >>
> >> "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,
fdman...@gmail.com

"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."


Re: svn commit: r925497 - /couchdb/trunk/src/couchdb/couch_rep.erl

2010-03-21 Thread Nikolai Teofilov
Thanks Filipe!

This is very nice and important contribution!

Cheers
Nikolai

On 21.03.2010, at 21:21, Jan Lehnardt wrote:

> Done and done.
> 
> Thanks again Filipe.
> 
> Cheers
> Jan
> --
> 
> On 21 Mar 2010, at 07:10, Filipe David Manana wrote:
> 
>> Jan,
>> 
>> It would also be a good idea to do the same with the filtered replication 
>> specific parameters ("filter" and "query_params"). The patch attached to 
>> this email does it.
>> 
>> Also, don't forget the regression test attached to 
>> https://issues.apache.org/jira/browse/COUCHDB-703
>> 
>> cheers
>> 
>> On Sat, Mar 20, 2010 at 12:22 AM,  wrote:
>> Author: jan
>> Date: Sat Mar 20 00:22:04 2010
>> New Revision: 925497
>> 
>> URL: http://svn.apache.org/viewvc?rev=925497&view=rev
>> Log:
>> backwards compatible ids for non-docid replications
>> 
>> Modified:
>>   couchdb/trunk/src/couchdb/couch_rep.erl
>> 
>> Modified: couchdb/trunk/src/couchdb/couch_rep.erl
>> URL: 
>> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_rep.erl?rev=925497&r1=925496&r2=925497&view=diff
>> ==
>> --- couchdb/trunk/src/couchdb/couch_rep.erl (original)
>> +++ couchdb/trunk/src/couchdb/couch_rep.erl Sat Mar 20 00:22:04 2010
>> @@ -455,7 +455,12 @@ make_replication_id({Props}, UserCtx) ->
>>QueryParams = proplists:get_value(<<"query_params">>, Props),
>>DocIds = proplists:get_value(<<"doc_ids">>, Props),
>>Base = couch_util:to_hex(erlang:md5(
>> -term_to_binary([HostName, Src, Tgt, Filter, QueryParams, DocIds])
>> +case DocIds of
>> +undefined ->
>> +term_to_binary([HostName, Src, Tgt, Filter, QueryParams]);
>> +DocIds ->
>> +term_to_binary([HostName, Src, Tgt, Filter, QueryParams, 
>> DocIds])
>> +end
>>)),
>>Extension = maybe_append_options(
>>[<<"continuous">>, <<"create_target">>], Props),
>> 
>> 
>> 
>> 
>> 
>> -- 
>> Filipe David Manana,
>> fdman...@gmail.com
>> 
>> "Reasonable men adapt themselves to the world.
>> Unreasonable men adapt the world to themselves.
>> That's why all progress depends on unreasonable men."
>> 
>> 
> 



Re: svn commit: r925497 - /couchdb/trunk/src/couchdb/couch_rep.erl

2010-03-21 Thread Jan Lehnardt
Done and done.

Thanks again Filipe.

Cheers
Jan
--

On 21 Mar 2010, at 07:10, Filipe David Manana wrote:

> Jan,
> 
> It would also be a good idea to do the same with the filtered replication 
> specific parameters ("filter" and "query_params"). The patch attached to this 
> email does it.
> 
> Also, don't forget the regression test attached to 
> https://issues.apache.org/jira/browse/COUCHDB-703
> 
> cheers
> 
> On Sat, Mar 20, 2010 at 12:22 AM,  wrote:
> Author: jan
> Date: Sat Mar 20 00:22:04 2010
> New Revision: 925497
> 
> URL: http://svn.apache.org/viewvc?rev=925497&view=rev
> Log:
> backwards compatible ids for non-docid replications
> 
> Modified:
>couchdb/trunk/src/couchdb/couch_rep.erl
> 
> Modified: couchdb/trunk/src/couchdb/couch_rep.erl
> URL: 
> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_rep.erl?rev=925497&r1=925496&r2=925497&view=diff
> ==
> --- couchdb/trunk/src/couchdb/couch_rep.erl (original)
> +++ couchdb/trunk/src/couchdb/couch_rep.erl Sat Mar 20 00:22:04 2010
> @@ -455,7 +455,12 @@ make_replication_id({Props}, UserCtx) ->
> QueryParams = proplists:get_value(<<"query_params">>, Props),
> DocIds = proplists:get_value(<<"doc_ids">>, Props),
> Base = couch_util:to_hex(erlang:md5(
> -term_to_binary([HostName, Src, Tgt, Filter, QueryParams, DocIds])
> +case DocIds of
> +undefined ->
> +term_to_binary([HostName, Src, Tgt, Filter, QueryParams]);
> +DocIds ->
> +term_to_binary([HostName, Src, Tgt, Filter, QueryParams, DocIds])
> +end
> )),
> Extension = maybe_append_options(
> [<<"continuous">>, <<"create_target">>], Props),
> 
> 
> 
> 
> 
> -- 
> Filipe David Manana,
> fdman...@gmail.com
> 
> "Reasonable men adapt themselves to the world.
> Unreasonable men adapt the world to themselves.
> That's why all progress depends on unreasonable men."
> 
> 



[jira] Closed: (COUCHDB-703) doc_ids replicate always the same document list

2010-03-21 Thread Jan Lehnardt (JIRA)

 [ 
https://issues.apache.org/jira/browse/COUCHDB-703?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jan Lehnardt closed COUCHDB-703.


Resolution: Fixed

Applied in trunk (r925883) and 0.11.x (r925884).

> doc_ids  replicate always the same document list 
> -
>
> Key: COUCHDB-703
> URL: https://issues.apache.org/jira/browse/COUCHDB-703
> Project: CouchDB
>  Issue Type: Bug
>  Components: Replication
>Affects Versions: 0.11
> Environment: windows xp and mac osx
>Reporter: Nikolai Teofilov
> Fix For: 0.11
>
> Attachments: couchdb-703-regression-test-trunk.patch, 
> couchdb-703-trunk.patch
>
>
> curl -X PUT http://192.168.10.239:5984/test
> curl -X PUT http://192.168.10.239/test/doc1 -d '{}'
> curl -X PUT http://192.168.10.239/test/doc2 -d '{}'
> curl -X PUT http://192.168.10.239/test/doc3 -d '{}'
> curl -X PUT http://192.168.10.239/test-replica 
> curl -X POST http://127.0.0.1:5984/_replicate -d 
> '{"source":"test","target":"test-replica", "doc_ids": ["doc1", "doc2", 
> "doc3"]}'
> replicate all documents. three documents.
> curl -X DELETE http://127.0.0.1:5984/test-replica
> create again the database:
> curl -X PUT http://127.0.0.1:5984/test-replica
> replicate just two of the documents:
> curl -X POST http://127.0.0.1:5984/_replicate -d 
> '{"source":"test","target":"test-replica", "doc_ids": ["doc1", "doc2"]}'
> will still replicate the all three documents.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.



Re: Test suite blocking release

2010-03-21 Thread Paul Davis
>> I don't have a dog in this fight (.ie. a paying customer) so this failure 
>> doesn't bother me. With respect to policy, given the various bogocities of 
>> browsers, I'd recommend something like these CLI tests plus the etaps ought 
>> to be the "official"  tests for vetting, and part of the build
>
> Not that I disagree, but part (most?) of the appeal of the browser based 
> tests are that they run in a real-world client instead of the lab that is 
> couchjs+http :)
>

The tests in Futon should be treated just as the tests for any other
client library: a way to check the client. Instead we treat them as
75% of the tests for the CouchDB implementation. That's bad. Its like
surgery with pruning shears. I don't know if the JS CLI runner is the
way forward or not, or some other scripting language. But there should
be a clear separation between what goes into Futon and what doesn't
based on the "What are we trying to test?" question.

On the flip side, I also think that the Futon tests would be a great
place to 'document' the entire API. Then they could double as a "this
browser is compatible with CouchDB" screen as well as a "this server
is Couch-y" regardless of what the actual implementation is.

Paul


Re: Test suite blocking release

2010-03-21 Thread Paul Davis
On Sun, Mar 21, 2010 at 1:16 PM, Jan Lehnardt  wrote:
>
> On 21 Mar 2010, at 12:10, Noah Slater wrote:
>
>
>> Are they integrated into the build system?
>
> No.
>

Just in case, to be specific, the CLI tests are just running the Futon
tests from the command line. I added the few bits so that I could run
the tests from the command line instead of having to switch over to
futon and get irritated at how it locks up when running the tests.

I've also specifically not included them in the build system because
they require a running instance of CouchDB. This plays badly with
build bot attempting to run two builds on the same machine. in the
future when we get around to making the "couchdb on random port"
behavior more workable there'd be more of an incentive to add them.

Paul

> Cheers
> Jan
> --
>
>


Re: Test suite blocking release

2010-03-21 Thread Jan Lehnardt

On 21 Mar 2010, at 13:38, Robert Dionne wrote:

> Ok Noah,  This is only a test case issue, and not in the changes code as I 
> though. Jan found the issue and it works fine for me now in both FF and CLI. 
> -- Bob

As an added bonus, the test suite should now work 100% in WebKit (for me at 
least :)

Cheers
Jan
--


> 
> 
> On Mar 21, 2010, at 1:30 PM, Jan Lehnardt wrote:
> 
>> 
>> On 21 Mar 2010, at 12:24, Robert Dionne wrote:
>> 
>>> 
>>> 
>>> 
>>> 
>>> On Mar 21, 2010, at 1:16 PM, Jan Lehnardt wrote:
>>> 
 
 On 21 Mar 2010, at 12:10, Noah Slater wrote:
 
> What are the CLI tests, if not the etap tests? Are they integrated into 
> the build system?
 
 The CLI tests are the same as the browser tests, just run through our 
 couchjs binary
 that has custom HTTP extensions to make the xhr work. At this point I 
 don't think it
 is reliable enough to mimic browser behaviour and that we shouldn't use it 
 for vetting
 the state of the code.
>>> 
>>> This is likely true, but in this particular case I think there's a bug in 
>>> the changes code (that I'm trying to dig out). It's nice that it works on 
>>> your machine but on my machine, using FF, it fails often enough. Moreover 
>>> it's been around for a long time now so I figure it's worth figuring out. 
>>> 
>>> I don't have a dog in this fight (.ie. a paying customer) so this failure 
>>> doesn't bother me. With respect to policy, given the various bogocities of 
>>> browsers, I'd recommend something like these CLI tests plus the etaps ought 
>>> to be the "official"  tests for vetting, and part of the build
>> 
>> Not that I disagree, but part (most?) of the appeal of the browser based 
>> tests are that they run in a real-world client instead of the lab that is 
>> couchjs+http :)
>> 
>> Cheers
>> Jan
>> --
>> 
>>> 
>>> 
 
 It is very useful when developing new code to not have to switch to and 
 reload the
 browser over and over again.
 
 Cheers
 Jan
 --
 
 
 
 
> 
> On 21 Mar 2010, at 17:05, Jan Lehnardt wrote:
> 
>> 
>> On 21 Mar 2010, at 06:04, Robert Dionne wrote:
>> 
>>> On Mar 21, 2010, at 4:00 AM, Jan Lehnardt wrote:
>>> 
 
 On 20 Mar 2010, at 20:06, Paul Davis wrote:
 
> On Sat, Mar 20, 2010 at 2:31 PM, Noah Slater  wrote:
>> I think faulty test case should block the release, if I am to have 
>> any
>> future sanity preparing releases. I don't want to delay and longer, 
>> so if
>> you guys are absolutely sure this is a test error and not code 
>> error, then I
>> propose that the test be commented out. Our tests form a contract 
>> between
>> us, internally, and our users. If that contract has a bug, it should 
>> be
>> removed or fixed - or it simply dilutes the importance of contract. 
>> If some
>> one comments out the test, and we agree it is not indicative of an 
>> important
>> bug, I can call the vote within hours.
>> 
> 
> I'd have to agree on this. From the point of view of a release, if a
> test reports a failure then it should be made to not report a failure.
> If that's accomplished by disabling it, then there will be a commit
> with a message that explains why it was disabled and etc and such on
> and so forth.
 
 I'd do that if the test was failing for me :)
>>> 
>>> it's not failing for you when you run changes.js with the CLI ?  Fails 
>>> for me every time. 
>> 
>> I don't consider the CLI tests as part of the official test suite just 
>> yet.
>> 
>> Cheers
>> Jan
>> --
>> 
>>> 
>>> Anyway I poked at this a bit yesterday and am not 100% sure the issue 
>>> is in the test. I tried putting a sleep in with no luck. If my 
>>> understanding of the JS is correct, CouchDB is supposed to be 
>>> synchronous so it's not timing.
>>> 
>>> If someone could comment on the test itself it would be helpful. The 
>>> section of the code that fails:
>>> 
>>> // changes get all_docs style with deleted docs
>>> var doc = {a:1};
>>> db.save(doc);
>>> db.deleteDoc(doc);
>>> var req = CouchDB.request("GET", 
>>> "/test_suite_db/_changes?filter=changes_filter/bop&style=all_docs");
>>> var resp = JSON.parse(req.responseText);
>>> TEquals(3, resp.results.length, "should return matching rows");
>>> 
>>> 
>>> seems odd to me. all_docs as I read the code will return docs with 
>>> deletes and conflicts but in this call the filter bop will not apply to 
>>> the doc {a:1} so I'm not sure what this delete prior to the call is 
>>> about. Anyway I can make it fail in the debugger so perhaps I can find 
>>> the root cause.
>>> 
>>> 
>>> 
>>> 
 

Re: Test suite blocking release

2010-03-21 Thread Robert Dionne
Ok Noah,  This is only a test case issue, and not in the changes code as I 
though. Jan found the issue and it works fine for me now in both FF and CLI. -- 
Bob


On Mar 21, 2010, at 1:30 PM, Jan Lehnardt wrote:

> 
> On 21 Mar 2010, at 12:24, Robert Dionne wrote:
> 
>> 
>> 
>> 
>> 
>> On Mar 21, 2010, at 1:16 PM, Jan Lehnardt wrote:
>> 
>>> 
>>> On 21 Mar 2010, at 12:10, Noah Slater wrote:
>>> 
 What are the CLI tests, if not the etap tests? Are they integrated into 
 the build system?
>>> 
>>> The CLI tests are the same as the browser tests, just run through our 
>>> couchjs binary
>>> that has custom HTTP extensions to make the xhr work. At this point I don't 
>>> think it
>>> is reliable enough to mimic browser behaviour and that we shouldn't use it 
>>> for vetting
>>> the state of the code.
>> 
>> This is likely true, but in this particular case I think there's a bug in 
>> the changes code (that I'm trying to dig out). It's nice that it works on 
>> your machine but on my machine, using FF, it fails often enough. Moreover 
>> it's been around for a long time now so I figure it's worth figuring out. 
>> 
>> I don't have a dog in this fight (.ie. a paying customer) so this failure 
>> doesn't bother me. With respect to policy, given the various bogocities of 
>> browsers, I'd recommend something like these CLI tests plus the etaps ought 
>> to be the "official"  tests for vetting, and part of the build
> 
> Not that I disagree, but part (most?) of the appeal of the browser based 
> tests are that they run in a real-world client instead of the lab that is 
> couchjs+http :)
> 
> Cheers
> Jan
> --
> 
>> 
>> 
>>> 
>>> It is very useful when developing new code to not have to switch to and 
>>> reload the
>>> browser over and over again.
>>> 
>>> Cheers
>>> Jan
>>> --
>>> 
>>> 
>>> 
>>> 
 
 On 21 Mar 2010, at 17:05, Jan Lehnardt wrote:
 
> 
> On 21 Mar 2010, at 06:04, Robert Dionne wrote:
> 
>> On Mar 21, 2010, at 4:00 AM, Jan Lehnardt wrote:
>> 
>>> 
>>> On 20 Mar 2010, at 20:06, Paul Davis wrote:
>>> 
 On Sat, Mar 20, 2010 at 2:31 PM, Noah Slater  wrote:
> I think faulty test case should block the release, if I am to have any
> future sanity preparing releases. I don't want to delay and longer, 
> so if
> you guys are absolutely sure this is a test error and not code error, 
> then I
> propose that the test be commented out. Our tests form a contract 
> between
> us, internally, and our users. If that contract has a bug, it should 
> be
> removed or fixed - or it simply dilutes the importance of contract. 
> If some
> one comments out the test, and we agree it is not indicative of an 
> important
> bug, I can call the vote within hours.
> 
 
 I'd have to agree on this. From the point of view of a release, if a
 test reports a failure then it should be made to not report a failure.
 If that's accomplished by disabling it, then there will be a commit
 with a message that explains why it was disabled and etc and such on
 and so forth.
>>> 
>>> I'd do that if the test was failing for me :)
>> 
>> it's not failing for you when you run changes.js with the CLI ?  Fails 
>> for me every time. 
> 
> I don't consider the CLI tests as part of the official test suite just 
> yet.
> 
> Cheers
> Jan
> --
> 
>> 
>> Anyway I poked at this a bit yesterday and am not 100% sure the issue is 
>> in the test. I tried putting a sleep in with no luck. If my 
>> understanding of the JS is correct, CouchDB is supposed to be 
>> synchronous so it's not timing.
>> 
>> If someone could comment on the test itself it would be helpful. The 
>> section of the code that fails:
>> 
>> // changes get all_docs style with deleted docs
>> var doc = {a:1};
>> db.save(doc);
>> db.deleteDoc(doc);
>> var req = CouchDB.request("GET", 
>> "/test_suite_db/_changes?filter=changes_filter/bop&style=all_docs");
>> var resp = JSON.parse(req.responseText);
>> TEquals(3, resp.results.length, "should return matching rows");
>> 
>> 
>> seems odd to me. all_docs as I read the code will return docs with 
>> deletes and conflicts but in this call the filter bop will not apply to 
>> the doc {a:1} so I'm not sure what this delete prior to the call is 
>> about. Anyway I can make it fail in the debugger so perhaps I can find 
>> the root cause.
>> 
>> 
>> 
>> 
>>> 
>>> Cheers
>>> Jan
>>> --
>>> 
>> 
> 
 
>>> 
>> 
> 



Re: Test suite blocking release

2010-03-21 Thread Jan Lehnardt

On 21 Mar 2010, at 12:24, Robert Dionne wrote:

> 
> 
> 
> 
> On Mar 21, 2010, at 1:16 PM, Jan Lehnardt wrote:
> 
>> 
>> On 21 Mar 2010, at 12:10, Noah Slater wrote:
>> 
>>> What are the CLI tests, if not the etap tests? Are they integrated into the 
>>> build system?
>> 
>> The CLI tests are the same as the browser tests, just run through our 
>> couchjs binary
>> that has custom HTTP extensions to make the xhr work. At this point I don't 
>> think it
>> is reliable enough to mimic browser behaviour and that we shouldn't use it 
>> for vetting
>> the state of the code.
> 
> This is likely true, but in this particular case I think there's a bug in the 
> changes code (that I'm trying to dig out). It's nice that it works on your 
> machine but on my machine, using FF, it fails often enough. Moreover it's 
> been around for a long time now so I figure it's worth figuring out. 
> 
> I don't have a dog in this fight (.ie. a paying customer) so this failure 
> doesn't bother me. With respect to policy, given the various bogocities of 
> browsers, I'd recommend something like these CLI tests plus the etaps ought 
> to be the "official"  tests for vetting, and part of the build

Not that I disagree, but part (most?) of the appeal of the browser based tests 
are that they run in a real-world client instead of the lab that is 
couchjs+http :)

Cheers
Jan
--

> 
> 
>> 
>> It is very useful when developing new code to not have to switch to and 
>> reload the
>> browser over and over again.
>> 
>> Cheers
>> Jan
>> --
>> 
>> 
>> 
>> 
>>> 
>>> On 21 Mar 2010, at 17:05, Jan Lehnardt wrote:
>>> 
 
 On 21 Mar 2010, at 06:04, Robert Dionne wrote:
 
> On Mar 21, 2010, at 4:00 AM, Jan Lehnardt wrote:
> 
>> 
>> On 20 Mar 2010, at 20:06, Paul Davis wrote:
>> 
>>> On Sat, Mar 20, 2010 at 2:31 PM, Noah Slater  wrote:
 I think faulty test case should block the release, if I am to have any
 future sanity preparing releases. I don't want to delay and longer, so 
 if
 you guys are absolutely sure this is a test error and not code error, 
 then I
 propose that the test be commented out. Our tests form a contract 
 between
 us, internally, and our users. If that contract has a bug, it should be
 removed or fixed - or it simply dilutes the importance of contract. If 
 some
 one comments out the test, and we agree it is not indicative of an 
 important
 bug, I can call the vote within hours.
 
>>> 
>>> I'd have to agree on this. From the point of view of a release, if a
>>> test reports a failure then it should be made to not report a failure.
>>> If that's accomplished by disabling it, then there will be a commit
>>> with a message that explains why it was disabled and etc and such on
>>> and so forth.
>> 
>> I'd do that if the test was failing for me :)
> 
> it's not failing for you when you run changes.js with the CLI ?  Fails 
> for me every time. 
 
 I don't consider the CLI tests as part of the official test suite just yet.
 
 Cheers
 Jan
 --
 
> 
> Anyway I poked at this a bit yesterday and am not 100% sure the issue is 
> in the test. I tried putting a sleep in with no luck. If my understanding 
> of the JS is correct, CouchDB is supposed to be synchronous so it's not 
> timing.
> 
> If someone could comment on the test itself it would be helpful. The 
> section of the code that fails:
> 
> // changes get all_docs style with deleted docs
> var doc = {a:1};
> db.save(doc);
> db.deleteDoc(doc);
> var req = CouchDB.request("GET", 
> "/test_suite_db/_changes?filter=changes_filter/bop&style=all_docs");
> var resp = JSON.parse(req.responseText);
> TEquals(3, resp.results.length, "should return matching rows");
> 
> 
> seems odd to me. all_docs as I read the code will return docs with 
> deletes and conflicts but in this call the filter bop will not apply to 
> the doc {a:1} so I'm not sure what this delete prior to the call is 
> about. Anyway I can make it fail in the debugger so perhaps I can find 
> the root cause.
> 
> 
> 
> 
>> 
>> Cheers
>> Jan
>> --
>> 
> 
 
>>> 
>> 
> 



Re: Test suite blocking release

2010-03-21 Thread Robert Dionne




On Mar 21, 2010, at 1:16 PM, Jan Lehnardt wrote:

> 
> On 21 Mar 2010, at 12:10, Noah Slater wrote:
> 
>> What are the CLI tests, if not the etap tests? Are they integrated into the 
>> build system?
> 
> The CLI tests are the same as the browser tests, just run through our couchjs 
> binary
> that has custom HTTP extensions to make the xhr work. At this point I don't 
> think it
> is reliable enough to mimic browser behaviour and that we shouldn't use it 
> for vetting
> the state of the code.

This is likely true, but in this particular case I think there's a bug in the 
changes code (that I'm trying to dig out). It's nice that it works on your 
machine but on my machine, using FF, it fails often enough. Moreover it's been 
around for a long time now so I figure it's worth figuring out. 

I don't have a dog in this fight (.ie. a paying customer) so this failure 
doesn't bother me. With respect to policy, given the various bogocities of 
browsers, I'd recommend something like these CLI tests plus the etaps ought to 
be the "official"  tests for vetting, and part of the build


> 
> It is very useful when developing new code to not have to switch to and 
> reload the
> browser over and over again.
> 
> Cheers
> Jan
> --
> 
> 
> 
> 
>> 
>> On 21 Mar 2010, at 17:05, Jan Lehnardt wrote:
>> 
>>> 
>>> On 21 Mar 2010, at 06:04, Robert Dionne wrote:
>>> 
 On Mar 21, 2010, at 4:00 AM, Jan Lehnardt wrote:
 
> 
> On 20 Mar 2010, at 20:06, Paul Davis wrote:
> 
>> On Sat, Mar 20, 2010 at 2:31 PM, Noah Slater  wrote:
>>> I think faulty test case should block the release, if I am to have any
>>> future sanity preparing releases. I don't want to delay and longer, so 
>>> if
>>> you guys are absolutely sure this is a test error and not code error, 
>>> then I
>>> propose that the test be commented out. Our tests form a contract 
>>> between
>>> us, internally, and our users. If that contract has a bug, it should be
>>> removed or fixed - or it simply dilutes the importance of contract. If 
>>> some
>>> one comments out the test, and we agree it is not indicative of an 
>>> important
>>> bug, I can call the vote within hours.
>>> 
>> 
>> I'd have to agree on this. From the point of view of a release, if a
>> test reports a failure then it should be made to not report a failure.
>> If that's accomplished by disabling it, then there will be a commit
>> with a message that explains why it was disabled and etc and such on
>> and so forth.
> 
> I'd do that if the test was failing for me :)
 
 it's not failing for you when you run changes.js with the CLI ?  Fails for 
 me every time. 
>>> 
>>> I don't consider the CLI tests as part of the official test suite just yet.
>>> 
>>> Cheers
>>> Jan
>>> --
>>> 
 
 Anyway I poked at this a bit yesterday and am not 100% sure the issue is 
 in the test. I tried putting a sleep in with no luck. If my understanding 
 of the JS is correct, CouchDB is supposed to be synchronous so it's not 
 timing.
 
 If someone could comment on the test itself it would be helpful. The 
 section of the code that fails:
 
 // changes get all_docs style with deleted docs
 var doc = {a:1};
 db.save(doc);
 db.deleteDoc(doc);
 var req = CouchDB.request("GET", 
 "/test_suite_db/_changes?filter=changes_filter/bop&style=all_docs");
 var resp = JSON.parse(req.responseText);
 TEquals(3, resp.results.length, "should return matching rows");
 
 
 seems odd to me. all_docs as I read the code will return docs with deletes 
 and conflicts but in this call the filter bop will not apply to the doc 
 {a:1} so I'm not sure what this delete prior to the call is about. Anyway 
 I can make it fail in the debugger so perhaps I can find the root cause.
 
 
 
 
> 
> Cheers
> Jan
> --
> 
 
>>> 
>> 
> 



Re: Test suite blocking release

2010-03-21 Thread Jan Lehnardt

On 21 Mar 2010, at 12:10, Noah Slater wrote:


> Are they integrated into the build system?

No.

Cheers
Jan
--



Re: Test suite blocking release

2010-03-21 Thread Jan Lehnardt

On 21 Mar 2010, at 12:10, Noah Slater wrote:

> What are the CLI tests, if not the etap tests? Are they integrated into the 
> build system?

The CLI tests are the same as the browser tests, just run through our couchjs 
binary
that has custom HTTP extensions to make the xhr work. At this point I don't 
think it
is reliable enough to mimic browser behaviour and that we shouldn't use it for 
vetting
the state of the code.

It is very useful when developing new code to not have to switch to and reload 
the
browser over and over again.

Cheers
Jan
--




> 
> On 21 Mar 2010, at 17:05, Jan Lehnardt wrote:
> 
>> 
>> On 21 Mar 2010, at 06:04, Robert Dionne wrote:
>> 
>>> On Mar 21, 2010, at 4:00 AM, Jan Lehnardt wrote:
>>> 
 
 On 20 Mar 2010, at 20:06, Paul Davis wrote:
 
> On Sat, Mar 20, 2010 at 2:31 PM, Noah Slater  wrote:
>> I think faulty test case should block the release, if I am to have any
>> future sanity preparing releases. I don't want to delay and longer, so if
>> you guys are absolutely sure this is a test error and not code error, 
>> then I
>> propose that the test be commented out. Our tests form a contract between
>> us, internally, and our users. If that contract has a bug, it should be
>> removed or fixed - or it simply dilutes the importance of contract. If 
>> some
>> one comments out the test, and we agree it is not indicative of an 
>> important
>> bug, I can call the vote within hours.
>> 
> 
> I'd have to agree on this. From the point of view of a release, if a
> test reports a failure then it should be made to not report a failure.
> If that's accomplished by disabling it, then there will be a commit
> with a message that explains why it was disabled and etc and such on
> and so forth.
 
 I'd do that if the test was failing for me :)
>>> 
>>> it's not failing for you when you run changes.js with the CLI ?  Fails for 
>>> me every time. 
>> 
>> I don't consider the CLI tests as part of the official test suite just yet.
>> 
>> Cheers
>> Jan
>> --
>> 
>>> 
>>> Anyway I poked at this a bit yesterday and am not 100% sure the issue is in 
>>> the test. I tried putting a sleep in with no luck. If my understanding of 
>>> the JS is correct, CouchDB is supposed to be synchronous so it's not timing.
>>> 
>>> If someone could comment on the test itself it would be helpful. The 
>>> section of the code that fails:
>>> 
>>> // changes get all_docs style with deleted docs
>>> var doc = {a:1};
>>> db.save(doc);
>>> db.deleteDoc(doc);
>>> var req = CouchDB.request("GET", 
>>>  "/test_suite_db/_changes?filter=changes_filter/bop&style=all_docs");
>>> var resp = JSON.parse(req.responseText);
>>> TEquals(3, resp.results.length, "should return matching rows");
>>> 
>>> 
>>> seems odd to me. all_docs as I read the code will return docs with deletes 
>>> and conflicts but in this call the filter bop will not apply to the doc 
>>> {a:1} so I'm not sure what this delete prior to the call is about. Anyway I 
>>> can make it fail in the debugger so perhaps I can find the root cause.
>>> 
>>> 
>>> 
>>> 
 
 Cheers
 Jan
 --
 
>>> 
>> 
> 



Re: Test suite blocking release

2010-03-21 Thread Noah Slater
What are the CLI tests, if not the etap tests? Are they integrated into the 
build system?

On 21 Mar 2010, at 17:05, Jan Lehnardt wrote:

> 
> On 21 Mar 2010, at 06:04, Robert Dionne wrote:
> 
>> On Mar 21, 2010, at 4:00 AM, Jan Lehnardt wrote:
>> 
>>> 
>>> On 20 Mar 2010, at 20:06, Paul Davis wrote:
>>> 
 On Sat, Mar 20, 2010 at 2:31 PM, Noah Slater  wrote:
> I think faulty test case should block the release, if I am to have any
> future sanity preparing releases. I don't want to delay and longer, so if
> you guys are absolutely sure this is a test error and not code error, 
> then I
> propose that the test be commented out. Our tests form a contract between
> us, internally, and our users. If that contract has a bug, it should be
> removed or fixed - or it simply dilutes the importance of contract. If 
> some
> one comments out the test, and we agree it is not indicative of an 
> important
> bug, I can call the vote within hours.
> 
 
 I'd have to agree on this. From the point of view of a release, if a
 test reports a failure then it should be made to not report a failure.
 If that's accomplished by disabling it, then there will be a commit
 with a message that explains why it was disabled and etc and such on
 and so forth.
>>> 
>>> I'd do that if the test was failing for me :)
>> 
>> it's not failing for you when you run changes.js with the CLI ?  Fails for 
>> me every time. 
> 
> I don't consider the CLI tests as part of the official test suite just yet.
> 
> Cheers
> Jan
> --
> 
>> 
>> Anyway I poked at this a bit yesterday and am not 100% sure the issue is in 
>> the test. I tried putting a sleep in with no luck. If my understanding of 
>> the JS is correct, CouchDB is supposed to be synchronous so it's not timing.
>> 
>> If someone could comment on the test itself it would be helpful. The section 
>> of the code that fails:
>> 
>> // changes get all_docs style with deleted docs
>> var doc = {a:1};
>> db.save(doc);
>> db.deleteDoc(doc);
>> var req = CouchDB.request("GET", 
>>   "/test_suite_db/_changes?filter=changes_filter/bop&style=all_docs");
>> var resp = JSON.parse(req.responseText);
>> TEquals(3, resp.results.length, "should return matching rows");
>> 
>> 
>> seems odd to me. all_docs as I read the code will return docs with deletes 
>> and conflicts but in this call the filter bop will not apply to the doc 
>> {a:1} so I'm not sure what this delete prior to the call is about. Anyway I 
>> can make it fail in the debugger so perhaps I can find the root cause.
>> 
>> 
>> 
>> 
>>> 
>>> Cheers
>>> Jan
>>> --
>>> 
>> 
> 



Re: Test suite blocking release

2010-03-21 Thread Jan Lehnardt

On 21 Mar 2010, at 06:04, Robert Dionne wrote:

> On Mar 21, 2010, at 4:00 AM, Jan Lehnardt wrote:
> 
>> 
>> On 20 Mar 2010, at 20:06, Paul Davis wrote:
>> 
>>> On Sat, Mar 20, 2010 at 2:31 PM, Noah Slater  wrote:
 I think faulty test case should block the release, if I am to have any
 future sanity preparing releases. I don't want to delay and longer, so if
 you guys are absolutely sure this is a test error and not code error, then 
 I
 propose that the test be commented out. Our tests form a contract between
 us, internally, and our users. If that contract has a bug, it should be
 removed or fixed - or it simply dilutes the importance of contract. If some
 one comments out the test, and we agree it is not indicative of an 
 important
 bug, I can call the vote within hours.
 
>>> 
>>> I'd have to agree on this. From the point of view of a release, if a
>>> test reports a failure then it should be made to not report a failure.
>>> If that's accomplished by disabling it, then there will be a commit
>>> with a message that explains why it was disabled and etc and such on
>>> and so forth.
>> 
>> I'd do that if the test was failing for me :)
> 
> it's not failing for you when you run changes.js with the CLI ?  Fails for me 
> every time. 

I don't consider the CLI tests as part of the official test suite just yet.

Cheers
Jan
--

> 
> Anyway I poked at this a bit yesterday and am not 100% sure the issue is in 
> the test. I tried putting a sleep in with no luck. If my understanding of the 
> JS is correct, CouchDB is supposed to be synchronous so it's not timing.
> 
> If someone could comment on the test itself it would be helpful. The section 
> of the code that fails:
> 
> // changes get all_docs style with deleted docs
>  var doc = {a:1};
>  db.save(doc);
>  db.deleteDoc(doc);
>  var req = CouchDB.request("GET", 
>"/test_suite_db/_changes?filter=changes_filter/bop&style=all_docs");
>  var resp = JSON.parse(req.responseText);
>  TEquals(3, resp.results.length, "should return matching rows");
> 
> 
> seems odd to me. all_docs as I read the code will return docs with deletes 
> and conflicts but in this call the filter bop will not apply to the doc {a:1} 
> so I'm not sure what this delete prior to the call is about. Anyway I can 
> make it fail in the debugger so perhaps I can find the root cause.
> 
> 
> 
> 
>> 
>> Cheers
>> Jan
>> --
>> 
> 



[jira] Commented: (COUCHDB-679) Pull replication no longer works when a source doc has a large attachment

2010-03-21 Thread Filipe Manana (JIRA)

[ 
https://issues.apache.org/jira/browse/COUCHDB-679?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12847921#action_12847921
 ] 

Filipe Manana commented on COUCHDB-679:
---

@Jan news about the test?

> Pull replication no longer works when a source doc has a large attachment
> -
>
> Key: COUCHDB-679
> URL: https://issues.apache.org/jira/browse/COUCHDB-679
> Project: CouchDB
>  Issue Type: Bug
>  Components: Replication
> Environment: trunk and 0.11
>Reporter: Filipe Manana
>Priority: Blocker
> Fix For: 0.11
>
> Attachments: CouchDB-679-etap_test-trunk-2.patch, 
> CouchDB-679-etap_test-trunk-3.patch, CouchDB-679-etap_test-trunk.patch
>
>
> After rev r916868  (fix for CouchDB-597), doing a pull replication of a doc 
> that has a large attachment no longer works.
> The problem is in couch_rep_att.erl:
> convert_stub(#att{data=stub, name=Name} = Attachment,
>{#http_db{} = Db, Id, Rev}) ->
> {Pos, [RevId|_]} = Rev,
> Request = Db#http_db{
> resource = lists:flatten([couch_util:url_encode(Id), "/",
> couch_util:url_encode(Name)]),
> qs = [{rev, couch_doc:rev_to_str({Pos,RevId})}]
> },
> Ref = make_ref(),
> RcvFun = fun() ->
>  Bin = attachment_receiver(Ref, Request),
>  cleanup(),
>  Bin
>  end,
> Attachment#att{data=RcvFun}.
> The cleanup/0 function can not be called there, since when the attachment is 
> received in multiple chunks, after receiving the first chunk the subsequent 
> ones are silently discarded by cleanup/0:
> cleanup() ->
>   receive
>   {ibrowse_async_response, _, _} ->
>  %% TODO maybe log, didn't expect to have data here
>  cleanup();
>   {ibrowse_async_response_end, _} ->
>   cleanup();
>   {ibrowse_async_headers, _, _, _} ->
>   cleanup()
>   after 0 ->
>   erase(),
>   ok
>   end. 
> If you look into couch_db.erl, you'll see that the attachment receiver 
> function maybe called multiple times:
> flush_att(Fd, #att{data=Fun,att_len=AttLen}=Att) when is_function(Fun) ->
> with_stream(Fd, Att, fun(OutputStream) ->
> write_streamed_attachment(OutputStream, Fun, AttLen)
> end).
> write_streamed_attachment(_Stream, _F, 0) ->
> ok;
> write_streamed_attachment(Stream, F, LenLeft) ->
> Bin = F(),
> ok = couch_stream:write(Stream, Bin),
> write_streamed_attachment(Stream, F, LenLeft - size(Bin)).
> This is serious :(
> However, removing that call to cleanup/0 may cause the return of CouchDB-597, 
> therefore I don't supply a patch here.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.



Re: svn commit: r925497 - /couchdb/trunk/src/couchdb/couch_rep.erl

2010-03-21 Thread Filipe David Manana
Jan,

It would also be a good idea to do the same with the filtered replication
specific parameters ("filter" and "query_params"). The patch attached to
this email does it.

Also, don't forget the regression test attached to
https://issues.apache.org/jira/browse/COUCHDB-703

cheers

On Sat, Mar 20, 2010 at 12:22 AM,  wrote:

> Author: jan
> Date: Sat Mar 20 00:22:04 2010
> New Revision: 925497
>
> URL: http://svn.apache.org/viewvc?rev=925497&view=rev
> Log:
> backwards compatible ids for non-docid replications
>
> Modified:
>couchdb/trunk/src/couchdb/couch_rep.erl
>
> Modified: couchdb/trunk/src/couchdb/couch_rep.erl
> URL:
> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_rep.erl?rev=925497&r1=925496&r2=925497&view=diff
>
> ==
> --- couchdb/trunk/src/couchdb/couch_rep.erl (original)
> +++ couchdb/trunk/src/couchdb/couch_rep.erl Sat Mar 20 00:22:04 2010
> @@ -455,7 +455,12 @@ make_replication_id({Props}, UserCtx) ->
> QueryParams = proplists:get_value(<<"query_params">>, Props),
> DocIds = proplists:get_value(<<"doc_ids">>, Props),
> Base = couch_util:to_hex(erlang:md5(
> -term_to_binary([HostName, Src, Tgt, Filter, QueryParams, DocIds])
> +case DocIds of
> +undefined ->
> +term_to_binary([HostName, Src, Tgt, Filter, QueryParams]);
> +DocIds ->
> +term_to_binary([HostName, Src, Tgt, Filter, QueryParams,
> DocIds])
> +end
> )),
> Extension = maybe_append_options(
> [<<"continuous">>, <<"create_target">>], Props),
>
>
>


-- 
Filipe David Manana,
fdman...@gmail.com

"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."


Re: svn commit: r925497 - /couchdb/trunk/src/couchdb/couch_rep.erl

2010-03-21 Thread Filipe David Manana
Jan,

It would also be a good idea to do the same with the filtered replication
specific parameters ("filter" and "query_params"). The patch attached to
this email does it.

Also, don't forget the regression test attached to
https://issues.apache.org/jira/browse/COUCHDB-703

cheers

On Sat, Mar 20, 2010 at 12:22 AM,  wrote:

> Author: jan
> Date: Sat Mar 20 00:22:04 2010
> New Revision: 925497
>
> URL: http://svn.apache.org/viewvc?rev=925497&view=rev
> Log:
> backwards compatible ids for non-docid replications
>
> Modified:
>couchdb/trunk/src/couchdb/couch_rep.erl
>
> Modified: couchdb/trunk/src/couchdb/couch_rep.erl
> URL:
> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_rep.erl?rev=925497&r1=925496&r2=925497&view=diff
>
> ==
> --- couchdb/trunk/src/couchdb/couch_rep.erl (original)
> +++ couchdb/trunk/src/couchdb/couch_rep.erl Sat Mar 20 00:22:04 2010
> @@ -455,7 +455,12 @@ make_replication_id({Props}, UserCtx) ->
> QueryParams = proplists:get_value(<<"query_params">>, Props),
> DocIds = proplists:get_value(<<"doc_ids">>, Props),
> Base = couch_util:to_hex(erlang:md5(
> -term_to_binary([HostName, Src, Tgt, Filter, QueryParams, DocIds])
> +case DocIds of
> +undefined ->
> +term_to_binary([HostName, Src, Tgt, Filter, QueryParams]);
> +DocIds ->
> +term_to_binary([HostName, Src, Tgt, Filter, QueryParams,
> DocIds])
> +end
> )),
> Extension = maybe_append_options(
> [<<"continuous">>, <<"create_target">>], Props),
>
>
>


-- 
Filipe David Manana,
fdman...@gmail.com

"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."


Re: Test suite blocking release

2010-03-21 Thread Robert Dionne
On Mar 21, 2010, at 4:00 AM, Jan Lehnardt wrote:

> 
> On 20 Mar 2010, at 20:06, Paul Davis wrote:
> 
>> On Sat, Mar 20, 2010 at 2:31 PM, Noah Slater  wrote:
>>> I think faulty test case should block the release, if I am to have any
>>> future sanity preparing releases. I don't want to delay and longer, so if
>>> you guys are absolutely sure this is a test error and not code error, then I
>>> propose that the test be commented out. Our tests form a contract between
>>> us, internally, and our users. If that contract has a bug, it should be
>>> removed or fixed - or it simply dilutes the importance of contract. If some
>>> one comments out the test, and we agree it is not indicative of an important
>>> bug, I can call the vote within hours.
>>> 
>> 
>> I'd have to agree on this. From the point of view of a release, if a
>> test reports a failure then it should be made to not report a failure.
>> If that's accomplished by disabling it, then there will be a commit
>> with a message that explains why it was disabled and etc and such on
>> and so forth.
> 
> I'd do that if the test was failing for me :)

it's not failing for you when you run changes.js with the CLI ?  Fails for me 
every time. 

Anyway I poked at this a bit yesterday and am not 100% sure the issue is in the 
test. I tried putting a sleep in with no luck. If my understanding of the JS is 
correct, CouchDB is supposed to be synchronous so it's not timing.

If someone could comment on the test itself it would be helpful. The section of 
the code that fails:

// changes get all_docs style with deleted docs
  var doc = {a:1};
  db.save(doc);
  db.deleteDoc(doc);
  var req = CouchDB.request("GET", 
"/test_suite_db/_changes?filter=changes_filter/bop&style=all_docs");
  var resp = JSON.parse(req.responseText);
  TEquals(3, resp.results.length, "should return matching rows");


seems odd to me. all_docs as I read the code will return docs with deletes and 
conflicts but in this call the filter bop will not apply to the doc {a:1} so 
I'm not sure what this delete prior to the call is about. Anyway I can make it 
fail in the debugger so perhaps I can find the root cause.




> 
> Cheers
> Jan
> --
> 



Re: Test suite blocking release

2010-03-21 Thread Jan Lehnardt

On 20 Mar 2010, at 20:06, Paul Davis wrote:

> On Sat, Mar 20, 2010 at 2:31 PM, Noah Slater  wrote:
>> I think faulty test case should block the release, if I am to have any
>> future sanity preparing releases. I don't want to delay and longer, so if
>> you guys are absolutely sure this is a test error and not code error, then I
>> propose that the test be commented out. Our tests form a contract between
>> us, internally, and our users. If that contract has a bug, it should be
>> removed or fixed - or it simply dilutes the importance of contract. If some
>> one comments out the test, and we agree it is not indicative of an important
>> bug, I can call the vote within hours.
>> 
> 
> I'd have to agree on this. From the point of view of a release, if a
> test reports a failure then it should be made to not report a failure.
> If that's accomplished by disabling it, then there will be a commit
> with a message that explains why it was disabled and etc and such on
> and so forth.

I'd do that if the test was failing for me :)

Cheers
Jan
--