Bug 25880 – js tests should move into jstests subdirectory instead of resources/

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Eric Seidel (no email)]

js tests should move into jstests subdirectory instead of resources/

This would get rid of make-js-test-wrappers exclude list.

It would also make it possible to add a "fallback" TEMPLATE.html which could be used, since TEMPLATE.html no longer serves as an indicator for "crawl this directory and make .js files into tests".

[Eric Seidel (no email)]

A "fallback" template would mean that to add a js test you no longer have to copy TEMPLATE.html into resources and fix it up, instead you just add a jstests subdirectory and add your test there.  :)

[Darin Adler]

All of this sounds good to me except for the name "jstests". I think we can do better.

[Shinichiro Hamaji]

I'd like to help this! Before I actually start, I'd like to check if I understand the requirements correctly.

I think the first step of this bug is moving all resources/*.js files which aren't excluded by the list in make-js-test-wrappers into somewhere, fix all layout tests so that they reference the new location, and make-js-test-wrappers look the destination directory. Am I understanding correctly?

Also, I'd like to know the better name for them. Maybe "js" or "scripts" ?

[Darin Adler]

(In reply to comment #3)
> I think the first step of this bug is moving all resources/*.js files which
> aren't excluded by the list in make-js-test-wrappers into somewhere, fix all
> layout tests so that they reference the new location, and make-js-test-wrappers
> look the destination directory. Am I understanding correctly?
> 
> Also, I'd like to know the better name for them. Maybe "js" or "scripts" ?

I think "scripts" is not a good name, because the whole point is that these are "script tests", not just scripts. Maybe "jstests" is OK, or "js-tests" or "script-tests".

The way I'd do it is:

    1) Change make-js-test-wrappers to work for tests in directories either named "resources" or the new name.

    2) Move one test with "svn mv". Run make-js-test-wrappers. Check in the new top level .html file and the test in the new location.

    3) If everything went well after (2), then repeat (2) for all other tests. As you say above, all the resources/*.js files that are not in the excluded list.

    4) Remove the support for directories named "resources" in make-js-test-wrappers and remove the exception list as well.

[Eric Seidel (no email)]

Obviously step 2 can be scripted once it's tested. :)

[Eric Seidel (no email)]

I vote for "script-tests" as the name.

[Eric Seidel (no email)]

(Although eventually we'll want to rename make-js-test-wrappers if we use "script-tests".)  Thanks again for looking at this!

[Darin Adler]

(In reply to comment #7)
> (Although eventually we'll want to rename make-js-test-wrappers if we use
> "script-tests".)  Thanks again for looking at this!

I think we could rename make-js-test-wrappers to make-script-tests-wrappers right away as step (0).

I also think that eventually we could teach run-webkit-tests to run these tests without checking in wrappers. But that would have a downside since it would be harder to open one of these tests directly from a browser.

Further, I think that in the future we might want to run some of these tests as pure JavaScript tests, using the "jsc" command line tool instead of "DumpRenderTree" -- some can't work that way but others can't, and I wonder how we could distinguish those.

Finally, I'd love to convert the Mozilla tests for JavaScript in the JavaScriptCore tree into these style tests so they all run with run-webkit-tests.

Not all relevant to this task, but since you asked I thought I'd talk a bit about more of the road map in my mind.

[Darin Adler]

(In reply to comment #8)
> (In reply to comment #7)
> > (Although eventually we'll want to rename make-js-test-wrappers if we use
> > "script-tests".)  Thanks again for looking at this!
> 
> I think we could rename make-js-test-wrappers to make-script-tests-wrappers
> right away as step (0).

make-script-test-wrappers -- an "s" crept in there

[Shinichiro Hamaji]

Filed Bug 28212 as the step 0.

[Shinichiro Hamaji]

Thanks for the detailed comments on this task and future road maps! The steps Darin suggest sounds definitely better than my "change at once" plan. I'd like to follow the steps. The future road maps also sound nice. I'll be happy if I can help any of them.

By the way, I've noticed that running make-js-test-wrappers change 4 HTMLs which are slightly modified from the original TEMPLATE. I think we need to fix this before the step 1. I'll send a patch soon later. I hope my change makes sense.

[Eric Seidel (no email)]

Yes, it's common the .js files get added to resources/ and not ignored in make-js-test-wrappers.  That's why we're doing this!  To make that problem go away.  Once we have a separate directory for the script-tests then we won't need this silly ignore list.

So yes, please feel encouraged to add those 4 files to the ignore list.  You have my rubber-stamp already and do no need to post such a change for review.

I might also add that further in my personal roadmap would be moving all of the js-test-pre.js stuff out of fast/js/resources and into a LayoutTests/script-tests-harness directory, or some other nicely-named top-level directory where we can put more of this testing framework in an easily accessible place.

I would also like to add more support to make-js-test-wrappers, including some sort of template system.  So that either via a comment in the script-test/my-test.js file or via a an otherwise-ignored template("name.html") call, that we can have custom templates on a per-file basis.

[Shinichiro Hamaji]

Created attachment 38599 [details]
Patch v1

[Shinichiro Hamaji]

Now I've got committer bit! Good time to restart this work as now I can svn mv by myself.

[Eric Seidel (no email)]

Comment on attachment 38599 [details]
Patch v1

Looks fine.  I look forward to seeing the script to do the big move! :)

The commit-queue should handle the moves just fine, but since Shinichiro is now a committer, maybe he wants to commit this himself.  he can always set cq+ if he'd prefer the bot to commit it for him.

[Shinichiro Hamaji]

Comment on attachment 38599 [details]
Patch v1

Thanks for the review, Eric. OK, I'd try commit-queue+. It's a bit tricky for me to access my machine as I'm out of office now.

[Eric Seidel (no email)]

Comment on attachment 38599 [details]
Patch v1

It seems you haven't yet used your new-found committer powers to add yourself to
WebKitTools/Scripts/modules/committers.py yet.  You'll need to do that before the commit-queue accepts your patches.

Exception: Unknown committer: [e-mail]

[Eric Seidel (no email)]

Comment on attachment 38599 [details]
Patch v1

Clearing flags on attachment: 38599

Committed r47956: <http://trac.webkit.org/changeset/47956>

[Eric Seidel (no email)]

All reviewed patches have been landed.  Closing bug.

[Shinichiro Hamaji]

Need more patch to complete this work.

[Shinichiro Hamaji]

Created attachment 39329 [details]
A huge migration done by script

[Shinichiro Hamaji]

Created attachment 39331 [details]
script for the above patch

The above patch is created by this script. I'm not sure if this script should be committed. This script is only for this one attempt.

This script invokes make-script-test-wrappers script, knows which html is generated using standard output, and moves the modified files into the new directory.

I made make-script-wrappers ignore the following files before I ran this script. They are newly modified/added and they don't agree with their TEMPLATE.

getOwnPropertyDescriptor.js
Geolocation/resources/*.js
resources/message-port-multi.js

I'd like to fix them after this migration. As Geolocation requires gtk port, it would take some time.

As for the huge patch, I think it makes few sense to have 1800 lines of ChangeLog. Is it OK to remove them? If OK, should I write instead of these lines?

[Eric Seidel (no email)]

Comment on attachment 39329 [details]
A huge migration done by script

Please announce this change for 24 hours on webkit-dev before committing.

Rubber stamp = me.

[Shinichiro Hamaji]

Created attachment 39817 [details]
last migration patch I couldn't commit

Though I committed patches for the most of layout tests directories, I couldn't commit this patch which moves the tests in LayoutTests/fast/js.

[Shinichiro Hamaji]

Comment on attachment 39817 [details]
last migration patch I couldn't commit

Still I cannot commit this... Asking review+ as I'd like to try commit-queue+ to commit this patch.

[Eric Seidel (no email)]

Comment on attachment 39817 [details]
last migration patch I couldn't commit

The ChangeLog diff might confuse svn-apply, so I'd rather not send this exact patch though the queue.

[Shinichiro Hamaji]

Created attachment 39904 [details]
last migration patch changelog fixed

Oops. Fixed the ChangeLog diff.

[Eric Seidel (no email)]

Comment on attachment 39904 [details]
last migration patch changelog fixed

OK.  Per your request: I'm happy to try the commit-queue.  I don't know why it would have more luck than you did, but we'll see.

[WebKit Commit Bot]

Comment on attachment 39904 [details]
last migration patch changelog fixed

Rejecting patch 39904 from commit-queue.

Failed to run "['git', 'svn', 'dcommit']" exit_code: 1
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	R	LayoutTests/fast/js/kde/resources/Array.js => LayoutTests/fast/js/kde/script-tests/Array.js
	R	LayoutTests/fast/js/kde/resources/Boolean.js => LayoutTests/fast/js/kde/script-tests/Boolean.js
	R	LayoutTests/fast/js/kde/resources/TEMPLATE-n.html => LayoutTests/fast/js/kde/script-tests/TEMPLATE-n.html
	R	LayoutTests/fast/js/kde/resources/TEMPLATE.html => LayoutTests/fast/js/kde/script-tests/TEMPLATE.html
	R	LayoutTests/fast/js/kde/resources/arguments-scope.js => LayoutTests/fast/js/kde/script-tests/arguments-scope.js
	R	LayoutTests/fast/js/kde/resources/assignments.js => LayoutTests/fast/js/kde/script-tests/assignments.js
	R	LayoutTests/fast/js/kde/resources/cast.js => LayoutTests/fast/js/kde/script-tests/cast.js
	R	LayoutTests/fast/js/kde/resources/comment-1.js => LayoutTests/fast/js/kde/script-tests/comment-1.js
	R	LayoutTests/fast/js/kde/resources/comment-2.js => LayoutTests/fast/js/kde/script-tests/comment-2.js
	R	LayoutTests/fast/js/kde/resources/completion.js => LayoutTests/fast/js/kde/script-tests/completion.js
	R	LayoutTests/fast/js/kde/resources/conditional.js => LayoutTests/fast/js/kde/script-tests/conditional.js
	R	LayoutTests/fast/js/kde/resources/constructor_length.js => LayoutTests/fast/js/kde/script-tests/constructor_length.js
	R	LayoutTests/fast/js/kde/resources/crash-1.js => LayoutTests/fast/js/kde/script-tests/crash-1.js
	R	LayoutTests/fast/js/kde/resources/crash-2.js => LayoutTests/fast/js/kde/script-tests/crash-2.js
	R	LayoutTests/fast/js/kde/resources/delete.js => LayoutTests/fast/js/kde/script-tests/delete.js
	R	LayoutTests/fast/js/resources/Array-isArray.js => LayoutTests/fast/js/script-tests/Array-isArray.js
	R	LayoutTests/fast/js/resources/TEMPLATE.html => LayoutTests/fast/js/script-tests/TEMPLATE.html
	R	LayoutTests/fast/js/resources/activation-proto.js => LayoutTests/fast/js/script-tests/activation-proto.js
	R	LayoutTests/fast/js/resources/arguments-bad-index.js => LayoutTests/fast/js/script-tests/arguments-bad-index.js
	R	LayoutTests/fast/js/resources/arguments.js => LayoutTests/fast/js/script-tests/arguments.js
	R	LayoutTests/fast/js/resources/array-enumerators-functions.js => LayoutTests/fast/js/script-tests/array-enumerators-functions.js
	R	LayoutTests/fast/js/resources/array-every.js => LayoutTests/fast/js/script-tests/array-every.js
	R	LayoutTests/fast/js/resources/array-filter.js => LayoutTests/fast/js/script-tests/array-filter.js
	R	LayoutTests/fast/js/resources/array-float-delete.js => LayoutTests/fast/js/script-tests/array-float-delete.js
	R	LayoutTests/fast/js/resources/array-functions-non-arrays.js => LayoutTests/fast/js/script-tests/array-functions-non-arrays.js
	R	LayoutTests/fast/js/resources/array-holes.js => LayoutTests/fast/js/script-tests/array-holes.js
	R	LayoutTests/fast/js/resources/array-index-immediate-types.js => LayoutTests/fast/js/script-tests/array-index-immediate-types.js
	R	LayoutTests/fast/js/resources/array-indexing.js => LayoutTests/fast/js/script-tests/array-indexing.js
	R	LayoutTests/fast/js/resources/array-iterate-backwards.js => LayoutTests/fast/js/script-tests/array-iterate-backwards.js
	R	LayoutTests/fast/js/resources/array-join-bug-11524.js => LayoutTests/fast/js/script-tests/array-join-bug-11524.js
	R	LayoutTests/fast/js/resources/array-lastIndexOf.js => LayoutTests/fast/js/script-tests/array-lastIndexOf.js
	R	LayoutTests/fast/js/resources/array-reduce.js => LayoutTests/fast/js/script-tests/array-reduce.js
	R	LayoutTests/fast/js/resources/array-reduceRight.js => LayoutTests/fast/js/script-tests/array-reduceRight.js
	R	LayoutTests/fast/js/resources/array-reset-large-index.js => LayoutTests/fast/js/script-tests/array-reset-large-index.js
	R	LayoutTests/fast/js/resources/array-sort-reentrance.js => LayoutTests/fast/js/script-tests/array-sort-reentrance.js
	R	LayoutTests/fast/js/resources/array-splice.js => LayoutTests/fast/js/script-tests/array-splice.js
	R	LayoutTests/fast/js/resources/array-tostring-and-join.js => LayoutTests/fast/js/script-tests/array-tostring-and-join.js
	R	LayoutTests/fast/js/resources/array-tostring-ignore-separator.js => LayoutTests/fast/js/script-tests/array-tostring-ignore-separator.js
	R	LayoutTests/fast/js/resources/assign.js => LayoutTests/fast/js/script-tests/assign.js
	R	LayoutTests/fast/js/resources/avl-crash.js => LayoutTests/fast/js/script-tests/avl-crash.js
	R	LayoutTests/fast/js/resources/cached-eval-gc.js => LayoutTests/fast/js/script-tests/cached-eval-gc.js
	R	LayoutTests/fast/js/resources/caller-property.js => LayoutTests/fast/js/script-tests/caller-property.js
	R	LayoutTests/fast/js/resources/char-at.js => LayoutTests/fast/js/script-tests/char-at.js
	R	LayoutTests/fast/js/resources/closure-inside-extra-arg-call.js => LayoutTests/fast/js/script-tests/closure-inside-extra-arg-call.js
	R	LayoutTests/fast/js/resources/code-serialize-paren.js => LayoutTests/fast/js/script-tests/code-serialize-paren.js
	R	LayoutTests/fast/js/resources/codegen-loops-logical-nodes.js => LayoutTests/fast/js/script-tests/codegen-loops-logical-nodes.js
	R	LayoutTests/fast/js/resources/codegen-peephole-locals.js => LayoutTests/fast/js/script-tests/codegen-peephole-locals.js
	R	LayoutTests/fast/js/resources/codegen-temporaries.js => LayoutTests/fast/js/script-tests/codegen-temporaries.js
	R	LayoutTests/fast/js/resources/comparefn-sort-stability.js => LayoutTests/fast/js/script-tests/comparefn-sort-stability.js
	R	LayoutTests/fast/js/resources/const-without-initializer.js => LayoutTests/fast/js/script-tests/const-without-initializer.js
	R	LayoutTests/fast/js/resources/constant-count.js => LayoutTests/fast/js/script-tests/constant-count.js
	R	LayoutTests/fast/js/resources/constant-folding.js => LayoutTests/fast/js/script-tests/constant-folding.js
	R	LayoutTests/fast/js/resources/constructor-attributes.js => LayoutTests/fast/js/script-tests/constructor-attributes.js
	R	LayoutTests/fast/js/resources/constructor.js => LayoutTests/fast/js/script-tests/constructor.js
	R	LayoutTests/fast/js/resources/continue-break-multiple-labels.js => LayoutTests/fast/js/script-tests/continue-break-multiple-labels.js
	R	LayoutTests/fast/js/resources/convert-nan-to-bool.js => LayoutTests/fast/js/script-tests/convert-nan-to-bool.js
	R	LayoutTests/fast/js/resources/cyclic-proto.js => LayoutTests/fast/js/script-tests/cyclic-proto.js
	R	LayoutTests/fast/js/resources/cyclic-prototypes.js => LayoutTests/fast/js/script-tests/cyclic-prototypes.js
	R	LayoutTests/fast/js/resources/cyclic-ref-toString.js => LayoutTests/fast/js/script-tests/cyclic-ref-toString.js
	R	LayoutTests/fast/js/resources/date-DST-pre-1970.js => LayoutTests/fast/js/script-tests/date-DST-pre-1970.js
	R	LayoutTests/fast/js/resources/date-DST-time-cusps.js => LayoutTests/fast/js/script-tests/date-DST-time-cusps.js
	R	LayoutTests/fast/js/resources/date-big-constructor.js => LayoutTests/fast/js/script-tests/date-big-constructor.js
	R	LayoutTests/fast/js/resources/date-big-setdate.js => LayoutTests/fast/js/script-tests/date-big-setdate.js
	R	LayoutTests/fast/js/resources/date-big-setmonth.js => LayoutTests/fast/js/script-tests/date-big-setmonth.js
	R	LayoutTests/fast/js/resources/date-constructor.js => LayoutTests/fast/js/script-tests/date-constructor.js
	R	LayoutTests/fast/js/resources/date-negative-setmonth.js => LayoutTests/fast/js/script-tests/date-negative-setmonth.js
	R	LayoutTests/fast/js/resources/date-parse-comments-test.js => LayoutTests/fast/js/script-tests/date-parse-comments-test.js
	R	LayoutTests/fast/js/resources/date-parse-test.js => LayoutTests/fast/js/script-tests/date-parse-test.js
	R	LayoutTests/fast/js/resources/date-preserve-milliseconds.js => LayoutTests/fast/js/script-tests/date-preserve-milliseconds.js
	R	LayoutTests/fast/js/resources/date-proto-generic-invocation.js => LayoutTests/fast/js/script-tests/date-proto-generic-invocation.js
	R	LayoutTests/fast/js/resources/date-set-to-nan.js => LayoutTests/fast/js/script-tests/date-set-to-nan.js
	R	LayoutTests/fast/js/resources/date-toisostring.js => LayoutTests/fast/js/script-tests/date-toisostring.js
	R	LayoutTests/fast/js/resources/debugger.js => LayoutTests/fast/js/script-tests/debugger.js
	R	LayoutTests/fast/js/resources/declaration-in-block.js => LayoutTests/fast/js/script-tests/declaration-in-block.js
	R	LayoutTests/fast/js/resources/delete-getters-setters.js => LayoutTests/fast/js/script-tests/delete-getters-setters.js
	R	LayoutTests/fast/js/resources/delete-syntax.js => LayoutTests/fast/js/script-tests/delete-syntax.js
	R	LayoutTests/fast/js/resources/delete-then-put.js => LayoutTests/fast/js/script-tests/delete-then-put.js
	R	LayoutTests/fast/js/resources/dictionary-no-cache.js => LayoutTests/fast/js/script-tests/dictionary-no-cache.js
	R	LayoutTests/fast/js/resources/do-while-semicolon.js => LayoutTests/fast/js/script-tests/do-while-semicolon.js
	R	LayoutTests/fast/js/resources/dot-node-base-exception.js => LayoutTests/fast/js/script-tests/dot-node-base-exception.js
	R	LayoutTests/fast/js/resources/duplicate-param-crash.js => LayoutTests/fast/js/script-tests/duplicate-param-crash.js
	R	LayoutTests/fast/js/resources/duplicate-param-gc-crash.js => LayoutTests/fast/js/script-tests/duplicate-param-gc-crash.js
	M	LayoutTests/ChangeLog
	M	LayoutTests/fast/js/Array-isArray.html
	M	LayoutTests/fast/js/activation-proto.html
	M	LayoutTests/fast/js/arguments-bad-index.html
	M	LayoutTests/fast/js/arguments.html
	M	LayoutTests/fast/js/array-enumerators-functions.html
	M	LayoutTests/fast/js/array-every.html
	M	LayoutTests/fast/js/array-filter.html
	M	LayoutTests/fast/js/array-float-delete.html
	M	LayoutTests/fast/js/array-functions-non-arrays.html
	M	LayoutTests/fast/js/array-holes.html
	M	LayoutTests/fast/js/array-index-immediate-types.html
	M	LayoutTests/fast/js/array-indexing.html
	M	LayoutTests/fast/js/array-iterate-backwards.html
	M	LayoutTests/fast/js/array-join-bug-11524.html
	M	LayoutTests/fast/js/array-lastIndexOf.html
	M	LayoutTests/fast/js/array-reduce.html
	M	LayoutTests/fast/js/array-reduceRight.html
	M	LayoutTests/fast/js/array-reset-large-index.html
	M	LayoutTests/fast/js/array-sort-reentrance.html
	M	LayoutTests/fast/js/array-splice.html
	M	LayoutTests/fast/js/array-tostring-and-join.html
	M	LayoutTests/fast/js/array-tostring-ignore-separator.html
	M	LayoutTests/fast/js/assign.html
	M	LayoutTests/fast/js/avl-crash.html
	M	LayoutTests/fast/js/cached-eval-gc.html
	M	LayoutTests/fast/js/caller-property.html
	M	LayoutTests/fast/js/char-at.html
	M	LayoutTests/fast/js/closure-inside-extra-arg-call.html
	M	LayoutTests/fast/js/code-serialize-paren.html
	M	LayoutTests/fast/js/codegen-loops-logical-nodes.html
	M	LayoutTests/fast/js/codegen-peephole-locals.html
	M	LayoutTests/fast/js/codegen-temporaries.html
	M	LayoutTests/fast/js/comparefn-sort-stability.html
	M	LayoutTests/fast/js/const-without-initializer.html
	M	LayoutTests/fast/js/constant-count.html
	M	LayoutTests/fast/js/constant-folding.html
	M	LayoutTests/fast/js/constructor-attributes.html
	M	LayoutTests/fast/js/constructor.html
	M	LayoutTests/fast/js/continue-break-multiple-labels.html
	M	LayoutTests/fast/js/convert-nan-to-bool.html
	M	LayoutTests/fast/js/cyclic-proto.html
	M	LayoutTests/fast/js/cyclic-prototypes.html
	M	LayoutTests/fast/js/cyclic-ref-toString.html
	M	LayoutTests/fast/js/date-DST-pre-1970.html
	M	LayoutTests/fast/js/date-DST-time-cusps.html
	M	LayoutTests/fast/js/date-big-constructor.html
	M	LayoutTests/fast/js/date-big-setdate.html
	M	LayoutTests/fast/js/date-big-setmonth.html
	M	LayoutTests/fast/js/date-constructor.html
	M	LayoutTests/fast/js/date-negative-setmonth.html
	M	LayoutTests/fast/js/date-parse-comments-test.html
	M	LayoutTests/fast/js/date-parse-test.html
	M	LayoutTests/fast/js/date-preserve-milliseconds.html
	M	LayoutTests/fast/js/date-proto-generic-invocation.html
	M	LayoutTests/fast/js/date-set-to-nan.html
	M	LayoutTests/fast/js/date-toisostring.html
	M	LayoutTests/fast/js/debugger.html
	M	LayoutTests/fast/js/declaration-in-block.html
	M	LayoutTests/fast/js/delete-getters-setters.html
	M	LayoutTests/fast/js/delete-syntax.html
	M	LayoutTests/fast/js/delete-then-put.html
	M	LayoutTests/fast/js/dictionary-no-cache.html
	M	LayoutTests/fast/js/do-while-semicolon.html
	M	LayoutTests/fast/js/dot-node-base-exception.html
	M	LayoutTests/fast/js/duplicate-param-crash.html
	M	LayoutTests/fast/js/duplicate-param-gc-crash.html
	M	LayoutTests/fast/js/kde/Array.html
	M	LayoutTests/fast/js/kde/Boolean.html
	M	LayoutTests/fast/js/kde/arguments-scope.html
	M	LayoutTests/fast/js/kde/assignments.html
	M	LayoutTests/fast/js/kde/cast.html
	M	LayoutTests/fast/js/kde/comment-1.html
	M	LayoutTests/fast/js/kde/comment-2.html
	M	LayoutTests/fast/js/kde/completion.html
	M	LayoutTests/fast/js/kde/conditional.html
	M	LayoutTests/fast/js/kde/constructor_length.html
	M	LayoutTests/fast/js/kde/crash-1.html
	M	LayoutTests/fast/js/kde/crash-2.html
	M	LayoutTests/fast/js/kde/delete.html
RA layer request failed: MERGE request failed on '/repository/webkit/trunk': MERGE of '/repository/webkit/trunk': 409 Conflict (http://svn.webkit.org) at /usr/local/libexec/git-core//git-svn line 469

[Shinichiro Hamaji]

Thanks Eric for cq+. It seems I'm unlucky...

(In reply to comment #29)
> (From update of attachment 39904 [details])
> Rejecting patch 39904 from commit-queue.

This error was the same as the error as I saw. Maybe I need to contact someone who can check the SVN server. Eric, could you tell me who is the best person for this issue?

[Eric Seidel (no email)]

Wow, that's a lot of spew.   I guess I should have known that would be the output if the commit failed.

I generally contact Mark Rowe or William Siegrist for SVN issues.

[Shinichiro Hamaji]

Ah, sorry for the spam.

Thanks for telling the contact persons. I'll ask them.

[Eric Seidel (no email)]

Not your fault.  I should set an size output limit on the commit-queue comments.  I'll file a bug about that.  Bug 29628.

[William Siegrist]

Can you do this commit manually with the standard subversion command line client and see what it says? This is probably just a normal conflict that you need to resolve.

[Shinichiro Hamaji]

(In reply to comment #34)
> Can you do this commit manually with the standard subversion command line
> client and see what it says? This is probably just a normal conflict that you
> need to resolve.

Here is the outputs of svn up => svn ci => svn up. Note that no one committed patches while I'm trying to commit this patch. So, I'm guessing this isn't a normal conflict.

http://shinh.skr.jp/t/log.txt

[Eric Seidel (no email)]

For completeness sake, what version of SVN are you using to commit with?

svn --version will tell you.

[Shinichiro Hamaji]

(In reply to comment #36)
> For completeness sake, what version of SVN are you using to commit with?
> 
> svn --version will tell you.

% svn --version
svn, version 1.4.4 (r25188)
   compiled May 31 2008, 03:45:57

Actually, I've never tried to change the version of SVN client. It sounds worth trying.

[Shinichiro Hamaji]

Well... With SVN 1.6.5, it provided much better error messages. It seems we need to remove tab characters from some files. I've created another bug for this: https://bugs.webkit.org/show_bug.cgi?id=29630

Thanks Eric for the suggestion!

The error messages follow:

Transmitting file data .........................................................................................................................................................................................................................................................................................svn: Commit failed (details follow):
svn: Commit blocked by pre-commit hook (exit code 1) with output:
svnlook: Path 'trunk/LayoutTests/fast/js/kde/script-tests' is not a file
svnlook: Path 'trunk/LayoutTests/fast/js/script-tests' is not a file

    The following files contain tab characters:

        trunk/LayoutTests/fast/js/kde/script-tests/RegExp.js
        trunk/LayoutTests/fast/js/kde/script-tests/cast.js
        trunk/LayoutTests/fast/js/kde/script-tests/exceptions.js
        trunk/LayoutTests/fast/js/kde/script-tests/md5-1.js
        trunk/LayoutTests/fast/js/kde/script-tests/statements.js
        trunk/LayoutTests/fast/js/script-tests/regexp-extended-characters-crash.js

    Please use spaces instead to indent.
    If you must commit a file with tabs, use svn propset to set the "allow-tabs" property.

[Eric Seidel (no email)]

Clearly the commit-queue needs an upgraded version of svn too.  I'll do that this afternoon.

I think those files had allow-tabs:1 property set on them before and that must have been lost in the svn mv that your script did.

[Eric Seidel (no email)]

We can also easily just replace the tabs in those few files with 4 spaces.

[Shinichiro Hamaji]

Created attachment 39959 [details]
Patch v1

[Eric Seidel (no email)]

Comment on attachment 39959 [details]
Patch v1

Yay!  Have you validated to make sure there are no remaining TEMPLATE.html files in resources/ ?  You should do that before committing this.  It would also be good to write a (short) announcement note about this change on webkit-dev.

Thanks!

[Shinichiro Hamaji]

I could commit the patch with propset. Thanks Eric for help!

(In reply to comment #41)
> Created an attachment (id=39959) [details]
> Patch v1

This patch removes support of resource directories. After this change, it
produces warning for fast/background/repeat and fast/dom/Geolocation because
they still have resources/TEMPLATE.html .

The former should be fixed by

https://bugs.webkit.org/show_bug.cgi?id=29659

For the latter, I asked a Geolocation person to re-generate files because I
couldn't set up WebKit gtk.

After this patch is committed, I'll send an email to webkit-dev to remind
people about script-tests again.

[Shinichiro Hamaji]

(In reply to comment #42)
> (From update of attachment 39959 [details])
> Yay!  Have you validated to make sure there are no remaining TEMPLATE.html
> files in resources/ ?  You should do that before committing this.  It would
> also be good to write a (short) announcement note about this change on
> webkit-dev.

Well, as I've just written, there are still 2 resources/TEMPLATE.html . One of them seems to be marked as r+ by you (thanks!). I'd like to commit this patch now (before we fix the another TEMPLATE in Geolocation directory). Otherwise, people may continue to (mis)use resources directories. What do you think?

[Eric Seidel (no email)]

Sounds OK.

[Shinichiro Hamaji]

Comment on attachment 39959 [details]
Patch v1

Thanks. So, I'll mark cq+ for this patch.

FYI, there are three TODOs around this bug:

- I notice that some js-only-tests are added after I created the migration patch. I have the list of them. I'll move them into script-tests directories.
- Make sure to move Geolocation/resources/TEMPLATE.html .
- Check if the files I moved had svn props before I moved these files.

[WebKit Commit Bot]

Comment on attachment 39959 [details]
Patch v1

Clearing flags on attachment: 39959

Committed r48660: <http://trac.webkit.org/changeset/48660>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.