| Summary: | Change remaining references in tests from bugzilla.opendarwin.org to bugs.webkit.org | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||||||||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | commit-queue, eric, hamaji, mitz, mrowe, webkit | ||||||||||||||
| Priority: | P4 | ||||||||||||||||
| Version: | 420+ | ||||||||||||||||
| Hardware: | Mac | ||||||||||||||||
| OS: | OS X 10.4 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
David Kilzer (:ddkilzer)
2007-01-06 11:26:05 PST
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. Genericized the title. :) Created attachment 40231 [details]
Patch v1
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. (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? (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.) > 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!
Created attachment 40290 [details]
Patch v2
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 on attachment 40290 [details]
Patch v2
r=me Thanks!
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
Created attachment 40310 [details]
Patch v1
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 on attachment 40310 [details]
Patch v1
Yay, fewer tabs! r=me
Comment on attachment 40310 [details] Patch v1 Clearing flags on attachment: 40310 Committed r48891: <http://trac.webkit.org/changeset/48891> All reviewed patches have been landed. Closing bug. (In reply to comment #8) > After the dumpAsText patch is committed, I'll send another patch for href > attributes. Reopening. Created attachment 40347 [details]
Patch for hrefs v1
(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 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.
Committed r48921: <http://trac.webkit.org/changeset/48921> > 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?
Oh true. That's a recent regression. Please do file a bug about that. Created attachment 40350 [details]
Patch for JS v1
Reopening this bug for fix of JS. They are used by dumpAsText tests so it should be safe to modify the test expectations. Comment on attachment 40350 [details]
Patch for JS v1
r=me
Comment on attachment 40350 [details] Patch for JS v1 Clearing flags on attachment: 40350 Committed r48925: <http://trac.webkit.org/changeset/48925> All reviewed patches have been landed. Closing bug. |