Bug 12140 - Change remaining references in tests from bugzilla.opendarwin.org to bugs.webkit.org
Summary: Change remaining references in tests from bugzilla.opendarwin.org to bugs.web...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P4 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-06 11:26 PST by David Kilzer (:ddkilzer)
Modified: 2009-09-30 03:49 PDT (History)
6 users (show)

See Also:


Attachments
Patch v1 (83.21 KB, patch)
2009-09-28 06:36 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
Patch v2 (83.38 KB, patch)
2009-09-29 02:57 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
a script to find dumpAsText tests (580 bytes, text/plain)
2009-09-29 03:02 PDT, Shinichiro Hamaji
no flags Details
Patch v1 (89.66 KB, patch)
2009-09-29 10:47 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
Patch for hrefs v1 (112.18 KB, patch)
2009-09-29 23:37 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
Patch for JS v1 (14.13 KB, patch)
2009-09-30 01:58 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2007-01-06 11:26:05 PST
Just realized that many layout tests contain URLs to bugzilla.opendarwin.org.  It would be nice to change them to bugs.webkit.org eventually.
Comment 1 David Kilzer (:ddkilzer) 2007-03-20 03:36:44 PDT
Some fixes applied in r20304 by bdash.

Comment 2 Mark Rowe (bdash) 2007-03-20 03:59:24 PDT
I didn't touch any layout tests in my change -- regenerating a whole bunch of results for a change like this didn't seem like a great idea.
Comment 3 David Kilzer (:ddkilzer) 2007-03-20 04:20:38 PDT
Genericized the title.  :)

Comment 4 Shinichiro Hamaji 2009-09-28 06:36:57 PDT
Created attachment 40231 [details]
Patch v1
Comment 5 Shinichiro Hamaji 2009-09-28 06:41:25 PDT
How about modifying all dumpAsText tests? Changing them may be harmless because we don't need to re-generate test results for all architectures we support.

The following two-liners created this patch:

% grep bugzilla.opendarwin.org -r --include='*.html' LayoutTests/*/* | perl -ne 's/:.*//; $e=$_; $e=~s/\.html/-expected.txt/; chomp($e); print if -e $e' > l
% perl -i -p -e 's@http://bugzilla.opendarwin.org/@http://bugs.webkit.org/@g' `cat l`

