On 2010.02.10., at 19:38, Jarosław Pałka wrote:
> Attila,
>
> I applied third version of your patch. I have two comments:
> - I don't see where you execute interoperablejs tests, I see them in source
> code but it looks like they are not run as part of junit-all target Maybe I
> missed something :(
No, I think it's me who missed something - in testsrc/build.xml, just under:
<copy todir="${coverage.classes.dir}">
<fileset dir="src" excludes="**/*.java"/>
</copy>
we need to add this too:
<copy todir="${coverage.classes.dir}">
<fileset dir="testsrc" excludes="**/*.java"/>
</copy>
I have to admit that I run my unit tests from Eclipse IDE, so didn't run into
this problem...
Attila.
> - In CachingModuleScriptProviderBase, in line 70, you return null when module
> source is null, according to CommonJS spec we should throw error when module
> was not found
>
> It looks great after your last refactoring :)
>
> Regards,
> Jarek
>
> Attila Szegedi pisze:
>> Actually, I was informed on the commonjs list that there's a new set of
>> compliance tests at
>> <http://github.com/kriskowal/commonjs/tree/master/tests/modules/1.0/> and
>> the one on interoperablejs is obsolete.
>>
>> Also, just this morning i put out a new patch at
>> <https://bugzilla.mozilla.org/show_bug.cgi?id=540724> that already has all
>> the tests I wrote in it - this likely makes your work unnecessary; I'm sorry
>> I didn't wait on you, but I have scarce spare time to work on this, so when
>> I do have, I take advantage of it... By all means though, give it a review.
>> I did end up taking your approach and including the files, as the new set of
>> files wasn't available through command line SVN...
>>
>> Anyway, others might want to take the opportunity to check the current patch
>> - it's now tested with 60% coverage; the untested stuff is mostly to do with
>> cache revalidation. It passes all the compliance tests + my additional
>> implementation tests.
>>
>> Also, a big improvement compared to the previous patch is that I made
>> require() threadsafe, with good concurrent use properties, while not
>> sacrificing performance for single threaded use. So it can now be used to
>> load modules on demand into a shared top-level scope (which is something
>> many server-side JS embeddings do).
>>
>> Attila.
>>
>> --
>> home: http://www.szegedi.org
>> twitter: http://twitter.com/szegedi
>> weblog: http://constc.blogspot.com
>>
>> On 2010.02.10., at 12:31, Jarosław Pałka wrote:
>>
>
_______________________________________________
dev-tech-js-engine-rhino mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-js-engine-rhino
