Bug 45610 - post-basic.html and post-frames.html in http/tests/navigation should not be pixel tests
Summary: post-basic.html and post-frames.html in http/tests/navigation should not be p...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other All
: P2 Normal
Assignee: Mihai Parparita
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-11 22:45 PDT by Mihai Parparita
Modified: 2010-09-14 09:35 PDT (History)
6 users (show)

See Also:


Attachments
Patch (286.33 KB, patch)
2010-09-11 22:48 PDT, Mihai Parparita
no flags Details | Formatted Diff | Diff
Patch (286.20 KB, patch)
2010-09-13 09:37 PDT, Mihai Parparita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Parparita 2010-09-11 22:45:46 PDT
post-basic.html and post-frames.html in http/tests/navigation should not be pixel tests
Comment 1 Mihai Parparita 2010-09-11 22:48:12 PDT
Created attachment 67330 [details]
Patch
Comment 2 Mihai Parparita 2010-09-11 22:51:30 PDT
Dimitri? Should be very similar to bug 45307. Let me know if I should convert all the tests in that directory to not be pixel tests in one go (though then the patch might be unwieldy).
Comment 3 Dimitri Glazkov (Google) 2010-09-12 07:09:07 PDT
Comment on attachment 67330 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67330&action=prettypatch

weee!
Comment 4 WebKit Commit Bot 2010-09-12 07:42:57 PDT
Comment on attachment 67330 [details]
Patch

Rejecting patch 67330 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Dimitri Glazkov', u'--force']" exit_code: 255
Last 500 characters of output:
urces/postresult.pl
patching file LayoutTests/platform/chromium-linux/http/tests/navigation/post-basic-expected.checksum
rm 'LayoutTests/platform/chromium-linux/http/tests/navigation/post-basic-expected.checksum'
literal is only avaliable with the XS version at /Library/Perl/5.8.8/Compress/Zlib.pm line 9
BEGIN failed--compilation aborted at /Library/Perl/5.8.8/Compress/Zlib.pm line 9.
Compilation failed in require at /Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/VCSUtils.pm line 1670.

Full output: http://queues.webkit.org/results/3964423
Comment 5 Dimitri Glazkov (Google) 2010-09-12 08:12:24 PDT
Comment on attachment 67330 [details]
Patch

Clearing flags on attachment: 67330

Committed r67328: <http://trac.webkit.org/changeset/67328>
Comment 6 Dimitri Glazkov (Google) 2010-09-12 08:13:09 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 James Robinson 2010-09-12 15:59:07 PDT
Reverted r67328 for reason:

Broke

Committed r67349: <http://trac.webkit.org/changeset/67349>
Comment 8 James Robinson 2010-09-12 16:10:37 PDT
Looks like the render tree expectations were lacking one 0x0 RenderText.  My feeble attempt to rebaseline the platform/mac and chromium expectations was a complete scripting failure so I rolled this out.
Comment 9 Eric Seidel (no email) 2010-09-12 16:58:48 PDT
I believe this caused persistant failures on Mac Leopard.
Comment 10 Eric Seidel (no email) 2010-09-12 16:59:38 PDT
I see, this was rolled out, thanks James!
Comment 11 Mihai Parparita 2010-09-12 19:19:50 PDT
James, thanks for the rollout. You said in #webkit "also it's really unclear to me why those tests have render tree dumps. that seems wrong" Agreed, that's what I've been trying to do, convert the tests in that directory to just use dumpAsText. I'm guessing the reason why post-goback1.html and other tests started to fail (even though I didn't touch them) is because I added a <script> tag to postresult.pl, which is used by those tests too. I'm guessing that showed up as the 0x0 render tree object in the output.

I think the safest thing to do in the redone version of this patch is to branch postresult.pl, so that I don't inadvertedly affect other tests in the directory. Eventually (by the end of the week, hopefully), I hope to have all the tests converted, and then I can get rid of the original postresult.pl (as well as a lot of other stuff in this directory).

