incubator-couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Anderson <jch...@apache.org>
Subject Re: couchjs refactoring
Date Mon, 02 Nov 2009 00:21:57 GMT
On Sun, Nov 1, 2009 at 3:49 PM, Paul Davis <paul.joseph.davis@gmail.com> wrote:
> Hey,
>
> I've finally completed the refactoring of the couchjs view server.
> I've put together a patch set consisting of three individual patches
> to show the progression of what's going on.
>
> Patch 1:
> http://github.com/davisp/couchdb/commit/3ab1154b77c75687cee55abed4ce936a41770b9b
>
>> Move all C code to src/couchdb/priv
>>
>> Shuffling bits around to conform to Erlang conventions.
>
> This first patch moves all C code into src/couchdb/priv/
> subdirectories to help make things more logical. This creates the
> directories couch_js, icu_driver, and spawnkillable that include the
> obviously related code. Part of this patch renames couch_erl_driver ->
> couch_icu_driver so that its more apparent what it actually is.
> Everything else is just the build changes required to support this
> move. There is zero behavior changed in this patch.
>
>
> Patch 2:
> http://github.com/davisp/couchdb/commit/cbf1efb383777ce8c64507fffdc044a85b0c69f2
>
>> Complete refactoring of couch_js.
>>
>> In particular, the cURL bindings have been rewritten to be more useful
>> and easily applied in command line scripts.
>
> This is the refactoring of couchjs. Highlights include splitting the
> code out of the single monolithic couch_js.c file and adding utf8.[hc]
> main.c and http.[hc] files to include the obviously related code. The
> new cURL bindings are included in this patch but are unused until the
> future patches. The only behavior changed in this patch is how HTTP
> connections are made from couchjs. As this only affects internal tests
> and the new benchmark tests this doesn't affect normal view server
> operations.
>
>
> Patch 3:
> http://github.com/davisp/couchdb/commit/22baacaf59e6422bc7edb3a3a2f9f9e1f19dbfc0
>
>> Integrate JavaScript tests into make check.
>>
>> All JS tests are now run along with the ETAP suite. In the future I
>> would like to improve on this support which would include patching the
>> test runner to provide etap output for individual test files. This would
>> probably require some major work to rewriting the test conditions in JS
>> tests though to make the output useful.
>
> This patch updates the build system and includes the necessary bits to
> add running the entire JS test suite from the command line. The output
> is formatted to adhere to TAP so that we can run the tests with prove
> as we do for the etap tests. As the commit message notes, in the
> future I'm thinking about making each call to T(condition, mesg)
> output a TAP message and then running each test individually. This
> will help us track things down if we have failures. As it is now, its
> one test, with asserts that each js file doesn't have errors.
>
> Another thing of note for this patch is that I had to make two minor
> edits to the test suite to support the command line runner. In
> changes.js I had to add an extra bit of logic to avoid running the
> asynchronous tests that aren't supported by the cURL bindings. The
> cookie_auth.js test modifies a check for a single HTTP status code
> assertion. The old behavior was to check for 200 after a redirect. The
> new assert allows 304 as a valid answer. This second change actually
> mirrors what's in the comment for the assert. My understanding is that
> the browser XHR must be following the redirect and returning the
> second response.
>
>
> Next on the chopping block is to move the bench tests into test/ and
> have them use the new cURL bindings. I didn't include them as part of
> these updates yet so people can focus on the big changes. Once I fix
> the bench stuff I plan on adding the --with-http flag to turn off cURL
> bindings by default. This should be a simple fix as including cURL
> support is a single function. The hard part is figuring out if I can
> reasonably assume all platforms we care about have some version of
> getopt or something.
>
> Anyway, I figured I'd make a formal note of this before committing. If
> anyone wants to read through and/or pull and run 'make check', 'make
> distcheck', and 'make distsign' to double check the builds that'd be
> super cool. If I don't hear any objections I'll push this in shortly.
>
> Paul Davis
>

Paul,

Thanks so much, this is really necessary and your attention to detail
and documentation is commendable.

I second the request to have people look through those patches, this
is the first attention the C program has received in a while, so
anyone looking at it would help.

Certainly no objections to you committing this, but if there are loose
ends like broken benches or whatever, please make loud noises after
the dust has settled too.

Chris

-- 
Chris Anderson
http://jchrisa.net
http://couch.io

Mime
View raw message