Bug 80602 - Convert some fast/regions pixel tests to reftests
Summary: Convert some fast/regions pixel tests to reftests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 74644 (view as bug list)
Depends on:
Blocks: 57312
  Show dependency treegraph
 
Reported: 2012-03-08 09:37 PST by Rebecca Hauck
Modified: 2013-04-25 05:08 PDT (History)
7 users (show)

See Also:


Attachments
patch for this bug (5.45 KB, patch)
2012-03-09 13:11 PST, hermwong
no flags Details | Formatted Diff | Diff
fix errors in 1st patch file attached. (254.07 KB, patch)
2012-03-09 13:48 PST, hermwong
no flags Details | Formatted Diff | Diff
re-uploading the patch to re-run gtk EWS (254.07 KB, patch)
2012-03-12 12:00 PDT, hermwong
krit: review-
krit: commit-queue-
Details | Formatted Diff | Diff
revised patch based on the reviewer's feedback (61.64 KB, patch)
2012-03-16 11:25 PDT, hermwong
no flags Details | Formatted Diff | Diff
updated comments in the change log (253.99 KB, patch)
2012-03-16 15:19 PDT, hermwong
no flags Details | Formatted Diff | Diff
converted tabs to spaces in the ChangeLog (254.02 KB, patch)
2012-03-16 15:39 PDT, hermwong
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
fixed the failing test in the previous patch & updated comments in the LayoutTests/ChangeLog (254.81 KB, patch)
2012-03-20 11:43 PDT, hermwong
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
updates to LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions-expected.html (254.78 KB, patch)
2012-03-20 15:49 PDT, hermwong
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rebecca Hauck 2012-03-08 09:37:31 PST
Convert the fast/regions/positioned-objects* pixel tests to reftests
Comment 1 hermwong 2012-03-09 13:11:34 PST
Created attachment 131088 [details]
patch for this bug

positioned-objects pixeltest>reftest conversions done at the hackathon
Comment 2 hermwong 2012-03-09 13:48:39 PST
Created attachment 131096 [details]
fix errors in 1st patch file attached.

positioned-objects pixeltest>reftest conversions done at the hackathon
Comment 3 hermwong 2012-03-12 12:00:36 PDT
Created attachment 131373 [details]
re-uploading the patch to re-run gtk EWS

re-uploading the 80602 patch to re-run gtk EWS
Comment 4 Dirk Schulze 2012-03-14 09:46:51 PDT
Comment on attachment 131373 [details]
re-uploading the patch to re-run gtk EWS

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

> LayoutTests/ChangeLog:8
> +        Convert some fast/regions pixel tests to reftests
> +        https://bugs.webkit.org/show_bug.cgi?id=80602
> +
> +        Reviewed by NOBODY (OOPS!).

Please add a more detailed description what you changed. Also, you changed some test files, these need comments as well. You can put the comments directly next to/under the file names in the ChangeLog.

> LayoutTests/fast/regions/positioned-objects-inline-static-in-rtl-regions.html:45
> +is contained in. It should fill the left half of the first region.</p>

Should you see red on the page? If so it is a bad tests :) Why did you remove the comment?

> LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions.html:-55
> -            Some text.<br>
> -             Some text.<br>
> -              Some text.<br>
> -               Some text.<br>
> -                Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br>      </span>

Why did you remove the text and the break lines?
Comment 5 hermwong 2012-03-16 11:25:44 PDT
Created attachment 132326 [details]
revised patch based on the reviewer's feedback
Comment 6 hermwong 2012-03-16 15:19:03 PDT
Created attachment 132392 [details]
updated comments in the change log

updated comments in the change log
Comment 7 Dirk Schulze 2012-03-16 15:22:07 PDT
Should it be reviewed?
Comment 8 WebKit Review Bot 2012-03-16 15:24:57 PDT
Attachment 132392 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
LayoutTests/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 5 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 hermwong 2012-03-16 15:39:50 PDT
Created attachment 132405 [details]
converted tabs to spaces in the ChangeLog
Comment 10 WebKit Review Bot 2012-03-16 18:27:40 PDT
Comment on attachment 132405 [details]
converted tabs to spaces in the ChangeLog

Attachment 132405 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11966553

New failing tests:
fast/regions/positioned-objects-inline-static-spanning-regions.html
Comment 11 hermwong 2012-03-20 11:43:50 PDT
Created attachment 132864 [details]
fixed the failing test in the previous patch & updated comments in the LayoutTests/ChangeLog
Comment 12 WebKit Review Bot 2012-03-20 15:23:11 PDT
Comment on attachment 132864 [details]
fixed the failing test in the previous patch & updated comments in the LayoutTests/ChangeLog

Attachment 132864 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12035066

New failing tests:
fast/dom/error-to-string-stack-overflow.html
fast/regions/positioned-objects-inline-static-spanning-regions.html
Comment 13 hermwong 2012-03-20 15:49:27 PDT
Created attachment 132914 [details]
updates to LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions-expected.html

a line of text needed to be removed from the LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions-expected.html file.
Comment 14 Dirk Schulze 2012-03-21 07:09:47 PDT
Comment on attachment 132914 [details]
updates to LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions-expected.html

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

LGTM. r=me

> LayoutTests/ChangeLog:12
> +        - removed the "no red should be visible..." text from the positioned-objects-inline-static-in-rtl-regions because there was no red in the test file and the statement was irrelevant to the test.   
> +        - removed text from the positioned-objects-inline-static-spanning-regions.html test because it had nothing to do with the test. We based this on the visuals of the test and the fact that the test description made no mention of the text at all     

Please break lines for change logs in the future.
Comment 15 WebKit Review Bot 2012-03-21 07:51:31 PDT
Comment on attachment 132914 [details]
updates to LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions-expected.html

Clearing flags on attachment: 132914

Committed r111545: <http://trac.webkit.org/changeset/111545>
Comment 16 WebKit Review Bot 2012-03-21 07:51:39 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 mitz 2012-03-24 02:08:38 PDT
(In reply to comment #15)
> (From update of attachment 132914 [details])
> Clearing flags on attachment: 132914
> 
> Committed r111545: <http://trac.webkit.org/changeset/111545>

This change accidentally removed Mac expected results for two tests that were not converted into reftests. I restored those results in <http://trac.webkit.org/r111989>.
Comment 18 Mihnea Ovidenie 2013-04-25 05:08:07 PDT
*** Bug 74644 has been marked as a duplicate of this bug. ***