Note that not all test results didn't require change of test results because some URLs were in href attribute, which doesn't affect test results. So, I think we can modify such links even for non-dumpAsText tests, too. If other people like this idea, I'll be happy to work on it.
Comment 6 David Kilzer (:ddkilzer) 2009-09-28 08:26:48 PDT
(In reply to comment #4)
> Created an attachment (id=40231) [details]
> Patch v1

The commands in Comment #5 don't test for the existence of pixel test results.  I know that we're not running a buildbot for pixel tests now, but we should only change the dumpAsText tests.  How do you know the tests that were changed don't have any pixel test results?
Comment 7 David Kilzer (:ddkilzer) 2009-09-28 08:29:27 PDT
(In reply to comment #5)
> How about modifying all dumpAsText tests? Changing them may be harmless because
> we don't need to re-generate test results for all architectures we support.

That sounds fine.

> The following two-liners created this patch:
> 
> % grep bugzilla.opendarwin.org -r --include='*.html' LayoutTests/*/* | perl -ne
> 's/:.*//; $e=$_; $e=~s/\.html/-expected.txt/; chomp($e); print if -e $e' > l
> % perl -i -p -e 's@http://bugzilla.opendarwin.org/@http://bugs.webkit.org/@g'
> `cat l`
> 
> Note that not all test results didn't require change of test results because
> some URLs were in href attribute, which doesn't affect test results. So, I
> think we can modify such links even for non-dumpAsText tests, too. If other
> people like this idea, I'll be happy to work on it.

Yes, if it doesn't affect the pixel test results or render tree dumps, that sounds fine.

You should also consider changing 'http://bugzilla.opendarwin.org/' to 'https://bugs.webkit.org/' while you're at it.  (It's not necessary to change 'http://bugs.webkit.org/' to 'https://bugs.webkit.org/', though, since those links still work fine.)
Comment 8 Shinichiro Hamaji 2009-09-28 11:31:29 PDT
> The commands in Comment #5 don't test for the existence of pixel test results. 
> I know that we're not running a buildbot for pixel tests now, but we should
> only change the dumpAsText tests.  How do you know the tests that were changed
> don't have any pixel test results?

Oh... I thought checking existence of -expected.txt is sufficient. There can be tests which have both -expected.txt and pixel test results? I'll update my patch tomorrow.

Also, I'll do s/http/https/g for my patch as you pointed.

After the dumpAsText patch is committed, I'll send another patch for href attributes.

Thank you for the comments!
Comment 9 Shinichiro Hamaji 2009-09-29 02:57:12 PDT
Created attachment 40290 [details]
Patch v2
Comment 10 Shinichiro Hamaji 2009-09-29 03:02:21 PDT
Created attachment 40291 [details]
a script to find dumpAsText tests

I updated my first line of the previous script so it checks if there are -expected.png . It seems that there were no pixel tests which have -expected.txt and opendarwin URL. I also confirmed pixel tests for these HTMLs succeeded. So, the patch v2 isn't different from the previous patch except s/http/https/g .
Comment 11 David Kilzer (:ddkilzer) 2009-09-29 09:04:19 PDT
Comment on attachment 40290 [details]
Patch v2

r=me  Thanks!
Comment 12 WebKit Commit Bot 2009-09-29 09:40:42 PDT
Comment on attachment 40290 [details]
Patch v2

Rejecting patch 40290 from commit-queue.

Failed to run "['git', 'svn', 'dcommit']" exit_code: 1
Last 500 characters of output:
tml
        trunk/LayoutTests/fast/encoding/charset-koi8-u.html
        trunk/LayoutTests/fast/frames/frame-name-reset.html
        trunk/LayoutTests/fast/frames/iframe-name-and-id.html
        trunk/LayoutTests/fast/loader/url-strip-cr-lf-tab.html
        trunk/LayoutTests/fast/overflow/generated-content-crash.html

    Please use spaces instead to indent.
    If you must commit a file with tabs, use svn propset to set the "allow-tabs" property.
 at /usr/local/libexec/git-core//git-svn line 469
Comment 13 Shinichiro Hamaji 2009-09-29 10:47:27 PDT
Created attachment 40310 [details]
Patch v1
Comment 14 Shinichiro Hamaji 2009-09-29 10:49:01 PDT
s/v1/v3/

OK... I've just eliminated all tabs in these HTMLs. I believe this is better than adding allow-tabs properties for them. If I'm wrong, please let me know and I'll svn propset for them.
Comment 15 David Kilzer (:ddkilzer) 2009-09-29 10:53:29 PDT
Comment on attachment 40310 [details]
Patch v1

Yay, fewer tabs!  r=me
Comment 16 WebKit Commit Bot 2009-09-29 12:15:23 PDT
Comment on attachment 40310 [details]
Patch v1

Clearing flags on attachment: 40310

Committed r48891: <http://trac.webkit.org/changeset/48891>
Comment 17 WebKit Commit Bot 2009-09-29 12:15:27 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 David Kilzer (:ddkilzer) 2009-09-29 12:29:42 PDT
(In reply to comment #8)
> After the dumpAsText patch is committed, I'll send another patch for href
> attributes.

Reopening.
Comment 19 Shinichiro Hamaji 2009-09-29 23:37:05 PDT
Created attachment 40347 [details]
Patch for hrefs v1
Comment 20 Shinichiro Hamaji 2009-09-29 23:39:11 PDT
(In reply to comment #19)
> Created an attachment (id=40347) [details]
> Patch for hrefs v1

This patch modifies opendarwin links in href attributes. Note that I also eliminated tabs in these files. The following two files are using tabs intentionally. I added allow-tabs properties for them.

LayoutTests/fast/css-generated-content/hover-style-change.html
LayoutTests/fast/dom/Element/class-attribute-whitespace.html

Here is the scripts I used.

% grep -l 'href="http://bugzilla.opendarwin.org' -r --include='*.html' LayoutTests/*/* > l
% perl -i -p -e 's@href="http://bugzilla.opendarwin.org/@href="https://bugs.webkit.org/@g' `cat l`

# This file contained whitespaces in its file name.
% perl -i -p -e 's@href="http://bugzilla.opendarwin.org/@href="https://bugs.webkit.org/@g' LayoutTests/fast/encoding/resources/%25%u0435\ 0\ %xx%%%ulike.html

# Run pixel tests excluding resources/*.html
% ./WebKitTools/Scripts/run-webkit-tests --release -p `grep -v 'resources/' l`

# Check the list of resources/*.html and invoke test manually for them.
% grep resources l
LayoutTests/fast/events/resources/drag-outside-window-frame.html
LayoutTests/fast/history/resources/clicked-link-is-visited-2.html
% ./WebKitTools/Scripts/run-webkit-tests --release -p LayoutTests/fast/events/drag-outside-window.html
% ./WebKitTools/Scripts/run-webkit-tests --release -p LayoutTests/fast/history/clicked-link-is-visited.html
% ./WebKitTools/Scripts/run-webkit-tests --release -p LayoutTests/fast/encoding/percent-escaping.html
Comment 21 Eric Seidel (no email) 2009-09-29 23:46:59 PDT
Comment on attachment 40347 [details]
Patch for hrefs v1

?       svn-commit.2.tmp
?       svn-commit.3.tmp
?       svn-commit.4.tmp
?       make-script-test-wrappers
?       svn-commit.5.tmp
?       patch
?       tests
?       log2
?       gitp
?       log
?       d
?       svn-commit.tmp
?       m
?       p
?       old_props
?       diff.rb

Shouldn't be in the diff, but svn-apply will ignore it.

This looks fine to me.
Comment 22 Shinichiro Hamaji 2009-09-29 23:55:50 PDT
Committed r48921: <http://trac.webkit.org/changeset/48921>
Comment 23 Shinichiro Hamaji 2009-09-29 23:57:25 PDT
> Shouldn't be in the diff, but svn-apply will ignore it.
> 
> This looks fine to me.

I've committed manually to make sure the unnecessary lines don't cause any errors.

This seems SCM.run_command in scm.py is redirecting stderr to stdout and the unnecessary lines were added.

  process = subprocess.Popen(args, stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, cwd=cwd)

Maybe we should file a bug for this?
Comment 24 Eric Seidel (no email) 2009-09-30 00:06:49 PDT
Oh true.  That's a recent regression.  Please do file a bug about that.
Comment 25 Shinichiro Hamaji 2009-09-30 01:58:21 PDT
Created attachment 40350 [details]
Patch for JS v1
Comment 26 Shinichiro Hamaji 2009-09-30 02:04:40 PDT
Reopening this bug for fix of JS. They are used by dumpAsText tests so it should be safe to modify the test expectations.
Comment 27 David Kilzer (:ddkilzer) 2009-09-30 03:41:38 PDT
Comment on attachment 40350 [details]
Patch for JS v1

r=me
Comment 28 WebKit Commit Bot 2009-09-30 03:49:15 PDT
Comment on attachment 40350 [details]
Patch for JS v1

Clearing flags on attachment: 40350

Committed r48925: <http://trac.webkit.org/changeset/48925>
Comment 29 WebKit Commit Bot 2009-09-30 03:49:20 PDT
All reviewed patches have been landed.  Closing bug.