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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
16 years, 8 months ago by felix8a
Modified:
15 years, 2 months ago
Reviewers:
MikeSamuel
CC:
google-caja-discuss_googlegroups.com
Base URL:
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
Unified diffs Side-by-side diffs Delta from patch set Stats (+216 lines, -104 lines) Patch
M build.xml View 1 1 chunk +29 lines, -8 lines 7 comments Download
A tests/com/google/caja/plugin/domita/aaTestAddEventListener.html View 1 chunk +45 lines, -0 lines 0 comments Download
A tests/com/google/caja/plugin/domita/testStrangeIds.html View 1 chunk +38 lines, -0 lines 0 comments Download
A tests/com/google/caja/plugin/domita/zzTestDocumentBodyAppendChild.html View 1 chunk +54 lines, -0 lines 0 comments Download
A tests/com/google/caja/plugin/domitaHead.html View 1 chunk +28 lines, -0 lines 0 comments Download
A tests/com/google/caja/plugin/domitaTail.html View 1 chunk +20 lines, -0 lines 0 comments Download
M tests/com/google/caja/plugin/domita_test.html View 1 chunk +1 line, -1 line 0 comments Download
M tests/com/google/caja/plugin/domita_test_untrusted.html View 8 chunks +1 line, -95 lines 2 comments Download

Messages

Total messages: 6
felix8a
16 years, 8 months ago (2009-10-09 01:22:01 UTC) #1
felix8a
updated snapshot. ant <fileset> is not ordered. adding <sort> to guarantee order.
16 years, 8 months ago (2009-10-09 02:03:33 UTC) #2
MikeSamuel
I like how you broke it up. Ihab, can you look at the note in ...
16 years, 7 months ago (2009-11-05 04:04:02 UTC) #3
zhangyong.net
On 2009/10/09 01:22:01, felix8a wrote: > o
16 years, 7 months ago (2009-11-05 09:16:58 UTC) #4
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!
16 years, 7 months ago (2009-11-05 09:20:00 UTC) #5
felix8a
16 years, 7 months ago (2009-11-05 22:15:48 UTC) #6
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 f62528b