Bug 118853 - More conversions from MathML pixel tests to reftests
Summary: More conversions from MathML pixel tests to reftests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords:
Depends on: 118812
Blocks: 118916
  Show dependency treegraph
 
Reported: 2013-07-18 09:27 PDT by Frédéric Wang (:fredw)
Modified: 2013-10-09 01:19 PDT (History)
8 users (show)

See Also:


Attachments
Patch V1 (470.60 KB, patch)
2013-07-18 09:31 PDT, Frédéric Wang (:fredw)
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (577.37 KB, application/zip)
2013-07-18 10:45 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (826.43 KB, application/zip)
2013-07-18 11:35 PDT, Build Bot
no flags Details
Patch V2 (471.19 KB, patch)
2013-07-18 11:37 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch V3 (471.56 KB, patch)
2013-07-19 02:16 PDT, Frédéric Wang (:fredw)
cfleizach: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Patch Final Version (471.58 KB, patch)
2013-07-19 09:52 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2013-07-18 09:27:56 PDT
I've prepared more conversions to reftests. After that it should only remain things testing operators and radicals, but I'll probably wait Martin's code refactoring on stretchy operators before considering them.

I've removed row-alignment.xhtml since all its tests are already verified elsewhere.

I couldn't find a reliable way to convert xHeigth.xhtml. This seemed to have been written by François Sausset to test the internal MathML code (bug 41535) but this test for "vertical-align: middle" is artificial and does not correspond to anything that is described in the MathML spec (for example Gecko renders that differently). Even in the bug report, they do not seem to agree on the relevance of the test. When the implementation was rewritten with flexboxes, the test was modified again by Dave Barton and I suspect it became even less relevant. So I've removed this test too.
Comment 1 Frédéric Wang (:fredw) 2013-07-18 09:31:46 PDT
Created attachment 206995 [details]
Patch V1
Comment 2 Build Bot 2013-07-18 10:45:07 PDT
Comment on attachment 206995 [details]
Patch V1

Attachment 206995 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1110595

New failing tests:
mathml/presentation/fenced-mi.html
Comment 3 Build Bot 2013-07-18 10:45:09 PDT
Created attachment 207000 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.3
Comment 4 Frédéric Wang (:fredw) 2013-07-18 11:21:37 PDT
(In reply to comment #3)
> Created an attachment (id=207000) [details]
> Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
> 
> The attached test failures were seen while running run-webkit-tests on the mac-ews.
> Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

This looks like a bug on Mac. I've opened bug 118856.
Comment 5 Build Bot 2013-07-18 11:35:00 PDT
Comment on attachment 206995 [details]
Patch V1

Attachment 206995 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1104562

New failing tests:
mathml/presentation/fenced-mi.html
Comment 6 Build Bot 2013-07-18 11:35:03 PDT
Created attachment 207005 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.3
Comment 7 Frédéric Wang (:fredw) 2013-07-18 11:37:42 PDT
Created attachment 207006 [details]
Patch V2
Comment 8 chris fleizach 2013-07-18 23:01:57 PDT
Comment on attachment 207006 [details]
Patch V2

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

> LayoutTests/ChangeLog:31
> +        * mathml/xHeight.xhtml: Removed.

is the xHeight test covered in one of the new tests?

> LayoutTests/mathml/presentation/tables-spans-dynamic.html:8
> +

can you add a comment what this one is testing

> LayoutTests/mathml/presentation/tables-spans.html:13
> +

can you add an explantation what this is testing
Comment 9 Frédéric Wang (:fredw) 2013-07-18 23:08:36 PDT
(In reply to comment #8)
> (From update of attachment 207006 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207006&action=review
> 
> > LayoutTests/ChangeLog:31
> > +        * mathml/xHeight.xhtml: Removed.
> 
> is the xHeight test covered in one of the new tests?

No, see comment 0. This was added in bug 41535.

> 
> > LayoutTests/mathml/presentation/tables-spans-dynamic.html:8
> > +
> 
> can you add a comment what this one is testing
> 
> > LayoutTests/mathml/presentation/tables-spans.html:13
> > +
> 
> can you add an explantation what this is testing

OK, I will do so. They are basically testing the rowspan and columnspan attributes (rowspan and colspan in HTML). And one sets these attributes dynamically via Javascript.
Comment 10 Frédéric Wang (:fredw) 2013-07-19 02:16:25 PDT
Created attachment 207073 [details]
Patch V3

Here is a new patch with comments for the tables-span* tests
Comment 11 Frédéric Wang (:fredw) 2013-07-19 08:51:30 PDT
Comment on attachment 207073 [details]
Patch V3

Thanks for the review.
Comment 12 WebKit Commit Bot 2013-07-19 09:15:46 PDT
Comment on attachment 207073 [details]
Patch V3

Rejecting attachment 207073 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 207073, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
/git.webkit.org/WebKit
   9e234c3..1573a7e  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 152901 = 9e234c3371e5786a5033d4e125d1edc45640fa02
r152902 = 1573a7e21e7d883d1d8c48ed6358e5e4aec448f3
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.appspot.com/results/1099884
Comment 13 Frédéric Wang (:fredw) 2013-07-19 09:25:22 PDT
Any idea what the problem is? Should I update my local git repository and refresh the patch?
Comment 14 chris fleizach 2013-07-19 09:35:36 PDT
(In reply to comment #13)
> Any idea what the problem is? Should I update my local git repository and refresh the patch?

That's probably the easiest thing to try
Comment 15 Frédéric Wang (:fredw) 2013-07-19 09:52:37 PDT
Created attachment 207109 [details]
Patch Final Version

OK, there were conflicts in the Mac's test expectations after my other changes that have just been committed.
Comment 16 WebKit Commit Bot 2013-07-19 13:12:53 PDT
Comment on attachment 207109 [details]
Patch Final Version

Clearing flags on attachment: 207109

Committed r152923: <http://trac.webkit.org/changeset/152923>
Comment 17 WebKit Commit Bot 2013-07-19 13:12:57 PDT
All reviewed patches have been landed.  Closing bug.