Bug 98260 – [NRWT] --skipped option is ignored when --test-list is used

• Component: New Bugs
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

Now NRWT runs all tests listed in test-list file, --skipped option is 
absolutely ignored. In my opinion it should respect --skipped option too.

This bug is revealed with a real life usecase:
- After a fix I run all tests on WK1: run-webkit-tests
- There are 600+ failing tests. I checked one by one, only rebase needed. (list-of-rebased-tests.txt)
- I did the rebase to platform/qt
- double check for WK2 (maybe some of them need qt-5.0-wk2 specific result):
  run-webkit-tests --test-list=./list-of-rebased-tests.txt --qt -2
--> But unfortunately NRWT runs skipped tests (wk2, qt-5.0-wk2) too, but it shouldn't

Any idea how to fix this annoying bug?

[Alexey Proskuryakov]

FWIW, it's intentional that tests listed on command line are run regardless of being skipped. Test-list file is similar conceptually, although use cases are clearly different.

[Dirk Pranke]

Alexey is basically right on the implementation and original intent, but that doesn't mean we shouldn't make your case work.

To make sure that I understand what you're seeing / asking for, you have a list of tests in a file, and some of those tests are listed as [ Skip ] in TestExpectations, but when you run NRWT, they get executed, right? 

So, you'd actually like the expectations to be honored and the files skipped if they do appear in the TestExpectations file (as skips)?

If so, I actually ran into this exact same problem a couple days ago, and the patch I made (but haven't posted yet) is a change to add --skipped=expectations which means "skip the tests listed in the expectations file even if they were listed on in a text file or on the command line". It's a ~3 line change :).

Would that work for you?

[Csaba Osztrogonác]

Ah, I can understand the intention of the implemetation too. My use case is
that I don't want to run all tests, but a small subset of tests. But it would
be great if somehow TestExpectations would honored.

Now the following 2 commands are same: (--test-list implicate --skipped=ignore)
- run-webkit-tests --test-list=./list.txt
- run-webkit-tests --test-list=./list.txt --skipped=ignore

What if we can override it with explicit --skipped=default option?
- run-webkit-tests --test-list=./list.txt --skipped=default

Additionally it would we great if we add a comment to the help of 
--test-list option to make it clear it implicates --skipped=ignore.

How does it sound?

[Dirk Pranke]

(In reply to comment #3)
> Ah, I can understand the intention of the implemetation too. My use case is
> that I don't want to run all tests, but a small subset of tests. But it would
> be great if somehow TestExpectations would honored.
> 
> Now the following 2 commands are same: (--test-list implicate --skipped=ignore)
> - run-webkit-tests --test-list=./list.txt
> - run-webkit-tests --test-list=./list.txt --skipped=ignore
> 
> What if we can override it with explicit --skipped=default option?
> - run-webkit-tests --test-list=./list.txt --skipped=default
> 
> Additionally it would we great if we add a comment to the help of 
> --test-list option to make it clear it implicates --skipped=ignore.
> 
> How does it sound?

That's almost exactly what my patch does. I'll post it shortly.

-- Dirk

[Dirk Pranke]

Created attachment 167026 [details]
Patch

[Balazs Kelemen]

It's nice that you fixed it, I was also struggling with that.

[Csaba Osztrogonác]

Comment on attachment 167026 [details]
Patch

Thanks for the quick fix.

[WebKit Review Bot]

Comment on attachment 167026 [details]
Patch

Clearing flags on attachment: 167026

Committed r130381: <http://trac.webkit.org/changeset/130381>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.