Bug 160529 – Remove old Qt database tests

• Component: Tools / Tests
• Status: RESOLVED INVALID

[Jonathan Bedard]

There are a number of failing tests which are remnants of QT support, which is no longer relevant.

[Jonathan Bedard]

Created attachment 285287 [details]
Patch

[Jonathan Bedard]

As per <https://bugs.webkit.org/show_bug.cgi?id=103861> and <https://bugs.webkit.org/show_bug.cgi?id=91778>, the removed tests were failing without this pair of functions in the TestRunner, which we are not planning on implementing.

[Alex Christensen]

Comment on attachment 285287 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285287&action=review

> LayoutTests/platform/mac-wk2/TestExpectations:-273
> -webkit.org/b/147075 http/tests/cache/disk-cache/disk-cache-disable.html [ Pass Failure Timeout ]

This seems unrelated.

> LayoutTests/storage/resources/storage-close-idle-localstorage-databases-immediately.html:-6
> -            window.testRunner.closeIdleLocalStorageDatabases();

This is implemented in DumpRenderTree.  It was added in https://trac.webkit.org/changeset/136323 
Does this test not pass with WebKit1?

[Jonathan Bedard]

Comment on attachment 285287 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285287&action=review

>> LayoutTests/platform/mac-wk2/TestExpectations:-273
>> -webkit.org/b/147075 http/tests/cache/disk-cache/disk-cache-disable.html [ Pass Failure Timeout ]
> 
> This seems unrelated.

This is in connection to r42684 which uncovered the failing tests noted here.  This test is marked as failing but in fact, passes on everything but GTK

>> LayoutTests/storage/resources/storage-close-idle-localstorage-databases-immediately.html:-6
>> -            window.testRunner.closeIdleLocalStorageDatabases();
> 
> This is implemented in DumpRenderTree.  It was added in https://trac.webkit.org/changeset/136323 
> Does this test not pass with WebKit1?

They do pass in WebKit1.  However, the tests were initially added for QT support, so it seems like they are testing irrelevant functionality.  Unless we still intend to support these QT functionalities in WebKit1 and still find it necessary to test for them, it seems appropriate to remove these tests.

[Daniel Bates]

Comment on attachment 285287 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285287&action=review

This patch does not make sense. Both the description in this bug (comment #0) and the description in the ChangeLog do not explain how the functions testRunner.{closeIdleLocalStorageDatabases, setStorageDatabaseIdleInterval}(), used to test functionality of local storage, are related to "QT" (sic).

> LayoutTests/ChangeLog:11
> +        Removed tests and references to a number of old tests designed
> +        to support QT.  The two TestRunner functions involved were:
> +             testRunner.closeIdleLocalStorageDatabases
> +             testRunner.setStorageDatabaseIdleInterval

I think when you write "QT" that you are referring to the Qt WebKit port, but I'm unclear these local storage functions relate to that port. As Alex remarked in test LayoutTests/storage/resources/storage-close-idle-localstorage-databases-immediately.html, the function testRunner.closeIdleLocalStorageDatabases() was added in <https://trac.webkit.org/changeset/136323> and only implemented for Mac. The function testRunner.setStorageDatabaseIdleInterval() was added in <https://trac.webkit.org/changeset/122174> and only implemented for Mac.

>>> LayoutTests/platform/mac-wk2/TestExpectations:-273
>>> -webkit.org/b/147075 http/tests/cache/disk-cache/disk-cache-disable.html [ Pass Failure Timeout ]
>> 
>> This seems unrelated.
> 
> This is in connection to r42684 which uncovered the failing tests noted here.  This test is marked as failing but in fact, passes on everything but GTK

Alex is referring to the fact that the change to this line is unrelated to the purpose of this bug to "remove old [QT] database tests" because the test http/tests/cache/disk-cache/disk-cache-disable.html is a disk cache test. That is, it is not a database test. The WebKit OpenSource Project prefers the approach of one fix per bug/patch. So, we should keep this patch focused on removing the Qt database tests and create a new bug/patch to remove the test http/tests/cache/disk-cache/disk-cache-disable.html.

On another note, I'm unclear how r42684 <http://trac.webkit.org/changeset/42684>, which is build fix that was made 7 years ago, relates to this test or any of the other tests removed in this patch.

[Jonathan Bedard]

The blame history for this is a bit of a mess, it is still unclear to me what these tests are supposed to be testing, and it also seems as if the final bug is no longer attached to it's patch in Bugzilla, and the Radars it references don't seem to exist either.

From what I can gather, here are the basics:

https://bugs.webkit.org/show_bug.cgi?id=90713 : Initial proposal of the function
https://bugs.webkit.org/show_bug.cgi?id=103469 : Added the test which uses this funcitonality
https://bugs.webkit.org/show_bug.cgi?id=103876 : Skipped storage/domstorage/localstorage/close-idle-localstorage-databases-immediately.html.

It was this last Bugzilla that connected these tests to the Qt port.  Looking through this more, it seems like actually, this has the opposite problem: Webkit2 needs to implement closeIdleLocalStorageDatabases.

[Jonathan Bedard]

Bugs <https://bugs.webkit.org/show_bug.cgi?id=103861> and <https://bugs.webkit.org/show_bug.cgi?id=91778> remain valid.  Marking resolved as invalid.