Bug 38457 – [Qt] Enable passing Sputnik tests

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

Sputnik test cases landed 4 days before: (~5000 tests)
https://bugs.webkit.org/show_bug.cgi?id=38296

I tested them on Qt bot, they are sufficiently 
stable, most of them pass, only ~50 tests fail.

Runtime of LayoutTests would be increased by ~40 sec on the bot.
(Now the runtime is ~240 sec.)

[Csaba Osztrogonác]

Created attachment 54919 [details]
enable tests

[Alexey Proskuryakov]

Comment on attachment 54919 [details]
enable tests

rs=me

+# Failing Sputnik tests

You could also consider landing platform specific results. Of course, these aren't all the tests that fail, we have many expected cross-platform failures checked in.

[Csaba Osztrogonác]

(In reply to comment #2)
> (From update of attachment 54919 [details])
> rs=me
Thx, landed: http://trac.webkit.org/changeset/58863
 
> You could also consider landing platform specific results. Of course, these
> aren't all the tests that fail, we have many expected cross-platform failures
> checked in.

In general we don't land Qt specific expected results for failures,
we prefer putting these tests into the skipped list and file a bug.

I know disabling a complex test can be worse than check in couple of failures  (eg. FAIL instead of PASS) into the expected file. But I don't see now how can we maintain which expected file is absolutely correct and which consists failures. We will think how can we do it. 

I cc-ed Simon, Tor Arne, Kenneth and András too. Guys, I think we 
should discuss check-in or not check-in failures policy sometime somewhere.

[Andras Becsi]

If I am correct these tests are dumpAsText() tests, so my opinion is that we could check in results for tests which have any PASS hunks in the output. If someone fixes some issues he or she could easily decide if the resulting test failure is due to wrong expected results or some regression.
In general I think this kind of policy would be a step forward, but I am categorically against checking in wrong DRT tree dumps, because to decide afterwards if the dump is correct or not is really hard.
 
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 54919 [details] [details])
> > rs=me
> Thx, landed: http://trac.webkit.org/changeset/58863
> 
> > You could also consider landing platform specific results. Of course, these
> > aren't all the tests that fail, we have many expected cross-platform failures
> > checked in.
> 
> In general we don't land Qt specific expected results for failures,
> we prefer putting these tests into the skipped list and file a bug.
> 
> I know disabling a complex test can be worse than check in couple of failures 
> (eg. FAIL instead of PASS) into the expected file. But I don't see now how can
> we maintain which expected file is absolutely correct and which consists
> failures. We will think how can we do it. 
> 
> I cc-ed Simon, Tor Arne, Kenneth and András too. Guys, I think we 
> should discuss check-in or not check-in failures policy sometime somewhere.

[Tor Arne Vestbø]

(In reply to comment #4)
> If I am correct these tests are dumpAsText() tests, so my opinion is that we
> could check in results for tests which have any PASS hunks in the output. If
> someone fixes some issues he or she could easily decide if the resulting test
> failure is due to wrong expected results or some regression.
> In general I think this kind of policy would be a step forward, but I am
> categorically against checking in wrong DRT tree dumps, because to decide
> afterwards if the dump is correct or not is really hard.

I agree, for dumpAsText it makes sense. For render tree output we prefer to skip tests for now.

[Alexey Proskuryakov]

Sputnik tests are weird - they silently run subtests until the first failure, at which time they bail out and print an error message. Only if there were no (unexpected) errors during the run, PASS is printed.