Re: [webkit-dev] Merging Skipped and test_expectations.txt formats WAS: Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-18 Thread Dirk Pranke
On Thu, May 17, 2012 at 11:03 PM, Ojan Vafai  wrote:
> On Thu, May 17, 2012 at 10:37 PM, Maciej Stachowiak  wrote:
>>
>>
>> On May 17, 2012, at 7:27 PM, Ojan Vafai  wrote:
>>
>> On Thu, May 17, 2012 at 4:29 PM, Maciej Stachowiak  wrote:
>>>
>>>
>>> On May 17, 2012, at 1:42 PM, Ojan Vafai  wrote:
>>>
>>> On Thu, May 17, 2012 at 1:37 PM, Peter Kasting 
>>> wrote:

 On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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.
>>
>>
>> That's great. I don't think anyone is opposed to adding chaining and I
>> think that's on Dirk short-list of todos.
>>
>> The only potentially tricky thing here is figuring out what the platform
>> modifiers mean for non-Chromium ports, e.g. I imagine Qt will want similar
>> modifiers to Chromium (mac, linux, win, debug, release, etc). But I think
>> the difficulty here is more in getting the python code right than agreeing
>> on what the correct behavior is.
>>
>>
>> I think it would be good if platform modifiers in the expectations file
>> matched the platform names we use under the platform/ directory, either
>> literally or as a suffix. So for example "mac" in the chromium expectations
>> file could mean chromium-mac, in the qt expectations file it could mean
>> qt-mac, in the mac expectations file it should not be used, but snowleopard
>> would mean mac-snowleopard.
>
>
> That seems like a good way to organize it to me. Dirk, that make sense to
> you?
>

If I understand this properly, I think this is fine.

-- Dirk
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Merging Skipped and test_expectations.txt formats WAS: Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-17 Thread Ojan Vafai
On Thu, May 17, 2012 at 10:37 PM, Maciej Stachowiak  wrote:

>
> On May 17, 2012, at 7:27 PM, Ojan Vafai  wrote:
>
> On Thu, May 17, 2012 at 4:29 PM, Maciej Stachowiak  wrote:
>
>>
>> On May 17, 2012, at 1:42 PM, Ojan Vafai  wrote:
>>
>> On Thu, May 17, 2012 at 1:37 PM, Peter Kasting wrote:
>>
>>> On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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.
>>
>
> That's great. I don't think anyone is opposed to adding chaining and I
> think that's on Dirk short-list of todos.
>
> The only potentially tricky thing here is figuring out what the platform
> modifiers mean for non-Chromium ports, e.g. I imagine Qt will want similar
> modifiers to Chromium (mac, linux, win, debug, release, etc). But I think
> the difficulty here is more in getting the python code right than agreeing
> on what the correct behavior is.
>
>
> I think it would be good if platform modifiers in the expectations file
> matched the platform names we use under the platform/ directory, either
> literally or as a suffix. So for example "mac" in the chromium expectations
> file could mean chromium-mac, in the qt expectations file it could mean
> qt-mac, in the mac expectations file it should not be used, but snowleopard
> would mean mac-snowleopard.
>

That seems like a good way to organize it to me. Dirk, that make sense to
you?

Implementation detail: this might be nice for automating the generation of
the macros used for determining what the generic ones map to (e.g. the mac
maps to snowleopard, leopard, mt lion for chromium). It would be a nice
secondary benefit to get rid of the current hard-coded lists. Only thing
we'd need to hard code is what platform the generic directory maps to (e.g.
that chromium-mac is actually the chromium mt lion directory).

> Also, currently the test_expectations.txt format requires either a bug
> number or a bug(ojan) entry. Would that be OK with you too? It has proven
> really good historically for keeping track of why a test was added to the
> file and for keeping track of getting the tests fixed (or, more
> importantly, having someone responsible for following up on it), but we
> could easily restrict this requirement to the Chromium expectations file if
> other ports dislike it.
>
> Requiring a bug seems good. I don't personally see the need for any
> exception to having a filed and tracked bug but perhaps folks closer to the
> problem know of a reason.
>

Great. That's simpler.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Merging Skipped and test_expectations.txt formats WAS: Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-17 Thread Maciej Stachowiak

On May 17, 2012, at 7:27 PM, Ojan Vafai  wrote:

> On Thu, May 17, 2012 at 4:29 PM, Maciej Stachowiak  wrote:
> 
> On May 17, 2012, at 1:42 PM, Ojan Vafai  wrote:
> 
>> On Thu, May 17, 2012 at 1:37 PM, Peter Kasting  wrote:
>> On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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.
> 
> That's great. I don't think anyone is opposed to adding chaining and I think 
> that's on Dirk short-list of todos.
> 
> The only potentially tricky thing here is figuring out what the platform 
> modifiers mean for non-Chromium ports, e.g. I imagine Qt will want similar 
> modifiers to Chromium (mac, linux, win, debug, release, etc). But I think the 
> difficulty here is more in getting the python code right than agreeing on 
> what the correct behavior is.

I think it would be good if platform modifiers in the expectations file matched 
the platform names we use under the platform/ directory, either literally or as 
a suffix. So for example "mac" in the chromium expectations file could mean 
chromium-mac, in the qt expectations file it could mean qt-mac, in the mac 
expectations file it should not be used, but snowleopard would mean 
mac-snowleopard.

> 
> Also, currently the test_expectations.txt format requires either a bug number 
> or a bug(ojan) entry. Would that be OK with you too? It has proven really 
> good historically for keeping track of why a test was added to the file and 
> for keeping track of getting the tests fixed (or, more importantly, having 
> someone responsible for following up on it), but we could easily restrict 
> this requirement to the Chromium expectations file if other ports dislike it.

