Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 17, 2012, at 5:39 PM, Dirk Pranke dpra...@chromium.org wrote: I probably polarized things by saying that your input was less valuable than those of people who were long-time users. I did not mean to offend, and I'm sorry. I certainly didn't mean to imply that I was not listening or not open to feedback from anyone, and I hope I've made that clear. That does not mean I will agree to any proposal without argument, obviously. Apology accepted. I, in turn, am sorry for getting overly grumpy at you. On May 17, 2012, at 4:57 PM, Ojan Vafai o...@chromium.org wrote: All the proposals that are not just bikeshedding we seem to agree on and will happen (the half-dozen things I listed above). I think it's great that we came up with some changes that seem useful and which no one is objecting to. We should certainly do those ASAP! Unfortunately they are buried in the middle of a megathread. Maybe it would be worth it to fork off a new thread just to notify the folks who may not be following this one, which would identify the changes and give examples of a new syntax. We can discuss further changes separately, with that set as a baseline. Sure TEXT, IMAGE, etc are not very clear, but noone has actually proposed something better. I believe proposals were made which were more clear, but were rejected for other reasons (mainly not being as familiar to people used to the format afaict). Let me add another. I would propose the following replacements for current states: neither TEXT nor IMAGE == (continue to say nothing) TEXT or TEXT+IMAGE == FAIL FAIL would mean the test fails - for text-only tests, it means text failure, for render tree tests it means text failure (who cares if the pixel test somehow accidentally pass at that point, that's not a meaningfully distinct state), for ref tests it would mean a reference failure IMAGE == PIXELFAIL or PIXELONLYFAIL This would be applied only to render tree tests and only in the case where only the pixel test mode fails, not text test. We have historically called this mode pixel tests not image tests, let's be consistent. Not applicable to text tests or reference tests. Mutually exclusive with FAIL. TEXT IMAGE == FLAKY If one of the text tests or the image tests will fail but maybe not both, that means the test is nondeterministic, so it should be marked as flaky and its results should not affect greenness of the bots, so long as it does not hang or crash. It doesn't seem like we currently have a FLAKY result expectation based on the bots, you are supposed to indicate it by listing all possible kinds of failures, but that seems unhelpful. Also, a flaky test that sometimes entirely passes on multiple runs in a row will turn the bots red, which seems bad. Let's just have FLAKY state instead where we don't get upset whether the test passes or fails. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 10:37 PM, Benjamin Poulain benja...@webkit.orgwrote:
On Wed, May 16, 2012 at 9:34 PM, Ryosuke Niwa rn...@webkit.org wrote:
I would go for something verbose in a easy to understand format. E.g.
{ test: 'fast/html/keygen.html',
platform: 'mac, linux',
bug: 'webkit.org/b/6',
expectedResults: ['crash', 'text', 'image'],
action: 'skip' }
For the tests that need rebaseline or new results, I would leave them
out of the file. I would use an second file, with entries sorted by bug
number.
I don't know all the problems related to test_expectations so that is
just some ideas from what I encountered in the past.
As far as I know, a lot of people edit this file on a text editor so the
syntax needs be simple enough for that.
Also, complex syntax tends to be error-prone. One of the pain points of the
current syntax is that many people forget to add : and =, or put modifiers
in where expectations appear and vice versa.
- Rysouke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 11:04 PM, Ryosuke Niwa rn...@webkit.org wrote:
{ test: 'fast/html/keygen.html',
platform: 'mac, linux',
bug: 'webkit.org/b/6',
expectedResults: ['crash', 'text', 'image'],
action: 'skip' }
As far as I know, a lot of people edit this file on a text editor so the
syntax needs be simple enough for that.
I do to, and it is why I split the dictionary on multiples lines. :)
Also, complex syntax tends to be error-prone. One of the pain points of the
current syntax is that many people forget to add : and =, or put modifiers
in where expectations appear and vice versa.
I did that mistake myself, which is why I suggested a well known
syntax (it would also be easy to verify the syntax automatically if it
is easy).
That was just an example, I am not pushing for JSON in particular. I
am just in favor of something so simple you do not have to remember
anything.
In any case, I prefer your last suggestion over the first one.
Cheers,
Benjamin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 11:33 PM, Benjamin Poulain benja...@webkit.orgwrote: On Wed, May 16, 2012 at 11:04 PM, Ryosuke Niwa rn...@webkit.org wrote: Also, complex syntax tends to be error-prone. One of the pain points of the current syntax is that many people forget to add : and =, or put modifiers in where expectations appear and vice versa. I did that mistake myself, which is why I suggested a well known syntax (it would also be easy to verify the syntax automatically if it is easy). Yeah, I think the biggest problem is that we differentiate platforms and expectations yet we mix modifiers (e.g. SKIP, WONTFIX, etc...) with platforms. That was just an example, I am not pushing for JSON in particular. I am just in favor of something so simple you do not have to remember anything. That's why I was advocating for getting rid of the distinction between modifiers (including platforms) and expectations so that we can put them in any order. Given that all tokens are unique, it's silly to require humans to sort them (rantwe even used to refuse to run the entire layout tests for a single syntax error!/rant). If anything, we can run a script that automatically sort those tokens periodically. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 16, 2012, at 10:39 PM, Dirk Pranke dpra...@chromium.org wrote: There was a semi-logical basis, in that the stuff on the right of the test clearly specified the outcome of the test (PASS, IMAGE, TEXT, etc.). The stuff on the left was less well defined: there's the bug numbers, the platform/configuration info (MAC LINUX RELEASE, etc.), and some other stuff that there is less of a good place for (SLOW, SKIP, WONTFIX). I think it makes sense to syntactically separate at least the platform/configuration keywords from the outcome keywords. It might be nice to separate the other things somehow as well, but I don't have any great ideas for things that are clearly better than the existing left/right convention. SKIP and WONTFIX seem parallel to PASS to me. I assume TEXT means a failure of text output and IMAGE means a pixel test failure? In that case those are parallel too. What does the build configuration info do? Does it apply the line to only those configurations? If that is the case, it does seem potentially different in kind, though maybe also better expressed by being able to combine multiple test_expectations files fro different platform/ directories. - Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 1:06 AM, Maciej Stachowiak m...@apple.com wrote: On May 16, 2012, at 10:39 PM, Dirk Pranke dpra...@chromium.org wrote: There was a semi-logical basis, in that the stuff on the right of the test clearly specified the outcome of the test (PASS, IMAGE, TEXT, etc.). The stuff on the left was less well defined: there's the bug numbers, the platform/configuration info (MAC LINUX RELEASE, etc.), and some other stuff that there is less of a good place for (SLOW, SKIP, WONTFIX). I think it makes sense to syntactically separate at least the platform/configuration keywords from the outcome keywords. It might be nice to separate the other things somehow as well, but I don't have any great ideas for things that are clearly better than the existing left/right convention. SKIP and WONTFIX seem parallel to PASS to me. I assume TEXT means a failure of text output and IMAGE means a pixel test failure? Yes. What does the build configuration info do? Does it apply the line to only those configurations? If that is the case, it does seem potentially different in kind, though maybe also better expressed by being able to combine multiple test_expectations files fro different platform/ directories. Yes. e.g. if you have: BUGWK12345 WIN DEBUG : test.html = TEXT then we expect test.html to have a failure of text output (i.e. -expected.txt doesn't match) on Windows debug builds and expect it to pass on all other platforms. I actually don't like the fact we're sort of duplicating platform fallback structure here (e.g. we should be able to have a separate text_expectations.txt in platform/chromium-win, platform/chromium-mac, etc... to do the same but we don't currently support that) but there was a strong resistance to do so among Chromium contributors the last time I checked because that'll increase the number of files to maintain. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
I have a proposal that hopefully addresses everyone's concerns, is minimally different from the current format *and* unifies the format with Skipped lists (i.e. Skipped lists as they exist today are valid test_expectations.txt format). The changes from the current format are as follows: -Leaving out any outcomes means the test is skipped, unless the test has a SLOW modifier, in which case the implied outcome is PASS. -Remove the SKIP modifier entirely. -Make everything but the test name case-insensitive. -Have the test name be the last item on the line -Separate modifiers/outcomes/testname with a common delimiter (i.e. :) -Any line starting with // or # is a comment -Including a bug entry is optional (maybe only if the test is skipped or wontfix?) -Bugs are listed as URLs, except for the bug_ojan format. Examples: foo/bar.html # Skipped wontfix : foo/bar.html # Skipped and we never intend to fix it. For things like dashboard compatibility tests that only Apple will ever want to make pass wontfix : text : foo/bar.html # We never intend to fix this, but we expect it to run and fail text diff. Will still fail if the test times out or crashes. webkit.org/b/12345 : text image : foo/bar.html # Flaky. Sometimes only fails text diff, sometimes only fails pixel diff. slow mac debug : foo/bar.html # Given extra time to run on mac debug, but is expected to pass. image+text : foo/bar.html # Fails both text and pixel diffs bug_ojan : fail : foo/bar.html # Fails and ojan is taking responsibility to address the failure. bug_ojan : foo/bar.html # Skipped and ojan is taking responsibility to address it. # The following would give lint errors image : text : foo/bar.html # two outcomes listed in separate groupings slow text : foo/bar.html # outcome listed with non-outcome modifier crbug.com/12345 text : foo/bar.html # outcome listed with non-outcome modifier crbug.com/12345 : wontfix : foo/bar.html # two non-outcomes modifiers listed in separate groupings On Thu, May 17, 2012 at 1:12 AM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 1:06 AM, Maciej Stachowiak m...@apple.com wrote: On May 16, 2012, at 10:39 PM, Dirk Pranke dpra...@chromium.org wrote: There was a semi-logical basis, in that the stuff on the right of the test clearly specified the outcome of the test (PASS, IMAGE, TEXT, etc.). The stuff on the left was less well defined: there's the bug numbers, the platform/configuration info (MAC LINUX RELEASE, etc.), and some other stuff that there is less of a good place for (SLOW, SKIP, WONTFIX). I think it makes sense to syntactically separate at least the platform/configuration keywords from the outcome keywords. It might be nice to separate the other things somehow as well, but I don't have any great ideas for things that are clearly better than the existing left/right convention. SKIP and WONTFIX seem parallel to PASS to me. I assume TEXT means a failure of text output and IMAGE means a pixel test failure? Yes. What does the build configuration info do? Does it apply the line to only those configurations? If that is the case, it does seem potentially different in kind, though maybe also better expressed by being able to combine multiple test_expectations files fro different platform/ directories. Yes. e.g. if you have: BUGWK12345 WIN DEBUG : test.html = TEXT then we expect test.html to have a failure of text output (i.e. -expected.txt doesn't match) on Windows debug builds and expect it to pass on all other platforms. I actually don't like the fact we're sort of duplicating platform fallback structure here (e.g. we should be able to have a separate text_expectations.txt in platform/chromium-win, platform/chromium-mac, etc... to do the same but we don't currently support that) but there was a strong resistance to do so among Chromium contributors the last time I checked because that'll increase the number of files to maintain. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 17, 2012, at 4:30 AM, Ojan Vafai wrote: I have a proposal that hopefully addresses everyone's concerns, is minimally different from the current format *and* unifies the format with Skipped lists (i.e. Skipped lists as they exist today are valid test_expectations.txt format). The changes from the current format are as follows: -Leaving out any outcomes means the test is skipped, unless the test has a SLOW modifier, in which case the implied outcome is PASS. -Remove the SKIP modifier entirely. -Make everything but the test name case-insensitive. -Have the test name be the last item on the line -Separate modifiers/outcomes/testname with a common delimiter (i.e. :) -Any line starting with // or # is a comment -Including a bug entry is optional (maybe only if the test is skipped or wontfix?) -Bugs are listed as URLs, except for the bug_ojan format. Examples: foo/bar.html # Skipped wontfix : foo/bar.html # Skipped and we never intend to fix it. For things like dashboard compatibility tests that only Apple will ever want to make pass wontfix : text : foo/bar.html # We never intend to fix this, but we expect it to run and fail text diff. Will still fail if the test times out or crashes. webkit.org/b/12345 : text image : foo/bar.html # Flaky. Sometimes only fails text diff, sometimes only fails pixel diff. slow mac debug : foo/bar.html # Given extra time to run on mac debug, but is expected to pass. image+text : foo/bar.html # Fails both text and pixel diffs bug_ojan : fail : foo/bar.html # Fails and ojan is taking responsibility to address the failure. bug_ojan : foo/bar.html # Skipped and ojan is taking responsibility to address it. # The following would give lint errors image : text : foo/bar.html # two outcomes listed in separate groupings slow text : foo/bar.html # outcome listed with non-outcome modifier crbug.com/12345 text : foo/bar.html # outcome listed with non-outcome modifier crbug.com/12345 : wontfix : foo/bar.html # two non-outcomes modifiers listed in separate groupings Direction seems good. I’d like to think it through and give some more detailed feedback on some of the specifics, but here are some immediate thoughts. I don’t understand why we need the : or + separators. Why not just a list of space-separated words and URLs that can be a mix of modifiers and outcomes? I don’t think we need to support the same words for both modifiers and outcomes. If we want to enforce some kind of order, that would be OK, to reduce arbitrary differences. Instead of bug_ojan I’d prefer a format more like bug(ojan). I’d like to see a list of the modifier and outcome words so I can easily review them. Maybe the test specifier (name or directory name) should come first, before the modifiers and outcomes. -- Darin___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 9:11 AM, Darin Adler da...@apple.com wrote: On May 17, 2012, at 4:30 AM, Ojan Vafai wrote: I have a proposal that hopefully addresses everyone's concerns, is minimally different from the current format *and* unifies the format with Skipped lists (i.e. Skipped lists as they exist today are valid test_expectations.txt format). The changes from the current format are as follows: -Leaving out any outcomes means the test is skipped, unless the test has a SLOW modifier, in which case the implied outcome is PASS. -Remove the SKIP modifier entirely. -Make everything but the test name case-insensitive. -Have the test name be the last item on the line -Separate modifiers/outcomes/testname with a common delimiter (i.e. :) -Any line starting with // or # is a comment -Including a bug entry is optional (maybe only if the test is skipped or wontfix?) -Bugs are listed as URLs, except for the bug_ojan format. Examples: foo/bar.html # Skipped wontfix : foo/bar.html # Skipped and we never intend to fix it. For things like dashboard compatibility tests that only Apple will ever want to make pass wontfix : text : foo/bar.html # We never intend to fix this, but we expect it to run and fail text diff. Will still fail if the test times out or crashes. webkit.org/b/12345 : text image : foo/bar.html # Flaky. Sometimes only fails text diff, sometimes only fails pixel diff. slow mac debug : foo/bar.html # Given extra time to run on mac debug, but is expected to pass. image+text : foo/bar.html # Fails both text and pixel diffs bug_ojan : fail : foo/bar.html # Fails and ojan is taking responsibility to address the failure. bug_ojan : foo/bar.html # Skipped and ojan is taking responsibility to address it. # The following would give lint errors image : text : foo/bar.html # two outcomes listed in separate groupings slow text : foo/bar.html # outcome listed with non-outcome modifier crbug.com/12345 text : foo/bar.html # outcome listed with non-outcome modifier crbug.com/12345 : wontfix : foo/bar.html # two non-outcomes modifiers listed in separate groupings Direction seems good. I’d like to think it through and give some more detailed feedback on some of the specifics, but here are some immediate thoughts. I don’t understand why we need the : or + separators. Why not just a list of space-separated words and URLs that can be a mix of modifiers and outcomes? I don’t think we need to support the same words for both modifiers and outcomes. If we want to enforce some kind of order, that would be OK, to reduce arbitrary differences. Maybe it's the Stockholm syndrome talking, but I actually quite like the clear delineation between test modifiers and test expectations. My intuitive reaction to the attempts to bunch them all together is that it will make both writing and reading of a test expectation harder. Having them on either side of the test file also helps, since the test file functions as a natural delimiter. I certainly don't want to be trying to recall who's-on-firsts of modifiers and expectations, if they are on the same side of the file. Sure, there are some bugs in existing formats (like SLOW being a modifier), but can we just fix those and avoid boiling the ocean? :) :DG Instead of bug_ojan I’d prefer a format more like bug(ojan). I’d like to see a list of the modifier and outcome words so I can easily review them. Maybe the test specifier (name or directory name) should come first, before the modifiers and outcomes. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
Seriously, syntax is a significant barrier. Having to know which special characters to use. I don’t see this “clear delineation” you speak of. Just special punctuation I have to use to satisfy the computer. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
Could that be because you haven't actually tried using it? :) On Thu, May 17, 2012 at 4:40 PM, Darin Adler da...@apple.com wrote: Seriously, syntax is a significant barrier. Having to know which special characters to use. I don’t see this “clear delineation” you speak of. Just special punctuation I have to use to satisfy the computer. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
Sorry, didn't mean to come across as flip. To illustrate, here's an example: WIN SNOWLEOPARD : foo.html = TEXT IMAGE # on all windows platforms and SL, test foo.html is flaky -- fails either with text or pixel results. Easy. TEXT WIN SNOWLEOPARD IMAGE foo.html # fails with text on all windows platforms and SL, oh, and actually it's flaky with pixel results, the foo.html test does. Less easy. Worse yet, since the expectations are commonly updated, you now need to fish through the whole set of specifiers and mentally separate them into expectations and modifiers, then update. It's more likely that this will happen: TEXT WIN SNOWLEOPARD IMAGE LINUX foo.html #test foo.html fails with text on all windows platforms and SL, oh, and actually it's flaky with pixel results, oh, and also on linux, the foo.html test does. :DG On Thu, May 17, 2012 at 4:43 PM, Dimitri Glazkov dglaz...@chromium.org wrote: Could that be because you haven't actually tried using it? :) On Thu, May 17, 2012 at 4:40 PM, Darin Adler da...@apple.com wrote: Seriously, syntax is a significant barrier. Having to know which special characters to use. I don’t see this “clear delineation” you speak of. Just special punctuation I have to use to satisfy the computer. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 9:11 AM, Darin Adler da...@apple.com wrote: I don’t understand why we need the : or + separators. Unfortunately, image+text is different is image text. The former indicates that the test will have mismatches in both .txt and .png files while the latter indicates that either .txt or .png mismatches (i.e. the test is flaky in terms of which failure it causes). I personally think this is extremely confusing, and we should get rid of this distinction altogether, and treat Image Text like both image text and image+text but I'll defer that to a separate discussion. I'd say we should rename text, image, and text+image to TextFailure, ImageFailure, TextAndImageFailures for now to clear the confusion. Why not just a list of space-separated words and URLs that can be a mix of modifiers and outcomes? I don’t think we need to support the same words for both modifiers and outcomes. If we want to enforce some kind of order, that would be OK, to reduce arbitrary differences. My thoughts exactly and hence my original proposal. On Thu, May 17, 2012 at 9:40 AM, Darin Adler da...@apple.com wrote: Seriously, syntax is a significant barrier. Having to know which special characters to use. I don’t see this “clear delineation” you speak of. Just special punctuation I have to use to satisfy the computer. +zillion to that. On Thu, May 17, 2012 at 9:43 AM, Dimitri Glazkov dglaz...@chromium.org wrote: Could that be because you haven't actually tried using it? :) I've been using it for 2 years and still find the syntax completely arbitrary and annoying. Otherwise, I won't be starting this thread in the first place :) e.g. I hate : and = for their guts as I've repeatedly forgotten to put them in the right place. It's so bad that I have to run a regex on my text editor to auto-insert them whenever I'm adding new entries. It's a significant cost I'm paying every time I have to edit that file. The problem with delimiter is that I would fee as if l I'm done as soon as I typed in Win Mac because that's the information I need to have so I have to actively think that I have to insert : afterwards. If we really wanted, I would advocate for grouping them in [ ] as I've suggested before. They're much easier to remember than delimiters like : and = as they're grouping symbols instead. So how about this? foo/bar.html WontFix foo/bar.html WontFix TextFailure foo/bar.html webkit.org/b/12345 TextFailure ImageFailure foo/bar.html ImageAndTextFailures [Win Mac Debug] foo/bar.html Bug(ojan) Fail foo/bar.html Bug(rniwa) foo/bar.html - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 9:26 AM, Dimitri Glazkov dglaz...@chromium.orgwrote: I actually quite like the clear delineation between test modifiers and test expectations. Me too. Please please please leave them on opposite sides. All these proposals that try to put them both before the test name are much harder for me to understand. Even dropping the all-caps format for keywords is a loss for me as this differentiation makes it easier for me to scan the line and distinguish the three important pieces of a line: what, where, and how. I find most of the changes that have been proposed so far to be a significant decrease in clarity, except for the idea to change BUGXX### into links (for all bug trackers, not just non-WebKit ones), which I think would be fine; and the idea of moving SLOW, SKIP, and WONTFIX from the left side to the right, as these seem more like expectations about how the test will run than platforms on which the test fails. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 11:08 AM, Peter Kasting pkast...@google.com wrote: On Thu, May 17, 2012 at 9:26 AM, Dimitri Glazkov dglaz...@chromium.orgwrote: I actually quite like the clear delineation between test modifiers and test expectations. Me too. Please please please leave them on opposite sides. All these proposals that try to put them both before the test name are much harder for me to understand. Even dropping the all-caps format for keywords is a loss for me as this differentiation makes it easier for me to scan the line and distinguish the three important pieces of a line: what, where, and how. I find most of the changes that have been proposed so far to be a significant decrease in clarity, except for the idea to change BUGXX### into links (for all bug trackers, not just non-WebKit ones), which I think would be fine; and the idea of moving SLOW, SKIP, and WONTFIX from the left side to the right, as these seem more like expectations about how the test will run than platforms on which the test fails. Interesting. I find the new format (I've replaced [] by () since that worked better for me) much easier to read than the old format because now all meta data is in one place. For me, I'm most annoyed by the unreasonably long bug URLs in the new format. Maybe putting the URL isn't such a great idea after all :( crbug.com/24182 Slow (Mac Debug) fast/css/large-list-of-rules-crash.html crbug.com/24182 Slow (Linux Mac Debug) fast/dom/Window/window-postmessage-clone-really-deep-array.html crbug.com/24182 Slow (Mac Debug) fast/forms/select-set-length-with-mutation-remove.html crbug.com/24182 Slow (All) fast/js/regexp-overflow.html crbug.com/24182 Slow (Debug) fast/js/toString-and-valueOf-override.html crbug.com/24182 Slow (Debug) html5lib/webkit-resumer.html crbug.com/24182 Slow (Win Release) http/tests/loading/ onload-vs-immediate-refresh.pl crbug.com/79910 Image (SnowLeopard) fast/text/international/bidi-linebreak-001.html crbug.com/79910 Image (SnowLeopard) fast/text/international/bidi-linebreak-002.html crbug.com/79910 Image (SnowLeopard) fast/text/international/bidi-linebreak-003.html webkit.org/b/58924 Pass Timeout (Mac) plugins/mouse-click-iframe-to-plugin.html webkit.org/b/60099 Pass Crash (Debug) fast/dom/Attr/access-after-element-destruction.html webkit.org/b/60097 Pass Text (Debug) fast/dom/HTMLLinkElement/link-and-subresource-test.html BUGCR24182 SLOW MAC DEBUG : fast/css/large-list-of-rules-crash.html = PASS BUGCR24182 SLOW LINUX MAC DEBUG : fast/dom/Window/window-postmessage-clone-really-deep-array.html = PASS BUGCR24182 SLOW MAC DEBUG : fast/forms/select-set-length-with-mutation-remove.html = PASS BUGCR24182 SLOW : fast/js/regexp-overflow.html = PASS BUGCR24182 SLOW DEBUG : fast/js/toString-and-valueOf-override.html = PASS BUGCR24182 SLOW DEBUG : html5lib/webkit-resumer.html = PASS BUGCR24182 SLOW WIN RELEASE : http/tests/loading/ onload-vs-immediate-refresh.pl = PASS BUGCR79910 SNOWLEOPARD : fast/text/international/bidi-linebreak-001.html = IMAGE BUGCR79910 SNOWLEOPARD : fast/text/international/bidi-linebreak-002.html = IMAGE BUGCR79910 SNOWLEOPARD : fast/text/international/bidi-linebreak-003.html = IMAGE BUGWK58924 MAC : plugins/mouse-click-iframe-to-plugin.html = PASS TIMEOUT BUGWK60099 DEBUG : fast/dom/Attr/access-after-element-destruction.html = PASS CRASH BUGWK60097 DEBUG : fast/dom/HTMLLinkElement/link-and-subresource-test.html = PASS TEXT PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 17, 2012, at 11:28 AM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 11:08 AM, Peter Kasting pkast...@google.com wrote: On Thu, May 17, 2012 at 9:26 AM, Dimitri Glazkov dglaz...@chromium.org wrote: I actually quite like the clear delineation between test modifiers and test expectations. Me too. Please please please leave them on opposite sides. All these proposals that try to put them both before the test name are much harder for me to understand. Even dropping the all-caps format for keywords is a loss for me as this differentiation makes it easier for me to scan the line and distinguish the three important pieces of a line: what, where, and how. I find most of the changes that have been proposed so far to be a significant decrease in clarity, except for the idea to change BUGXX### into links (for all bug trackers, not just non-WebKit ones), which I think would be fine; and the idea of moving SLOW, SKIP, and WONTFIX from the left side to the right, as these seem more like expectations about how the test will run than platforms on which the test fails. Interesting. I find the new format (I've replaced [] by () since that worked better for me) much easier to read than the old format because now all meta data is in one place. For me, I'm most annoyed by the unreasonably long bug URLs in the new format. Maybe putting the URL isn't such a great idea after all :( Perhaps aligning the fields column after the bug URL would improve readability (though it makes things a little harder to edit): crbug.com/24182Slow (Mac Debug) fast/css/large-list-of-rules-crash.html crbug.com/24182Slow (Linux Mac Debug) fast/dom/Window/window-postmessage-clone-really-deep-array.html crbug.com/24182Slow (Mac Debug) fast/forms/select-set-length-with-mutation-remove.html crbug.com/24182Slow (All) fast/js/regexp-overflow.html crbug.com/24182Slow (Debug) fast/js/toString-and-valueOf-override.html crbug.com/24182Slow (Debug) html5lib/webkit-resumer.html crbug.com/24182Slow (Win Release) http/tests/loading/onload-vs-immediate-refresh.pl crbug.com/79910Image (SnowLeopard) fast/text/international/bidi-linebreak-001.html crbug.com/79910Image (SnowLeopard) fast/text/international/bidi-linebreak-002.html crbug.com/79910Image (SnowLeopard) fast/text/international/bidi-linebreak-003.html webkit.org/b/58924 Pass Timeout (Mac) plugins/mouse-click-iframe-to-plugin.html webkit.org/b/60099 Pass Crash (Debug) fast/dom/Attr/access-after-element-destruction.html webkit.org/b/60097 Pass Text (Debug) fast/dom/HTMLLinkElement/link-and-subresource-test.html I would also suggest omitting (All), particularly if it turns out to be the common case. Regards, Maciej crbug.com/24182 Slow (Mac Debug) fast/css/large-list-of-rules-crash.html crbug.com/24182 Slow (Linux Mac Debug) fast/dom/Window/window-postmessage-clone-really-deep-array.html crbug.com/24182 Slow (Mac Debug) fast/forms/select-set-length-with-mutation-remove.html crbug.com/24182 Slow (All) fast/js/regexp-overflow.html crbug.com/24182 Slow (Debug) fast/js/toString-and-valueOf-override.html crbug.com/24182 Slow (Debug) html5lib/webkit-resumer.html crbug.com/24182 Slow (Win Release) http/tests/loading/onload-vs-immediate-refresh.pl crbug.com/79910 Image (SnowLeopard) fast/text/international/bidi-linebreak-001.html crbug.com/79910 Image (SnowLeopard) fast/text/international/bidi-linebreak-002.html crbug.com/79910 Image (SnowLeopard) fast/text/international/bidi-linebreak-003.html webkit.org/b/58924 Pass Timeout (Mac) plugins/mouse-click-iframe-to-plugin.html webkit.org/b/60099 Pass Crash (Debug) fast/dom/Attr/access-after-element-destruction.html webkit.org/b/60097 Pass Text (Debug) fast/dom/HTMLLinkElement/link-and-subresource-test.html BUGCR24182 SLOW MAC DEBUG : fast/css/large-list-of-rules-crash.html = PASS BUGCR24182 SLOW LINUX MAC DEBUG : fast/dom/Window/window-postmessage-clone-really-deep-array.html = PASS BUGCR24182 SLOW MAC DEBUG : fast/forms/select-set-length-with-mutation-remove.html = PASS BUGCR24182 SLOW : fast/js/regexp-overflow.html = PASS BUGCR24182 SLOW DEBUG : fast/js/toString-and-valueOf-override.html = PASS BUGCR24182 SLOW DEBUG : html5lib/webkit-resumer.html = PASS BUGCR24182 SLOW WIN RELEASE : http/tests/loading/onload-vs-immediate-refresh.pl = PASS BUGCR79910 SNOWLEOPARD : fast/text/international/bidi-linebreak-001.html = IMAGE BUGCR79910 SNOWLEOPARD : fast/text/international/bidi-linebreak-002.html = IMAGE BUGCR79910 SNOWLEOPARD : fast/text/international/bidi-linebreak-003.html = IMAGE BUGWK58924 MAC : plugins/mouse-click-iframe-to-plugin.html = PASS TIMEOUT BUGWK60099 DEBUG : fast/dom/Attr/access-after-element-destruction.html = PASS CRASH BUGWK60097 DEBUG :
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 11:54 AM, Maciej Stachowiak m...@apple.com wrote: On May 17, 2012, at 11:28 AM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 11:08 AM, Peter Kasting pkast...@google.comwrote: On Thu, May 17, 2012 at 9:26 AM, Dimitri Glazkov dglaz...@chromium.orgwrote: I actually quite like the clear delineation between test modifiers and test expectations. Me too. Please please please leave them on opposite sides. All these proposals that try to put them both before the test name are much harder for me to understand. Even dropping the all-caps format for keywords is a loss for me as this differentiation makes it easier for me to scan the line and distinguish the three important pieces of a line: what, where, and how. I find most of the changes that have been proposed so far to be a significant decrease in clarity, except for the idea to change BUGXX### into links (for all bug trackers, not just non-WebKit ones), which I think would be fine; and the idea of moving SLOW, SKIP, and WONTFIX from the left side to the right, as these seem more like expectations about how the test will run than platforms on which the test fails. Interesting. I find the new format (I've replaced [] by () since that worked better for me) much easier to read than the old format because now all meta data is in one place. For me, I'm most annoyed by the unreasonably long bug URLs in the new format. Maybe putting the URL isn't such a great idea after all :( Perhaps aligning the fields column after the bug URL would improve readability (though it makes things a little harder to edit): crbug.com/24182Slow (Mac Debug) fast/css/large-list-of-rules-crash.html crbug.com/24182Slow (Linux Mac Debug) fast/dom/Window/window-postmessage-clone-really-deep-array.html crbug.com/24182Slow (Mac Debug) fast/forms/select-set-length-with-mutation-remove.html crbug.com/24182Slow (All) fast/js/regexp-overflow.html crbug.com/24182Slow (Debug) fast/js/toString-and-valueOf-override.html crbug.com/24182Slow (Debug) html5lib/webkit-resumer.html crbug.com/24182Slow (Win Release) http/tests/loading/ onload-vs-immediate-refresh.pl crbug.com/79910Image (SnowLeopard) fast/text/international/bidi-linebreak-001.html crbug.com/79910Image (SnowLeopard) fast/text/international/bidi-linebreak-002.html crbug.com/79910Image (SnowLeopard) fast/text/international/bidi-linebreak-003.html webkit.org/b/58924 Pass Timeout (Mac) plugins/mouse-click-iframe-to-plugin.html webkit.org/b/60099 Pass Crash (Debug) fast/dom/Attr/access-after-element-destruction.html webkit.org/b/60097 Pass Text (Debug) fast/dom/HTMLLinkElement/link-and-subresource-test.html We certainly could. We treat \s+ as \s anyway so we have a lot of freedom here. However, aligning things using whitespace in test_expectations.txt is just as problematic in C++ code because the modifiers and platforms can grow: e.g. Pass Image+Text Image Crash Timeout (SnowLeopard Leopard Linux Win7 Debug) I would also suggest omitting (All), particularly if it turns out to be the common case. Yes, a lot of entries tend to be (All) so we should make it implicit when omitted or not have All at all (I prefer not having it at all). - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 12:19 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 11:54 AM, Maciej Stachowiak m...@apple.com wrote: Perhaps aligning the fields column after the bug URL would improve readability (though it makes things a little harder to edit): We certainly could. We treat \s+ as \s anyway so we have a lot of freedom here. However, aligning things using whitespace in test_expectations.txt is just as problematic in C++ code because the modifiers and platforms can grow: e.g. Pass Image+Text Image Crash Timeout (SnowLeopard Leopard Linux Win7 Debug) Yeah, this is my worry too. None of the options (have ridiculously wide columns, have some rules split across multiple lines, have some rules violate the alignment) seems terribly appealing. If we knew we could restrict the platforms and expectations to a small horizontal space, I think this would work nicely, but to do that we might need to move to cryptic abbreviations :/ PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
There's lot of good discussion going on in this thread ... I'm going
to attempt to reply to various threads in one message, hixie-style :)
On Thu, May 17, 2012 at 1:06 AM, Maciej Stachowiak m...@apple.com wrote:
SKIP and WONTFIX seem parallel to PASS to me.
[and]
On Thu, May 17, 2012 at 11:07 AM, Maciej Stachowiak m...@apple.com wrote:
To me, it seems like all the current modifiers except for the platform
limitations are actually expected outcomes.
SKIP and SLOW are in some ways as much instructions as outcomes.
That said, I'm not sure making this distinction is worth the cost. The
bug URLs and WONTFIX as annotations, not outcomes (although perhaps
distinguishing WONTFIX separately also isn't worth the cost).
On Thu, May 17, 2012 at 9:40 AM, Darin Adler da...@apple.com wrote:
Seriously, syntax is a significant barrier. Having to know which special
characters to use. I don’t see this “clear delineation” you speak of. Just
special punctuation I have to use to satisfy the computer.
Syntax is a double-edged sword, to be certain. There's clearly no need
to keep = and : around just to satisfy the computer, i.e., the
grammar doesn't need them to disambiguate or anything. However, I --
and clearly at least some others -- find tokens mixed together to be
harder to read.
There are other techniques we could use to increase readability.
Maciej suggests column alignment, which is perhaps kinda fragile but
does have some advantages.
On Wed, May 16, 2012 at 10:37 PM, Benjamin Poulain benja...@webkit.org wrote:
I would go for something verbose in a easy to understand format. E.g.
{ test: 'fast/html/keygen.html',
platform: 'mac, linux',
bug: 'webkit.org/b/6',
expectedResults: ['crash', 'text', 'image'],
action: 'skip' }
This is very easy to understand. Unfortunately, it's also verbose as
heck, and, at least in chromium's case, we have hundreds of tests
marked as WONTFIX or SKIP, so this would be very hard to navigate
unless you put each expectation in a single (long) line, at which
point the readability suffers, I think.
On Thu, May 17, 2012 at 10:06 AM, Ryosuke Niwa rn...@webkit.org wrote:
Unfortunately, image+text is different is image text. The former
indicates that the test will have mismatches in both .txt and .png files
while the latter indicates that either .txt or .png mismatches (i.e. the
test is flaky in terms of which failure it causes).
I personally think this is extremely confusing, and we should get rid of
this distinction altogether, and treat Image Text like both image text
and image+text but I'll defer that to a separate discussion.
In a world where failures live in the file for an extended period of
time (i.e., how the chromium port currently operates), I would not be
a fan of this. In a world where failures that persist beyond a short
time are checked in (e.g., as foo-failing.txt or whatever), I can see
the argument that having anything more than a FAIL suffix is unneeded.
I'd say we should rename text, image, and text+image to TextFailure,
ImageFailure, TextAndImageFailures for now to clear the confusion.
Please, no. Failure is basically implied and your suggestion to me
makes things more verbose and harder to read.
Now, as for the more comprehensive comments:
On Wed, May 16, 2012 at 10:47 PM, Ryosuke Niwa rn...@webkit.org wrote:
how about something like
webkit.org/12356 [Win Mac] [Text] failing-test.html
rniwa [Linux] [Skip] temporarily-skipped-test.html
Here, platforms and expectations are wrapped around [] and non-URL comments
are wrapped in . (I think we should just use IRC nick).
Something like this isn't too bad, and ...
On Thu, May 17, 2012 at 4:30 AM, Ojan Vafai o...@chromium.org wrote:
I have a proposal that hopefully addresses everyone's concerns, is minimally
different from the current format *and* unifies the format with Skipped
lists (i.e. Skipped lists as they exist today are valid
test_expectations.txt format).
I like this :)
The changes from the current format are as follows:
-Leaving out any outcomes means the test is skipped, unless the test has a
SLOW modifier, in which case the implied outcome is PASS.
-Remove the SKIP modifier entirely.
ok.
-Make everything but the test name case-insensitive.
I don't think I like this; it could lead to a lot of arbitrarily
different formatting in the file, making things harder to read. I
think we'd probably find ourselves (re-)converging on some convention
pretty quickly. I personally find all uppercase fairly easy to read in
this case since it distinguishes the modifiers from the test name
(like Peter points out). If we have some other clear delimiter this
would probably be less important, in which case all lower case would
be fine as well. Initial caps seems less good to me.
-Have the test name be the last item on the line
this seems reasonable. I have wondered (along with Darin) if it would
make more sense to put the test name first (and then sort the file by
test?), but
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 12:36 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 4:30 AM, Ojan Vafai o...@chromium.org wrote: -Make everything but the test name case-insensitive. I don't think I like this; it could lead to a lot of arbitrarily different formatting in the file, making things harder to read. Modifiers and expectations are already case-insensitive as far as I read the code yesterday. I think we'd probably find ourselves (re-)converging on some convention pretty quickly. I personally find all uppercase fairly easy to read in this case since it distinguishes the modifiers from the test name. I think this problem will disappear once we place modifiers and expectations on the same side because then there is exactly one place those tokens could appear. There is no need to scan through a line then. If we have some other clear delimiter this would probably be less important, in which case all lower case would be fine as well. Initial caps seems less good to me. I find either all-lowercase or all-caps to be much harder to read than capitalized words. They look like a blob of letters to me. Also, I don't think we use all-caps name anywhere else in WebKit so it's inconsistent with the convention we use elsewhere. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 1:06 AM, Maciej Stachowiak m...@apple.com wrote: What does the build configuration info do? Does it apply the line to only those configurations? If that is the case, it does seem potentially different in kind, though maybe also better expressed by being able to combine multiple test_expectations files fro different platform/ directories. I am planning to add support for cascading files to NRWT shortly (it's mostly there now, but I need to add some better diagnostics and reimplement the code to actually use the logic), but I would not suggest eliminating the build configuration in favor of multiple files. First, you can use build configurations to distinguish between debug and release failures, which is very useful. Second, if you are maintaining multiple platforms and actively trying to keep the tree green, I think it is much easier to add one test with multiple configurations to one file than to update the same test in multiple files. I think having multiple files makes sense for when expectations actually cross organizational boundaries, e.g., we have a set of failures that apply to all ports, and a set of failures that are implementation specific (e.g., to all chromium ports, or a subset of the qt ports). I think it makes sense to organize the files to minimize merge conflicts, but at least in chromium's case, we don't typically get merge conflicts from platform-specific failures, we get them because people are adding test-specific failures. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 12:48 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 1:06 AM, Maciej Stachowiak m...@apple.com wrote: What does the build configuration info do? Does it apply the line to only those configurations? If that is the case, it does seem potentially different in kind, though maybe also better expressed by being able to combine multiple test_expectations files fro different platform/ directories. I am planning to add support for cascading files to NRWT shortly (it's mostly there now, but I need to add some better diagnostics and reimplement the code to actually use the logic), but I would not suggest eliminating the build configuration in favor of multiple files. First, you can use build configurations to distinguish between debug and release failures, which is very useful. Second, if you are maintaining multiple platforms and actively trying to keep the tree green, I think it is much easier to add one test with multiple configurations to one file than to update the same test in multiple files. I think having multiple files makes sense for when expectations actually cross organizational boundaries, e.g., we have a set of failures that apply to all ports, and a set of failures that are implementation specific (e.g., to all chromium ports, or a subset of the qt ports). I think it makes sense to organize the files to minimize merge conflicts, but at least in chromium's case, we don't typically get merge conflicts from platform-specific failures, we get them because people are adding test-specific failures. Oh, I should have added that there can be an advantage to having platform/port/configuration-specific files for long-lived entries (the WONTFIX lines), since it seems that those can tend to be configuration-specific and and it makes sense to have all of them in one place. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 12:47 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 12:36 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 4:30 AM, Ojan Vafai o...@chromium.org wrote: -Make everything but the test name case-insensitive. I don't think I like this; it could lead to a lot of arbitrarily different formatting in the file, making things harder to read. Modifiers and expectations are already case-insensitive as far as I read the code yesterday. It may be that it's legal to mix the case, but no one does it. I think we'd probably find ourselves (re-)converging on some convention pretty quickly. I personally find all uppercase fairly easy to read in this case since it distinguishes the modifiers from the test name. I think this problem will disappear once we place modifiers and expectations on the same side because then there is exactly one place those tokens could appear. There is no need to scan through a line then. It's possible. As I said, having some other clear delimiter would help. If we have some other clear delimiter this would probably be less important, in which case all lower case would be fine as well. Initial caps seems less good to me. I find either all-lowercase or all-caps to be much harder to read than capitalized words. They look like a blob of letters to me. We might have to agree to disagree here, then, but that's fine. If there was a clear consensus that one style or another is better, we should go with that. Also, I don't think we use all-caps name anywhere else in WebKit so it's inconsistent with the convention we use elsewhere. I don't think this particularly matters. We should design a format based on what is most useful in this context. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: 2. Make outcomes optional. If they are left out, then the test is skipped (unless the test is marked SLOW, in which case it's expected to pass). There is no SKIP modifier. I don't think we should do this. It seems very subtle. I'd rather be explicit. I'm OK with the rest of your numbered proposals. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 1:37 PM, Peter Kasting pkast...@chromium.orgwrote: On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: 2. Make outcomes optional. If they are left out, then the test is skipped (unless the test is marked SLOW, in which case it's expected to pass). There is no SKIP modifier. I don't think we should do this. It seems very subtle. I'd rather be explicit. I'm OK with the rest of your numbered proposals. I disagree, but I'm fine with punting this to the list of controversial changes that we should discuss separately. FWIW, my main motivation here is that it allows us to unify the Skipped file format with the test_expectations.txt format. But again, we can discuss that separately. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
Seems reasonable as the first step except: On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: 6. Use the same character as a separator before and after the test (:). I counter-propose to drop : or any other delimiter altogether. Delimiters are the biggest pain points in the current syntax as suggested by Benjanmin, Darin, and I. If we're going to have expectations after test name, then there's no need for delimiters other than imposing arbitrary maintenance cost on everyone who edits the file. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:06 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 2:05 PM, Ojan Vafai o...@chromium.org wrote: On Thu, May 17, 2012 at 2:02 PM, Ryosuke Niwa rn...@webkit.org wrote: Seems reasonable as the first step except: On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: 6. Use the same character as a separator before and after the test (:). I counter-propose to drop : or any other delimiter altogether. Delimiters are the biggest pain points in the current syntax as suggested by Benjanmin, Darin, and I. If we're going to have expectations after test name, then there's no need for delimiters other than imposing arbitrary maintenance cost on everyone who edits the file. There is clearly not consensus on this. Getting rid of = as a delimiter seemed uncontroversial to me. Note that I explicitly put getting rid of delimiters in the controversial list. Who opposed this? And why is he or she not replying on this thread? Oh, I supposed I misread Peter's earlier email as being opposed to this. But, I for one am opposed to it. I find the jumble of modifiers in the second set of lines here much harder to quickly parse than the groupings in the first set of lines. webkit.org/b/12345 : foo/bar.html MAC DEBUG : foo/bar.html : SLOW crbug.com/1234 : foo/bar.html : SLOW TEXT IMAGE webkit.org/b/12345 foo/bar.html MAC DEBUG foo/bar.html SLOW crbug.com/1234 foo/bar.html SLOW TEXT IMAGE ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai o...@chromium.org wrote: Oh, I supposed I misread Peter's earlier email as being opposed to this. But, I for one am opposed to it. I find the jumble of modifiers in the second set of lines here much harder to quickly parse than the groupings in the first set of lines. I don't think anyone else has opposed, and this has been complaints from many people who have complained about the current syntax (See Darin Ben's responses earlier in the thread). Given that, I don't see how we can rationalize to keep the delimiters if our goal is to accommodate their needs. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:16 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai o...@chromium.org wrote: Oh, I supposed I misread Peter's earlier email as being opposed to this. But, I for one am opposed to it. I find the jumble of modifiers in the second set of lines here much harder to quickly parse than the groupings in the first set of lines. I don't think anyone else has opposed, and this has been complaints from many people who have complained about the current syntax (See Darin Ben's responses earlier in the thread). Given that, I don't see how we can rationalize to keep the delimiters if our goal is to accommodate their needs. I would like for us to move forward on the subset of things we can all agree on so that we can focus the rest of the bikeshedding on a smaller number of issues. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai o...@chromium.org wrote: Oh, I supposed I misread Peter's earlier email as being opposed to this. You didn't misread me. I have the same opinion as you: this would be a change for the worse. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:19 PM, Ojan Vafai o...@chromium.org wrote: On Thu, May 17, 2012 at 2:16 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai o...@chromium.org wrote: Oh, I supposed I misread Peter's earlier email as being opposed to this. But, I for one am opposed to it. I find the jumble of modifiers in the second set of lines here much harder to quickly parse than the groupings in the first set of lines. I don't think anyone else has opposed, and this has been complaints from many people who have complained about the current syntax (See Darin Ben's responses earlier in the thread). Given that, I don't see how we can rationalize to keep the delimiters if our goal is to accommodate their needs. I would like for us to move forward on the subset of things we can all agree on so that we can focus the rest of the bikeshedding on a smaller number of issues. But then we won't be solving the problem that some people find the current syntax confusing. Replacing BUGCR12345/BUGWK12345 by URLs sound like a good idea. Moving SKIP/WONTFIX after test names also sound like a good idea. However, they aren't things people have complained about. On the other hand, people have, and repeatedly have, complained about delimiters on mailing lists and at the contributors' meeting. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:29 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 2:19 PM, Ojan Vafai o...@chromium.org wrote: On Thu, May 17, 2012 at 2:16 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai o...@chromium.org wrote: Oh, I supposed I misread Peter's earlier email as being opposed to this. But, I for one am opposed to it. I find the jumble of modifiers in the second set of lines here much harder to quickly parse than the groupings in the first set of lines. I don't think anyone else has opposed, and this has been complaints from many people who have complained about the current syntax (See Darin Ben's responses earlier in the thread). Given that, I don't see how we can rationalize to keep the delimiters if our goal is to accommodate their needs. I would like for us to move forward on the subset of things we can all agree on so that we can focus the rest of the bikeshedding on a smaller number of issues. But then we won't be solving the problem that some people find the current syntax confusing. Replacing BUGCR12345/BUGWK12345 by URLs sound like a good idea. Moving SKIP/WONTFIX after test names also sound like a good idea. However, they aren't things people have complained about. On the other hand, people have, and repeatedly have, complained about delimiters on mailing lists and at the contributors' meeting. I'm just trying to make forward progress on the things we agree on. If someone makes a proposal that most people are happy with, then we can do that. But if we can't find such a proposal, then changing syntax to appease some people and make other people upset seems like unnecessary churn (e.g. now every developer who is already accustomed to the current format needs to make sense of the new one). So far, there has yet to be a proposal that doesn't have significant objections that addresses the problems you care about. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:28 PM, Peter Kasting pkast...@chromium.org wrote: On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai o...@chromium.org wrote: Oh, I supposed I misread Peter's earlier email as being opposed to this. You didn't misread me. I have the same opinion as you: this would be a change for the worse. I thought I was also fairly clear that I thought there should be some sort of delimiter. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:34 PM, Ojan Vafai o...@chromium.org wrote: I'm just trying to make forward progress on the things we agree on. Anyway, the said changes (replace bogus BUGCR/BUGWK with URLs) and moving SKIP/WONTFIX to after test names sound like a good change anyways. I have no intention in opposing to make those changes if you or someone else were to work on it on those isolated changes first. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:39 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 2:36 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 2:28 PM, Peter Kasting pkast...@chromium.org wrote: On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai o...@chromium.org wrote: Oh, I supposed I misread Peter's earlier email as being opposed to this. You didn't misread me. I have the same opinion as you: this would be a change for the worse. I thought I was also fairly clear that I thought there should be some sort of delimiter. Could you elaborate on what basis you think we need delimiters? Is it really worth the cost of everyone having to learn to put delimiters and breaking tests every now and then due to someone forgetting to put them in? As I said before, I believe they increase the readability of the file. I believe the cost of learning to put delimiters in is near zero, and as far as breaking the file goes, we have warnings from that six ways from Sunday ... -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 3:01 PM, Dirk Pranke dpra...@chromium.org wrote: As I said before, I believe they increase the readability of the file. I see them as pure noise. I believe the cost of learning to put delimiters in is near zero, That clearly isn't near zero. Or else people wouldn't be complaining about it. To quote Darin's response: Seriously, syntax is a significant barrier. Having to know which special characters to use. I don’t see this “clear delineation” you speak of. Just special punctuation I have to use to satisfy the computer Like I said, I'm also annoyed every time I edit test_expectations.txt because I always forget about them. Also, colon is a terrible delimiter in that we need to press a shift key to type a colon on a standard 104 US keyboard. I would be less anal about it if it it were , or any other symbol that doesn't require me to press a shift key in order to type. The same goes for modifiers and expectations. It's annoying having to either enable caps-lock or press the shift key while typing pass, text, and other modifiers and expectations just to satisfy someone's preference. Here's a thing. There appears to be a conflict of interests between two goals: 1. Being able to read or skim through expectations quickly as possible 2. Being able to write new entries as quickly as possible Perhaps we can address these two problems using some tools. e.g. I don't care about the format of test_expectations.txt at all if there was a GUI tool that lets me add/edit entries easily without ever having to look at raw files. Similarly, people who are advocating to keep the current syntax may not mind changing the format in the repository if there were a way to view the file using the current format for easier readability. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 3:21 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 3:01 PM, Dirk Pranke dpra...@chromium.org wrote: As I said before, I believe they increase the readability of the file. I see them as pure noise. Clearly, different people can have different syntactic preferences :). I believe the cost of learning to put delimiters in is near zero, That clearly isn't near zero. Or else people wouldn't be complaining about it. To quote Darin's response: Seriously, syntax is a significant barrier. Having to know which special characters to use. I don’t see this “clear delineation” you speak of. Just special punctuation I have to use to satisfy the computer You don't have to re-quote this, I already did in this thread (and responded to it). With all due respect to Maciej and Darin, neither of them have spent any significant amount of time working with test_expectations.txt files. While I appreciate that it's nice for the syntax to be approachable for newbies, I'm not inclined to bias in favor of newbies over people who are experienced. Of the people who have actually used the file, so far you're the only person who's spoken up as not liking them. Since different people prefer different things, I'm inclined to go with the majority of experienced users here. I am sorry if that means you lose out; I don't like it if anyone is unhappy and would prefer it if we could please everyone. As I have said before, I'm open to other syntax proposals that don't (IMO) decrease the readability. I've even said I liked some of your other suggestions. Being able to read or skim through expectations quickly as possible Being able to write new entries as quickly as possible As in most other source code, I suspect things are read a lot more than they are written. It's also pretty easy to write macros using your editor of choice, or even to just copypaste the required lines in from the NRWT output. Perhaps we can address these two problems using some tools. e.g. I don't care about the format of test_expectations.txt at all if there was a GUI tool that lets me add/edit entries easily without ever having to look at raw files. Similarly, people who are advocating to keep the current syntax may not mind changing the format in the repository if there were a way to view the file using the current format for easier readability. Sure. Perhaps it would be more useful to spend time working on adding suppression support to garden-o-matic and getting other ports to use that instead of worrying about hand-editing files. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 3:37 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 3:21 PM, Ryosuke Niwa rn...@webkit.org wrote: Perhaps we can address these two problems using some tools. e.g. I don't care about the format of test_expectations.txt at all if there was a GUI tool that lets me add/edit entries easily without ever having to look at raw files. Similarly, people who are advocating to keep the current syntax may not mind changing the format in the repository if there were a way to view the file using the current format for easier readability. Sure. Perhaps it would be more useful to spend time working on adding suppression support to garden-o-matic and getting other ports to use that instead of worrying about hand-editing files. FWIW, adding support for this to garden-o-matic is almost entirely a matter of coming up with the right UI. When you rebaseline expected failures, we already automatically update the test_expectations.txt entries for that test. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 3:37 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 3:21 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 3:01 PM, Dirk Pranke dpra...@chromium.org wrote: As I said before, I believe they increase the readability of the file. I see them as pure noise. Clearly, different people can have different syntactic preferences :). Yup. With all due respect to Maciej and Darin, neither of them have spent any significant amount of time working with test_expectations.txt files. While I appreciate that it's nice for the syntax to be approachable for newbies, I'm not inclined to bias in favor of newbies over people who are experienced. Of the people who have actually used the file, so far you're the only person who's spoken up as not liking them. Since different people prefer different things, I'm inclined to go with the majority of experienced users here. I am sorry if that means you lose out; I don't like it if anyone is unhappy and would prefer it if we could please everyone. It's important for tools to be easy to use. I don't think we should justify the status quo solely on the basis that people have been using it like it. As in most other source code, I suspect things are read a lot more than they are written. It's also pretty easy to write macros using your editor of choice, or even to just copypaste the required lines in from the NRWT output. I agree. But like I said, the current format is hard to read for me. Perhaps we can address these two problems using some tools. e.g. I don't care about the format of test_expectations.txt at all if there was a GUI tool that lets me add/edit entries easily without ever having to look at raw files. Similarly, people who are advocating to keep the current syntax may not mind changing the format in the repository if there were a way to view the file using the current format for easier readability. Sure. Perhaps it would be more useful to spend time working on adding suppression support to garden-o-matic and getting other ports to use that instead of worrying about hand-editing files. Yeah, I'm increasingly convinced that's a better use of our time than bikeshedding on this thread. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 17, 2012, at 12:53 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 12:47 PM, Ryosuke Niwa rn...@webkit.org wrote: I find either all-lowercase or all-caps to be much harder to read than capitalized words. They look like a blob of letters to me. We might have to agree to disagree here, then, but that's fine. If there was a clear consensus that one style or another is better, we should go with that. Which you like better esthetically may be a matter of taste. But it's an objective, scientifically established fact that all-caps text is harder to read than lowercase or mixed case, and reduces reading speed: http://en.wikipedia.org/wiki/All_caps#Readability http://uxmovement.com/content/all-caps-hard-for-users-to-read/ Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 17, 2012, at 3:37 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 3:21 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 3:01 PM, Dirk Pranke dpra...@chromium.org wrote: As I said before, I believe they increase the readability of the file. I see them as pure noise. Clearly, different people can have different syntactic preferences :). I believe the cost of learning to put delimiters in is near zero, That clearly isn't near zero. Or else people wouldn't be complaining about it. To quote Darin's response: Seriously, syntax is a significant barrier. Having to know which special characters to use. I don’t see this “clear delineation” you speak of. Just special punctuation I have to use to satisfy the computer You don't have to re-quote this, I already did in this thread (and responded to it). With all due respect to Maciej and Darin, neither of them have spent any significant amount of time working with test_expectations.txt files. While I appreciate that it's nice for the syntax to be approachable for newbies, I'm not inclined to bias in favor of newbies over people who are experienced. Of the people who have actually used the file, so far you're the only person who's spoken up as not liking them. Since different people prefer different things, I'm inclined to go with the majority of experienced users here. I am sorry if that means you lose out; I don't like it if anyone is unhappy and would prefer it if we could please everyone. If you reject the input of people who are not yet users of test_expectations.txt, you probably won't get new users of text_expectations.txt. That would be bad for the project, so I hope that's not your final answer. More generally, I think understandability (whether for news or experts) should take priority over familiarity. Let's take an example. TEXT next to a test name apparently means that the text fails. There is no way in the world I would guess that just from reading an expectations file. This is only conceivably understandable to someone who is an expert on the format. If someone used TEXT in code to mean fail, I would r- their patch for failure to use meaningful identifiers. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 4:00 PM, Maciej Stachowiak m...@apple.com wrote: On May 17, 2012, at 12:53 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 12:47 PM, Ryosuke Niwa rn...@webkit.org wrote: I find either all-lowercase or all-caps to be much harder to read than capitalized words. They look like a blob of letters to me. We might have to agree to disagree here, then, but that's fine. If there was a clear consensus that one style or another is better, we should go with that. Which you like better esthetically may be a matter of taste. But it's an objective, scientifically established fact that all-caps text is harder to read than lowercase or mixed case, and reduces reading speed: http://en.wikipedia.org/wiki/All_caps#Readability http://uxmovement.com/content/all-caps-hard-for-users-to-read/ Ooo! Citation fight! http://www.whatmakesthemclick.net/2009/12/23/100-things-you-should-know-about-people-19-its-a-myth-that-all-capital-letters-are-inherently-harder-to-read/ http://www.microsoft.com/typography/ctfonts/wordrecognition.aspx :). While I am aware that large block of text are obviously harder to read in all caps, there is research out there that suggests that the problem is just that you're not used to recognizing the particular shapes at hand. In this hand, we're talking about a small set of keywords, and it seems likely that many of us who have grown used to the syntax immediately process IMAGE, TEXT etc. and are not slowed by reading them. The bulk of the text is still lower case (test names) or mixed case (comments) I'm not wedded to the all-caps thing, but, as I have said repeatedly, I believe it is useful in helping to parse the overall shape of the line into distinct chunks.. I am open to accomplish the same end via other means. -- Dirk Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 17, 2012, at 4:17 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 4:00 PM, Maciej Stachowiak m...@apple.com wrote: On May 17, 2012, at 12:53 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 12:47 PM, Ryosuke Niwa rn...@webkit.org wrote: I find either all-lowercase or all-caps to be much harder to read than capitalized words. They look like a blob of letters to me. We might have to agree to disagree here, then, but that's fine. If there was a clear consensus that one style or another is better, we should go with that. Which you like better esthetically may be a matter of taste. But it's an objective, scientifically established fact that all-caps text is harder to read than lowercase or mixed case, and reduces reading speed: http://en.wikipedia.org/wiki/All_caps#Readability http://uxmovement.com/content/all-caps-hard-for-users-to-read/ Ooo! Citation fight! http://www.whatmakesthemclick.net/2009/12/23/100-things-you-should-know-about-people-19-its-a-myth-that-all-capital-letters-are-inherently-harder-to-read/ http://www.microsoft.com/typography/ctfonts/wordrecognition.aspx Your citations do not contradict mine. They dispute the mechanism that makes people read all-caps text slower, not the fact that it happens. Even if it's true that in theory people could be trained to read all-caps text just as quickly, I think it is unwise to make text files that require this uncommon skill. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 17, 2012, at 1:42 PM, Ojan Vafai o...@chromium.org wrote: On Thu, May 17, 2012 at 1:37 PM, Peter Kasting pkast...@chromium.org wrote: On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: 2. Make outcomes optional. If they are left out, then the test is skipped (unless the test is marked SLOW, in which case it's expected to pass). There is no SKIP modifier. I don't think we should do this. It seems very subtle. I'd rather be explicit. I'm OK with the rest of your numbered proposals. I disagree, but I'm fine with punting this to the list of controversial changes that we should discuss separately. FWIW, my main motivation here is that it allows us to unify the Skipped file format with the test_expectations.txt format. But again, we can discuss that separately. Adding SKIP (or whatever) to every line of skipped files is not a big hurdle, I think we could live with that is a transitions tep. I think the bigger hurdle is supporting chaining across multiple directories. - Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 4:16 PM, Maciej Stachowiak m...@apple.com wrote: On May 17, 2012, at 3:37 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 3:21 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 3:01 PM, Dirk Pranke dpra...@chromium.org wrote: As I said before, I believe they increase the readability of the file. I see them as pure noise. Clearly, different people can have different syntactic preferences :). I believe the cost of learning to put delimiters in is near zero, That clearly isn't near zero. Or else people wouldn't be complaining about it. To quote Darin's response: Seriously, syntax is a significant barrier. Having to know which special characters to use. I don’t see this “clear delineation” you speak of. Just special punctuation I have to use to satisfy the computer You don't have to re-quote this, I already did in this thread (and responded to it). With all due respect to Maciej and Darin, neither of them have spent any significant amount of time working with test_expectations.txt files. While I appreciate that it's nice for the syntax to be approachable for newbies, I'm not inclined to bias in favor of newbies over people who are experienced. Of the people who have actually used the file, so far you're the only person who's spoken up as not liking them. Since different people prefer different things, I'm inclined to go with the majority of experienced users here. I am sorry if that means you lose out; I don't like it if anyone is unhappy and would prefer it if we could please everyone. If you reject the input of people who are not yet users of test_expectations.txt, you probably won't get new users of text_expectations.txt. That would be bad for the project, so I hope that's not your final answer. I hope it is clear that I am not rejecting the input of people who are not yet users. But, I also do not think that they should necessarily be given higher priority to people that have been using it heavily for a long time, given that all of us will quickly find ourselves in the latter camp. More generally, I think understandability (whether for news or experts) should take priority over familiarity. Sure, but I am inclined to bias for ease-of-use for experienced users over either of those two requirements. Let's take an example. TEXT next to a test name apparently means that the text fails. There is no way in the world I would guess that just from reading an expectations file. This is only conceivably understandable to someone who is an expert on the format. If someone used TEXT in code to mean fail, I would r- their patch for failure to use meaningful identifiers. I hardly think you have to be an expert on the format. I think you probably need it explained to you once, or you could just read http://trac.webkit.org/wiki/TestExpectations (which is linked to from (I think) all of the expectations files). At the risk of overly repeating myself, I am not wedded to any one format here, but I'm also not inclined to change things just because a couple of people have vocally objected. If there was a clear consensus that any change was preferred, that's fine by me. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 17, 2012, at 4:40 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 4:16 PM, Maciej Stachowiak m...@apple.com wrote: Let's take an example. TEXT next to a test name apparently means that the text fails. There is no way in the world I would guess that just from reading an expectations file. This is only conceivably understandable to someone who is an expert on the format. If someone used TEXT in code to mean fail, I would r- their patch for failure to use meaningful identifiers. I hardly think you have to be an expert on the format. I think you probably need it explained to you once, or you could just read http://trac.webkit.org/wiki/TestExpectations (which is linked to from (I think) all of the expectations files). If someone gave that kind of explanation for a variable in a patch to C++ code, I would still r- their patch. The token's meaning should be apparent without having to read out-of-line docs first. At the risk of overly repeating myself, I am not wedded to any one format here, but I'm also not inclined to change things just because a couple of people have vocally objected. If there was a clear consensus that any change was preferred, that's fine by me. I feel like the people objecting to change, you included, are objecting because mostly because they are already familiar and comfortable with the current format. And not because it is genuinely better than another format that people might be similarly experienced with in the future. It is totally possible to be both newbie-friendly and experienced-friendly if you do not limit experienced to only the people who already have experience today. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 4:50 PM, Maciej Stachowiak m...@apple.com wrote: On May 17, 2012, at 4:40 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 4:16 PM, Maciej Stachowiak m...@apple.com wrote: Let's take an example. TEXT next to a test name apparently means that the text fails. There is no way in the world I would guess that just from reading an expectations file. This is only conceivably understandable to someone who is an expert on the format. If someone used TEXT in code to mean fail, I would r- their patch for failure to use meaningful identifiers. I hardly think you have to be an expert on the format. I think you probably need it explained to you once, or you could just read http://trac.webkit.org/wiki/TestExpectations (which is linked to from (I think) all of the expectations files). If someone gave that kind of explanation for a variable in a patch to C++ code, I would still r- their patch. The token's meaning should be apparent without having to read out-of-line docs first. Fair enough. I am not attempting to suggest that these are the best possible names. I'm just objecting to the claim that you need to be an expert to understand them. At the risk of overly repeating myself, I am not wedded to any one format here, but I'm also not inclined to change things just because a couple of people have vocally objected. If there was a clear consensus that any change was preferred, that's fine by me. I feel like the people objecting to change, you included, are objecting because mostly because they are already familiar and comfortable with the current format. And not because it is genuinely better than another format that people might be similarly experienced with in the future. It is totally possible to be both newbie-friendly and experienced-friendly if you do not limit experienced to only the people who already have experience today. I probably polarized things by saying that your input was less valuable than those of people who were long-time users. I did not mean to offend, and I'm sorry. I certainly didn't mean to imply that I was not listening or not open to feedback from anyone, and I hope I've made that clear. That does not mean I will agree to any proposal without argument, obviously. As Ojan keeps pointing out, pretty much everyone in the objecting to change camp has in fact indicated several things they'd be happy to change, and in fact we've all mostly agreed on them. In addition, I believe the people who have indicated that they like the existing format have given reasons for WHY -- :) -- they like it: the format allows them to quickly and easily chunk the line, and it separates the platform configuration and other modifiers from the expected outcomes, so that it's easier to see each group. The put all the keywords together idea does not have these properties, and that's why we don't like it. Other proposals, like your use of column formatting, and rniwa's use of brackets, are improvements. Your column formatting proposal did have some obvious drawbacks, but I haven't actually seen anyone object too much to rniwa's latest proposal. I personally don't care for the mixed-case names (I'd prefer all upper or all lower), but I would hardly make that a dealbreaker. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
Closing the loop...bug filed https://bugs.webkit.org/show_bug.cgi?id=86796 minus item 2 since that turned out to be controversial. On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: We are arguing about too many orthogonal things at once. It seems like there are a few things we can agree on. Lets make incremental progress on those and after have a more targeted discussion on the controversial issues that are left. We don't need to make this change in one big commit (and shouldn't really). 1. Make SLOW and WONTFIX outcomes instead of modifiers. 2. Make outcomes optional. If they are left out, then the test is skipped (unless the test is marked SLOW, in which case it's expected to pass). There is no SKIP modifier. 3. Change bug modifiers to be URLs. The format for person bugs is bug(ojan). 4. The only things that come before the test are bug modifiers and platform/configuration modifiers (e.g. MAC DEBUG). 5. WONTFIX tests are either skipped or we ignore their output as long as they don't crash (I don't care which one we do). 6. Use the same character as a separator before and after the test (:). 7. Standardize both Skipped and test_expectations.txt on a common comment delimiter (i.e. #). Examples: webkit.org/b/12345 : foo/bar.html # test is skipped on all platforms/configurations this test_expectations.txt file applies to MAC DEBUG : foo/bar.html : SLOW # test is slow on mac debug, but is expected to pass crbug.com/1234 : foo/bar.html : SLOW TEXT IMAGE # test is slow and flaky. sometimes fails text diff, other times fails image diff Talking to people off-thread, there are strong differing opinions on the below controversial issues, so we should discuss separately since getting consensus will be difficult (maybe even on dedicated email threads): -Lowercasing the modifiers/outcomes -Moving all modifiers/outcomes before/after the test name -Making bug modifiers optional for non-wontfix tests -Getting rid of delimiters entirely On Thu, May 17, 2012 at 12:53 PM, Dirk Pranke dpra...@chromium.orgwrote: On Thu, May 17, 2012 at 12:47 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 12:36 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 4:30 AM, Ojan Vafai o...@chromium.org wrote: -Make everything but the test name case-insensitive. I don't think I like this; it could lead to a lot of arbitrarily different formatting in the file, making things harder to read. Modifiers and expectations are already case-insensitive as far as I read the code yesterday. It may be that it's legal to mix the case, but no one does it. I think we'd probably find ourselves (re-)converging on some convention pretty quickly. I personally find all uppercase fairly easy to read in this case since it distinguishes the modifiers from the test name. I think this problem will disappear once we place modifiers and expectations on the same side because then there is exactly one place those tokens could appear. There is no need to scan through a line then. It's possible. As I said, having some other clear delimiter would help. If we have some other clear delimiter this would probably be less important, in which case all lower case would be fine as well. Initial caps seems less good to me. I find either all-lowercase or all-caps to be much harder to read than capitalized words. They look like a blob of letters to me. We might have to agree to disagree here, then, but that's fine. If there was a clear consensus that one style or another is better, we should go with that. Also, I don't think we use all-caps name anywhere else in WebKit so it's inconsistent with the convention we use elsewhere. I don't think this particularly matters. We should design a format based on what is most useful in this context. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
Hi, There has been some complaints / discussions about how syntax in test_expectations.txt is confusing (and I agree with you) on webkit-dev and at contributors' meeting. So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to simplify syntax in test_expectation.txt as follows: - Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit bugs e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345respectively - : and = delimiters are no longer needed - All modifiers and expectations show before test name e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE *Note*: Just listing a test DOES NOT automatically skip a test even after my patch is applied: test.html will result in a warning, and we still expect it to pass. I'll cite Ojan's comment for the rationale: The bigger problem with implying SKIP, is how do you decide when SKIP isn't applied? That needs some hidden set of rules that's too complicated IMO. We can come back to this problem later if we wanted but let us start from the above cleanups first. For those who are interested, we're also going to rename test_expectations.txt to something more WebKitty on https://bugs.webkit.org/show_bug.cgi?id=86690. Best regards, Ryosuke Niwa Software Engineer Google Inc. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE I wonder how this is gonna help making test_expectation.txt less obscure. If anything, I would make the file more verbose. I also think it could be split between regular failure, test expectations that we expect to diverge, and temporary failures that just need a rebaseline. For temporary failure, a system that would avoid conflicts on update/pull would be nice. Cheers, Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
Any concrete proposals? On May 16, 2012 9:23 PM, Benjamin Poulain benja...@webkit.org wrote: On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE I wonder how this is gonna help making test_expectation.txt less obscure. If anything, I would make the file more verbose. I also think it could be split between regular failure, test expectations that we expect to diverge, and temporary failures that just need a rebaseline. For temporary failure, a system that would avoid conflicts on update/pull would be nice. Cheers, Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, There has been some complaints / discussions about how syntax in test_expectations.txt is confusing (and I agree with you) on webkit-dev and at contributors' meeting. So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to simplify syntax in test_expectation.txt as follows: Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit bugs e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345 respectively : and = delimiters are no longer needed All modifiers and expectations show before test name e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE As I've commented in the bug, I'm not a fan of the proposed syntax above :( Note: Just listing a test DOES NOT automatically skip a test even after my patch is applied: test.html will result in a warning, and we still expect it to pass. I'll cite Ojan's comment for the rationale: The bigger problem with implying SKIP, is how do you decide when SKIP isn't applied? That needs some hidden set of rules that's too complicated IMO. As I've also noted in the bug, I don't follow this ... how is this any more complicated than if there's no modifier, default to SKIP? Why is defaulting to PASS any easier? We can come back to this problem later if we wanted but let us start from the above cleanups first. For those who are interested, we're also going to rename test_expectations.txt to something more WebKitty on https://bugs.webkit.org/show_bug.cgi?id=86690. I would have preferred it if you'd phrased this as proposed renaming test_expectations.txt :). It's not clear to me that renaming it to any of the other suggestions thus far are improvements, and there are lots of people (and scripts) used to the existing names. I'm sorry for sounding negative, but a lot of this feels like superficial stylistic tweaks, so I would be inclined to leave things the way they are unless there is a clear consensus that people prefer the new syntax? -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 16, 2012, at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, There has been some complaints / discussions about how syntax in test_expectations.txt is confusing (and I agree with you) on webkit-dev and at contributors' meeting. So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to simplify syntax in test_expectation.txt as follows: Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit bugs e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345 respectively I think I might prefer webkit.org/b/12345 - it's a bit less concise but the meaning is more contextually obvious and you can cut and paste it into a browser address field to follow the link, rather than having to know what site to enter it into. : and = delimiters are no longer needed All modifiers and expectations show before test name e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE Would it be possible to also change the modifiers to lower case or mixed case? All-caps is hard to type and uncomfortable to read. As for where the modifiers go, was there a logical basis for the previous separation into two groups? Would we be losing something by doing that? Cheers, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 9:59 PM, Dirk Pranke dpra...@chromium.org wrote: On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: For those who are interested, we're also going to rename test_expectations.txt to something more WebKitty on https://bugs.webkit.org/show_bug.cgi?id=86690. I would have preferred it if you'd phrased this as proposed renaming test_expectations.txt :). It's not clear to me that renaming it to any of the other suggestions thus far are improvements, and there are lots of people (and scripts) used to the existing names. It needs to be renamed to be consistent with the naming convention we use in LayoutTests and the rest of the WebKit directories (e.g. ChangeLog, Skipped, etc...). Besides files in webkitpy, few tests, and maybe few other files, we don't use underscore delimited file names. I even think that we shouldn't use it in webkitpy either but that's an orthogonal issue. I'm signing up to do the work since I've been always annoyed and nobody has signed up to do it so far. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 9:34 PM, Ryosuke Niwa rn...@webkit.org wrote:
Any concrete proposals?
I would go for something verbose in a easy to understand format. E.g.
{ test: 'fast/html/keygen.html',
platform: 'mac, linux',
bug: 'webkit.org/b/6',
expectedResults: ['crash', 'text', 'image'],
action: 'skip' }
For the tests that need rebaseline or new results, I would leave them
out of the file.
I would use an second file, with entries sorted by bug number.
I don't know all the problems related to test_expectations so that is
just some ideas from what I encountered in the past.
Cheers,
Benjamin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 10:30 PM, Maciej Stachowiak m...@apple.com wrote: On May 16, 2012, at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, There has been some complaints / discussions about how syntax in test_expectations.txt is confusing (and I agree with you) on webkit-dev and at contributors' meeting. So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to simplify syntax in test_expectation.txt as follows: Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit bugs e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345 respectively I think I might prefer webkit.org/b/12345 - it's a bit less concise but the meaning is more contextually obvious and you can cut and paste it into a browser address field to follow the link, rather than having to know what site to enter it into. : and = delimiters are no longer needed All modifiers and expectations show before test name e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE Would it be possible to also change the modifiers to lower case or mixed case? All-caps is hard to type and uncomfortable to read. As for where the modifiers go, was there a logical basis for the previous separation into two groups? Would we be losing something by doing that? There was a semi-logical basis, in that the stuff on the right of the test clearly specified the outcome of the test (PASS, IMAGE, TEXT, etc.). The stuff on the left was less well defined: there's the bug numbers, the platform/configuration info (MAC LINUX RELEASE, etc.), and some other stuff that there is less of a good place for (SLOW, SKIP, WONTFIX). I think it makes sense to syntactically separate at least the platform/configuration keywords from the outcome keywords. It might be nice to separate the other things somehow as well, but I don't have any great ideas for things that are clearly better than the existing left/right convention. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 10:39 PM, Dirk Pranke dpra...@chromium.org wrote: On Wed, May 16, 2012 at 10:30 PM, Maciej Stachowiak m...@apple.com wrote: On May 16, 2012, at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: There has been some complaints / discussions about how syntax in test_expectations.txt is confusing (and I agree with you) on webkit-dev and at contributors' meeting. So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to simplify syntax in test_expectation.txt as follows: Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit bugs e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345 respectively I think I might prefer webkit.org/b/12345 - it's a bit less concise but the meaning is more contextually obvious and you can cut and paste it into a browser address field to follow the link, rather than having to know what site to enter it into. : and = delimiters are no longer needed All modifiers and expectations show before test name e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE Would it be possible to also change the modifiers to lower case or mixed case? All-caps is hard to type and uncomfortable to read. As for where the modifiers go, was there a logical basis for the previous separation into two groups? Would we be losing something by doing that? There was a semi-logical basis, in that the stuff on the right of the test clearly specified the outcome of the test (PASS, IMAGE, TEXT, etc.). The stuff on the left was less well defined: there's the bug numbers, the platform/configuration info (MAC LINUX RELEASE, etc.), and some other stuff that there is less of a good place for (SLOW, SKIP, WONTFIX). What I find most annoying with the current syntax is that modifiers and expectations appear on the opposite ends of a test name. So if I'm copying pasting a test name to add a new entry to test_expectations.txt, then I would have to type something on both ends instead of typing stuff in before or after pasting the test name. Taking Benjamin's idea to make the meaning of tokens explicit and Maciej's (and my original) proposal to use webkit.org/b/12345, how about something like webkit.org/12356 [Win Mac] [Text] failing-test.html rniwa [Linux] [Skip] temporarily-skipped-test.html Here, platforms and expectations are wrapped around [] and non-URL comments are wrapped in . (I think we should just use IRC nick). - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
