Re: svn commit: r925497 - /couchdb/trunk/src/couchdb/couch_rep.erl
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
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
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
[ 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
>> 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
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
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
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
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
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
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
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
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
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
[ 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
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
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
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
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 --