Incidentally, the cq should've caught those other test failures, but this couldn't be landed by the cq because of patch apply failures (comment 4). Eric, is this because of svn-apply being unable to handle git binary diffs (even for deletes?), or something else?
Comment 12 Mihai Parparita 2010-09-13 09:37:36 PDT
Created attachment 67416 [details]
Patch

New version of the patch that doesn't touch postresult.pl
Comment 13 Dimitri Glazkov (Google) 2010-09-13 09:48:18 PDT
Comment on attachment 67416 [details]
Patch

Yes!
Comment 14 WebKit Commit Bot 2010-09-13 17:49:53 PDT
Comment on attachment 67416 [details]
Patch

Rejecting patch 67416 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Dimitri Glazkov', u'--force']" exit_code: 255
Last 500 characters of output:
age-that-posts.html
patching file LayoutTests/platform/chromium-linux/http/tests/navigation/post-basic-expected.checksum
rm 'LayoutTests/platform/chromium-linux/http/tests/navigation/post-basic-expected.checksum'
literal is only avaliable with the XS version at /Library/Perl/5.8.8/Compress/Zlib.pm line 9
BEGIN failed--compilation aborted at /Library/Perl/5.8.8/Compress/Zlib.pm line 9.
Compilation failed in require at /Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/VCSUtils.pm line 1670.

Full output: http://queues.webkit.org/results/4024002
Comment 15 Mihai Parparita 2010-09-13 17:56:22 PDT
I filed bug 45723 about the cq bot error.

Dimitri, mind landing this?
Comment 16 Eric Seidel (no email) 2010-09-13 21:22:16 PDT
Comment on attachment 67416 [details]
Patch

I attempted to fix the CQ bot just now.  Hopefully I was successful.  bug 45723.
Comment 17 WebKit Commit Bot 2010-09-13 23:45:07 PDT
Comment on attachment 67416 [details]
Patch

Rejecting patch 67416 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Dimitri Glazkov', u'--force']" exit_code: 255
Last 500 characters of output:
age-that-posts.html
patching file LayoutTests/platform/chromium-linux/http/tests/navigation/post-basic-expected.checksum
rm 'LayoutTests/platform/chromium-linux/http/tests/navigation/post-basic-expected.checksum'
literal is only avaliable with the XS version at /Library/Perl/5.8.8/Compress/Zlib.pm line 9
BEGIN failed--compilation aborted at /Library/Perl/5.8.8/Compress/Zlib.pm line 9.
Compilation failed in require at /Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/VCSUtils.pm line 1670.

Full output: http://queues.webkit.org/results/4060005
Comment 18 Eric Seidel (no email) 2010-09-14 03:57:20 PDT
Comment on attachment 67416 [details]
Patch

I believe it's fixed for real this time.
Comment 19 WebKit Commit Bot 2010-09-14 07:06:40 PDT
Comment on attachment 67416 [details]
Patch

Clearing flags on attachment: 67416

Committed r67466: <http://trac.webkit.org/changeset/67466>
Comment 20 WebKit Commit Bot 2010-09-14 07:06:46 PDT
All reviewed patches have been landed.  Closing bug.
Comment 21 WebKit Review Bot 2010-09-14 09:14:07 PDT
http://trac.webkit.org/changeset/67466 might have broken GTK Linux 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/67465
http://trac.webkit.org/changeset/67466
http://trac.webkit.org/changeset/67467
http://trac.webkit.org/changeset/67468
Comment 22 Mihai Parparita 2010-09-14 09:35:41 PDT
(In reply to comment #21)
> http://trac.webkit.org/changeset/67466 might have broken GTK Linux 32-bit Debug
> The following changes are on the blame list:
> http://trac.webkit.org/changeset/67465
> http://trac.webkit.org/changeset/67466
> http://trac.webkit.org/changeset/67467
> http://trac.webkit.org/changeset/67468

The new failure is that fast/dom/HTMLScriptElement/script-decoding-error-after-setting-src.html crashes, which I doubt was caused by my test-only change. r67467 (to fix bug 45649) seems like a more probable culprit.