Bug 82627

Summary: Mac should not use a text_expectations file until it is ready to fully move over to it
Product: WebKit Reporter: Jessie Berlin <jberlin>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dpranke, enrica, eric, jberlin, rniwa, simon.fraser, thorton
Priority: P2 Keywords: NRWT
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 38756    
Bug Blocks:    

Description Jessie Berlin 2012-03-29 10:55:12 PDT
The combination of Skipped / text_expectation files that is being used by NRWT on Mac is

1. Confusing. Should something go on the Skipped list or in the text_expectations file?
2. More places to look to figure out why some test is or isn't running, is or isn't expected to fail, etc.
3. Difficult to deal with when it comes to WK2 and Windows. NRWT doesn't work on Apple Win yet. That means that everything that we want Skipped for WK2 already has to go in the Skipped list.

We should stop using that file until we are ready to switch over to it entirely.
Comment 1 Ryosuke Niwa 2012-03-29 11:02:08 PDT
(In reply to comment #0)
> The combination of Skipped / text_expectation files that is being used by NRWT on Mac is
> 
> 1. Confusing. Should something go on the Skipped list or in the text_expectations file?

test_expectations.txt is useful when some failure happens only in release or debug.

> 2. More places to look to figure out why some test is or isn't running, is or isn't expected to fail, etc.

We should just merge Skipped and test_expectations. test_expectations already support SKIP keyword but it's redundant because we can default to skip tests when there are no modifiers.

> 3. Difficult to deal with when it comes to WK2 and Windows. NRWT doesn't work on Apple Win yet. That means that everything that we want Skipped for WK2 already has to go in the Skipped list.

Maybe we just need WK2 modifier?
Comment 2 Eric Seidel (no email) 2012-03-29 11:03:49 PDT
I certainly have no complaints about removing it.  It was originally there to handle flaky-tests during the NRWT transition.  Those have long since been handled.

It looks like it's mostly been populated by Chromium folks submitting patches who are used to test_expectations handling in Chromium and probably don't even know what a Skipped list is.

Obviously there are several things which you can do in test_expectations which you can't in Skipped, including:
- Continue to run a test, yet expect it to fail (common when you write a patch from windows, but don't have Mac baselines, for example)
- Mark a test as flaky (useful for keeping the bots green)

But the folks who work on the Mac port should set it up however they would like things set up.  Dirk is a good person to ask about how you'd go about disabling it if that is the desire.
Comment 3 Jessie Berlin 2012-03-29 11:11:35 PDT
(In reply to comment #1)
> (In reply to comment #0)
> > The combination of Skipped / text_expectation files that is being used by NRWT on Mac is
> > 
> > 1. Confusing. Should something go on the Skipped list or in the text_expectations file?
> 
> test_expectations.txt is useful when some failure happens only in release or debug.

Which is why it is definitely something we want to move to in the future.

> 
> > 2. More places to look to figure out why some test is or isn't running, is or isn't expected to fail, etc.
> 
> We should just merge Skipped and test_expectations. test_expectations already support SKIP keyword but it's redundant because we can default to skip tests when there are no modifiers.

This definitely should be done in the future in one step (probably as part of this bug), when our concerns about Windows and WK2 are addressed.

> 
> > 3. Difficult to deal with when it comes to WK2 and Windows. NRWT doesn't work on Apple Win yet. That means that everything that we want Skipped for WK2 already has to go in the Skipped list.
> 
> Maybe we just need WK2 modifier?

I don't see how that helps the WK2 on Windows case. Even if we had a WK2 modifier in test_expectations for Mac, we would have to add all tests failing on WK2 to the WK2 Skipped list to deal with Windows.
Comment 4 Ryosuke Niwa 2012-03-29 11:26:15 PDT
(In reply to comment #3)
> (In reply to comment #1)
> > (In reply to comment #0)
> > > 3. Difficult to deal with when it comes to WK2 and Windows. NRWT doesn't work on Apple Win yet. That means that everything that we want Skipped for WK2 already has to go in the Skipped list.
> > 
> > Maybe we just need WK2 modifier?
> 
> I don't see how that helps the WK2 on Windows case. Even if we had a WK2 modifier in test_expectations for Mac, we would have to add all tests failing on WK2 to the WK2 Skipped list to deal with Windows.

In the world where we have merged Skilled and test_expectations.txt, old-run-webkit-tests should also be able to read the same file and skip those files as needed, ignoring lines that it can't comprehend :)
Comment 5 Jessie Berlin 2012-03-29 11:37:32 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #1)
> > > (In reply to comment #0)
> > > > 3. Difficult to deal with when it comes to WK2 and Windows. NRWT doesn't work on Apple Win yet. That means that everything that we want Skipped for WK2 already has to go in the Skipped list.
> > > 
> > > Maybe we just need WK2 modifier?
> > 
> > I don't see how that helps the WK2 on Windows case. Even if we had a WK2 modifier in test_expectations for Mac, we would have to add all tests failing on WK2 to the WK2 Skipped list to deal with Windows.
> 
> In the world where we have merged Skilled and test_expectations.txt, old-run-webkit-tests should also be able to read the same file and skip those files as needed, ignoring lines that it can't comprehend :)

Please correct me if I am wrong, but I don't see anything in old-run-webkit-tests that indicates that it reads the test_expectations.txt files, only the Skipped files.
Comment 6 Jessie Berlin 2012-03-29 11:43:45 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #1)
> > > > (In reply to comment #0)
> > > > > 3. Difficult to deal with when it comes to WK2 and Windows. NRWT doesn't work on Apple Win yet. That means that everything that we want Skipped for WK2 already has to go in the Skipped list.
> > > > 
> > > > Maybe we just need WK2 modifier?
> > > 
> > > I don't see how that helps the WK2 on Windows case. Even if we had a WK2 modifier in test_expectations for Mac, we would have to add all tests failing on WK2 to the WK2 Skipped list to deal with Windows.
> > 
> > In the world where we have merged Skilled and test_expectations.txt, old-run-webkit-tests should also be able to read the same file and skip those files as needed, ignoring lines that it can't comprehend :)
> 
> Please correct me if I am wrong, but I don't see anything in old-run-webkit-tests that indicates that it reads the test_expectations.txt files, only the Skipped files.

And if I misunderstood you and what you meant is that we should add the ability for old-run-webkit-tests to read the test_expectations.txt files, maybe adding that ability is part of getting Mac ready to move over to test_expectations.txt.

Would much rather see NRWT Windows support and cascading test_expectations.txt files :)
Comment 7 Ryosuke Niwa 2012-03-29 11:46:49 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > 
> > In the world where we have merged Skilled and test_expectations.txt, old-run-webkit-tests should also be able to read the same file and skip those files as needed, ignoring lines that it can't comprehend :)
> 
> Please correct me if I am wrong, but I don't see anything in old-run-webkit-tests that indicates that it reads the test_expectations.txt files, only the Skipped files.

So once we merged those two files, presumably, we'll have one file that both old-run-webkit-tests and new-run-webkit-tests would read :)
Comment 8 Dirk Pranke 2012-03-29 12:03:52 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #1)
> > > (In reply to comment #0)
> > > > 3. Difficult to deal with when it comes to WK2 and Windows. NRWT doesn't work on Apple Win yet. That means that everything that we want Skipped for WK2 already has to go in the Skipped list.
> > > 
> > > Maybe we just need WK2 modifier?
> > 
> > I don't see how that helps the WK2 on Windows case. Even if we had a WK2 modifier in test_expectations for Mac, we would have to add all tests failing on WK2 to the WK2 Skipped list to deal with Windows.
> 
> In the world where we have merged Skilled and test_expectations.txt, old-run-webkit-tests should also be able to read the same file and skip those files as needed, ignoring lines that it can't comprehend :)

I would strongly prefer it if we just got rid of old-run-webkit-tests; I don't think modifying it to understand test_expectations.txt, even in a limited sense, is a great idea.
Comment 9 Dirk Pranke 2012-03-29 12:04:59 PDT
in the meantime, we can certainly either just delete the platform/mac/test_expectations.txt file or patch the code to ignore it. I agree that people don't really understand how to use both files and it's confusing.
Comment 10 Dirk Pranke 2012-06-08 16:41:52 PDT
Jessie, should we close this?
Comment 11 Jessie Berlin 2012-06-13 12:19:55 PDT
(In reply to comment #10)
> Jessie, should we close this?

Has anything changed? Are we closer to getting NRWT working on Windows?
Comment 12 Dirk Pranke 2012-06-13 12:30:02 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > Jessie, should we close this?
> 
> Has anything changed? Are we closer to getting NRWT working on Windows?

Not particularly, although I hope to get back to that soon. I'm also planning on changing ORWT to support (a limited form of) the TestExpectations syntax since it seems like ORWT might have a longer lifespan than I had hoped for, and at least that way we could still have a single set of files.

I was mostly asking because it seems like people are still actively using the TestExpectations files and so maybe this bug was a moot point?
Comment 13 Dirk Pranke 2012-11-13 13:00:45 PST
closing this, since we don't have Skipped files any more and ORWT supports TestExpectations files.