Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(172)

Issue 129059: slice domita tests into separate files, phase 1

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 3 weeks ago by felix8a
Modified:
4 weeks, 1 day ago
Reviewers:
MikeSamuel
CC:
google-caja-discuss_googlegroups.com
SVN Base:
http://google-caja.googlecode.com/svn/trunk/
Visibility:
Public.

Description

the monolithic domita_test_untrusted.html file is
awkward in several ways.

this change allows individual domita tests to be
separate html files, which are then concatenated
together before cajoling.

phase 1 just sets up the framework and migrates
a few exemplar tests to individual files.

any in-flight changes to domita_test_untrusted.html
can still be committed, and they should still work.

if this framework seems reasonable and gets committed,
then new domita tests should be written as new files.

then I'll start phase 2, which will move all the old
tests into separate files and eventually delete
domita_test_untrusted.html

Patch Set 1

Patch Set 2 : slice domita tests into separate files, phase 1

Total comments: 9

Messages

Total messages: 6
felix8a
1 month, 3 weeks ago
felix8a
updated snapshot. ant <fileset> is not ordered. adding <sort> to guarantee order.
1 month, 3 weeks ago
MikeSamuel
I like how you broke it up. Ihab, can you look at the note in ...
1 month ago
zhangyong.net
On 2009/10/09 01:22:01, felix8a wrote: > o
4 weeks, 1 day ago
zhangyong.net
http://codereview.appspot.com/129059/diff/1009/18 File build.xml (right): http://codereview.appspot.com/129059/diff/1009/18#newcode438 Line 438: <output file="${lib.plugin}/domitaTests.co.html" language="cajita"/> oh!
4 weeks, 1 day ago
felix8a
4 weeks, 1 day ago
http://codereview.appspot.com/129059/diff/1009/18
File build.xml (right):

http://codereview.appspot.com/129059/diff/1009/18#newcode417
Line 417: <name
xmlns="antlib:org.apache.tools.ant.types.resources.comparators"/>
On 2009/11/05 04:04:02, MikeSamuel wrote:
> In which jar are these defined?

it's in ant.jar, I don't know why I'm forced to use xmlns, but it's documented
that way in the ant manual.

http://codereview.appspot.com/129059/diff/1009/18#newcode431
Line 431: </target>
On 2009/11/05 04:04:02, MikeSamuel wrote:
> So is this basically supposed to be a
>     cat domitaHead.html domita/a*.html domita_test_untrusted.html
> domita/{b,c,d,e...}*.html domitaTail.html

yeah

http://codereview.appspot.com/129059/diff/1009/18#newcode437
Line 437: <include file="${lib.plugin}/domitaTests.html"/>
On 2009/11/05 04:04:02, MikeSamuel wrote:
> Why not just have a bunch of includes?  We can always modify the include task
to
> deal with globs.

I started looking at doing that, but got lost in a maze of complexity in ant
that I didn't understand, so deferred that for something simple that works,
which can be replaced later.

http://codereview.appspot.com/129059/diff/1009/13
File tests/com/google/caja/plugin/domita_test_untrusted.html (right):

http://codereview.appspot.com/129059/diff/1009/13#newcode3056
Line 3056: </script>
On 2009/11/05 04:04:02, MikeSamuel wrote:
> Is the plan is to migrate tests out of here on an as-modified basis?

yeah.  I might also tackle migration in batches as I have time.  this is
basically a trial balloon to see if this refactoring is feasible and acceptable.

waiting now on discussion about potentially running tests as modules.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld r497