Requiring a bug seems good. I don't personally see the need for any exception 
to having a filed and tracked bug but perhaps folks closer to the problem know 
of a reason.

> 
> I think with those three things and 
> https://bugs.webkit.org/show_bug.cgi?id=86796 addressed, then the formats 
> will be unified and the only thing to bikeshed over is the filename. :)

And maybe more follow-up discussion about the syntax.

Regards,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Merging Skipped and test_expectations.txt formats WAS: Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-17 Thread Dirk Pranke
On Thu, May 17, 2012 at 7:27 PM, Ojan Vafai  wrote:
> On Thu, May 17, 2012 at 4:29 PM, Maciej Stachowiak  wrote:
>>
>>
>> On May 17, 2012, at 1:42 PM, Ojan Vafai  wrote:
>>
>> On Thu, May 17, 2012 at 1:37 PM, Peter Kasting 
>> wrote:
>>>
>>> On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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.
>
>
> That's great. I don't think anyone is opposed to adding chaining and I think
> that's on Dirk short-list of todos.
>
> The only potentially tricky thing here is figuring out what the platform
> modifiers mean for non-Chromium ports, e.g. I imagine Qt will want similar
> modifiers to Chromium (mac, linux, win, debug, release, etc). But I think
> the difficulty here is more in getting the python code right than agreeing
> on what the correct behavior is.
>

Yes, I am implementing cascading expectations files as soon as this
thread dies :).

As far as sharing modifiers goes, I am of two minds. One: switch to
one big list of modifiers that everyone shares, and add support for
"implementation" as a dimension of modifier alongside operating system
version and debug/release. Two: allow for per-implementation specific
sets of modifiers but require all files to share the same sets.
Practically speaking, the latter would mean that if multiple
implementations shared a single file, then that file would only be
allowed to have lines without any modifier, or modifiers that all
implementations implemented.

This is perhaps better illustrated by two use cases. The first: we
should be able to mark a set of tests that will fail across all ports
while we're waiting for new baselines for a test. The second: it would
be nice to put expectaions where everyone that meets a specific
criteria will fail in one place. Obvious examples: WebKit2-specific
bugs (or WebKit1-specific bugs), and DEBUG assertions.

The former can be met by having a top-level expectations file that
contains lines with no modifiers at all. The latter can be met by
having a lines in that same file with DEBUG or WEBKIT2 as long as
everyone who imports that file recognizes those modifiers. The latter
can also be met by having platform/wk2/test_expectations.txt with no
modifiers, which is okay but not great. Creating a separate
platform/debug/test_expectations.txt (or something) feel more awkward,
and this clearly doesn't scale all that well.

I think there is a point in filing bugs even for WONTFIX features,
just so there is a record of the rationale for why it's WONTFIX and
that rationale doesn't clutter up the expectations file.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Merging Skipped and test_expectations.txt formats WAS: Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-17 Thread Ryosuke Niwa
On Thu, May 17, 2012 at 7:27 PM, Ojan Vafai  wrote:

> On Thu, May 17, 2012 at 4:29 PM, Maciej Stachowiak  wrote:
>>
>> On May 17, 2012, at 1:42 PM, Ojan Vafai  wrote:
>>
>> On Thu, May 17, 2012 at 1:37 PM, Peter Kasting wrote:
>>
>>> On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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.
>>
>
> That's great. I don't think anyone is opposed to adding chaining and I
> think that's on Dirk short-list of todos.
>
> The only potentially tricky thing here is figuring out what the platform
> modifiers mean for non-Chromium ports, e.g. I imagine Qt will want similar
> modifiers to Chromium (mac, linux, win, debug, release, etc). But I think
> the difficulty here is more in getting the python code right than agreeing
> on what the correct behavior is.
>
> Also, currently the test_expectations.txt format requires either a bug
> number or a bug(ojan) entry. Would that be OK with you too? It has proven
> really good historically for keeping track of why a test was added to the
> file and for keeping track of getting the tests fixed (or, more
> importantly, having someone responsible for following up on it), but we
> could easily restrict this requirement to the Chromium expectations file if
> other ports dislike it.
>

Most of entries in Skipped files have a bug URL associated with them so
this is probably not a problem.

I can see that we probably want to omit it for WontFix or NotImplemented
since there isn't really a point in filing a bug for WontFix features but
we already do that so there should be no blocker.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


[webkit-dev] Merging Skipped and test_expectations.txt formats WAS: Simplifying syntax in test_expectations.txt (bug 86691)

2012-05-17 Thread Ojan Vafai
On Thu, May 17, 2012 at 4:29 PM, Maciej Stachowiak  wrote:

>
> On May 17, 2012, at 1:42 PM, Ojan Vafai  wrote:
>
> On Thu, May 17, 2012 at 1:37 PM, Peter Kasting wrote:
>
>> On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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.
>

That's great. I don't think anyone is opposed to adding chaining and I
think that's on Dirk short-list of todos.

The only potentially tricky thing here is figuring out what the platform
modifiers mean for non-Chromium ports, e.g. I imagine Qt will want similar
modifiers to Chromium (mac, linux, win, debug, release, etc). But I think
the difficulty here is more in getting the python code right than agreeing
on what the correct behavior is.

Also, currently the test_expectations.txt format requires either a bug
number or a bug(ojan) entry. Would that be OK with you too? It has proven
really good historically for keeping track of why a test was added to the
file and for keeping track of getting the tests fixed (or, more
importantly, having someone responsible for following up on it), but we
could easily restrict this requirement to the Chromium expectations file if
other ports dislike it.

I think with those three things and
https://bugs.webkit.org/show_bug.cgi?id=86796 addressed, then the formats
will be unified and the only thing to bikeshed over is the filename. :)

Ojan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev