RESOLVED FIXED 22492
UTF7 test cases from bug 21635 fail on some buildbots
https://bugs.webkit.org/show_bug.cgi?id=22492
Summary UTF7 test cases from bug 21635 fail on some buildbots
Darin Fisher (:fishd, Google)
Reported 2008-11-25 13:03:48 PST
UTF7 test cases from bug 21635 fail on some buildbots They were disabled here: http://trac.webkit.org/changeset/38759 This bug is about restoring the tests and fixing whatever caused the buildbots to be unhappy.
Attachments
patch (utf-7 files, changelog, php files) (9.87 KB, patch)
2008-12-01 21:21 PST, Jungshik Shin
no flags
patch (include everything - binary or not) (43.30 KB, patch)
2008-12-02 09:51 PST, Jungshik Shin
no flags
updated patch that works without iconv module in php (43.01 KB, patch)
2008-12-02 10:55 PST, Jungshik Shin
darin: review+
Mark Rowe (bdash)
Comment 1 2008-11-25 14:24:39 PST
Jungshik Shin
Comment 2 2008-12-01 20:12:09 PST
I have been puzzled as to why only utf-7 cases failed on buildbot while locally they all pass (on Mac OS X with webkit and on Windows with Chrome). Looking at results.html file, I thought somehow '/' is prepended to the path for php files ('/resources/echo-query-param.php' instead of 'resources/echo-query-param.php'), but had no idea how. I've just set up webkit build environment on Windows as well and realized that utf-{16,32}{be,le} html files were missing. I scratched the head and took a look at http://trac.webkit.org/changeset/38755. They're indeed missing. It turned out that 'patch' *silently* ignores binary files. That still did not explain why utf-7 (for which html and result files were landed) cases failed. On Windows webkit, those tests timed out. Only then did I realize that I made a blunder of not including resources/echo-query-param.php and resources/check-query-param.php in the patch for bug 21635. I'm uploading a tar.gz file (for binary files) and a patch file that includes 2 php files and ChangeLog.
Jungshik Shin
Comment 3 2008-12-01 21:21:33 PST
Created attachment 25661 [details] patch (utf-7 files, changelog, php files) This patch enables UTF-7 tests again and adds two PHP files that I should have incldued in the patch for bug 21635.
Jungshik Shin
Comment 4 2008-12-02 09:51:13 PST
Created attachment 25674 [details] patch (include everything - binary or not) This patch includes everything (utf-16,32 files as well as utf-7 tests and php scripts) necessary for this bug and bug 21635. It turned out that 'svn-apply' can deal with b64-encoded binary files.
Jungshik Shin
Comment 5 2008-12-02 10:55:58 PST
Created attachment 25676 [details] updated patch that works without iconv module in php With the previous patch, tests timed out on Windows (webkit) perhaps because php in cygwin does not have iconv module. After converting php to perl, I realized that iconv module is not necessary at all in php, which turned out to be the case. This patch is the same as the previous one except that php scripts do not have 3 iconv function calls at the beginning.
Darin Adler
Comment 6 2008-12-03 12:32:36 PST
Comment on attachment 25676 [details] updated patch that works without iconv module in php r=me
Alexey Proskuryakov
Comment 7 2008-12-03 23:30:30 PST
Committed revision 38988. We usually use patches rooted at top level WebKit directory - when landing this, I blindly issued svn-apply at top level directory as usual, and had to clean up a lot of misplaced files, because it was supposed to be applied in LayoutTests/.
Jungshik Shin
Comment 8 2008-12-04 13:23:08 PST
(In reply to comment #7) > Committed revision 38988. Thank you for checking the patch in and sorry for 'mis-rooting' it. I was lazy and made the patch for LayoutTests to avoid pick up other changes lurking around in WebCore. I'll make sure to make patch from the top of the tree.
Alexey Proskuryakov
Comment 9 2008-12-04 14:05:55 PST
No problem. FYI, both prepare-ChangeLog and svn-create-patch take a space-separated list of files/directories to scan, so it's possible to skip unneeded changes, or just speed up their operation, e.g.: svn-create-patch WebCore LayoutTests/ChangeLog LayoutTests/fast/encoding
Note You need to log in before you can comment on or make changes to this bug.