Bug 163826 - REGRESSION(r207238): [GTK] Layout test storage/domstorage/events/basic-body-attribute.html is failing
Summary: REGRESSION(r207238): [GTK] Layout test storage/domstorage/events/basic-body-a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ms2ger (he/him; ⌚ UTC+1/+2)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-21 17:59 PDT by Michael Catanzaro
Modified: 2020-11-04 06:40 PST (History)
10 users (show)

See Also:


Attachments
Patch (3.28 KB, patch)
2017-12-20 05:56 PST, Ms2ger (he/him; ⌚ UTC+1/+2)
no flags Details | Formatted Diff | Diff
Patch (4.86 KB, patch)
2017-12-21 06:11 PST, Ms2ger (he/him; ⌚ UTC+1/+2)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2016-10-21 17:59:02 PDT
Layout test storage/domstorage/events/basic-body-attribute.html has been failing since r207238-r207243 (inclusive):

--- /home/slave/webkitgtk/gtk-linux-64-release-tests/build/layout-test-results/storage/domstorage/events/basic-body-attribute-expected.txt
+++ /home/slave/webkitgtk/gtk-linux-64-release-tests/build/layout-test-results/storage/domstorage/events/basic-body-attribute-actual.txt
@@ -44,7 +44,7 @@
 PASS storageEventList[6].newValue is null
 storage.clear()
 PASS storageEventList.length is 8
-PASS storageEventList[7].key is ""
+FAIL storageEventList[7].key should be  (of type string). Was null (of type object).
 PASS storageEventList[7].oldValue is null
 PASS storageEventList[7].newValue is null
 
@@ -90,7 +90,7 @@
 PASS storageEventList[6].newValue is null
 storage.clear()
 PASS storageEventList.length is 8
-PASS storageEventList[7].key is ""
+FAIL storageEventList[7].key should be  (of type string). Was null (of type object).
 PASS storageEventList[7].oldValue is null
 PASS storageEventList[7].newValue is null

Updating expectations accordingly.
Comment 1 Yusuke Suzuki 2016-10-21 18:11:24 PDT
@mcatanzaro,

Hm, this test is flaky before that patch.
It seems that this test is not related to nodeType, right?
https://bugs.webkit.org/show_bug.cgi?id=148435

I'll look into it tonight.
Comment 2 Michael Catanzaro 2016-10-21 18:17:24 PDT
(In reply to comment #1)
> @mcatanzaro,
> 
> Hm, this test is flaky before that patch.

No, from quick visual inspection it looks like <10 failures in the past 30,000 commits. It's been failing consistently since r207243.

> It seems that this test is not related to nodeType, right?
> https://bugs.webkit.org/show_bug.cgi?id=148435

I really don't know, maybe your change is totally unrelated! I had trouble guessing which commit might be to blame. Yours looked most likely since I saw the words "DOM" and "type." :-)
Comment 3 Michael Catanzaro 2016-10-21 18:22:05 PDT
(In reply to comment #2)
> I really don't know, maybe your change is totally unrelated! I had trouble
> guessing which commit might be to blame.

Well looks like I guessed wrong, it was surely r207238 "Update more events to stop using legacy [ConstructorTemplate=Event]". Chris, do the expectations need to be updated, or is it a bug with your change? I guess the type change might be expected, since the value can be null now?
Comment 4 Chris Dumez 2016-10-22 10:13:57 PDT
r207238 made StorageEvent.key nullable, as per the spec.

This particular test just needs to be updated, this is expected.
Comment 5 Michael Catanzaro 2016-10-22 10:41:02 PDT
I wonder why it's not failing on Mac.
Comment 6 Chris Dumez 2016-10-22 10:42:26 PDT
(In reply to comment #5)
> I wonder why it's not failing on Mac.

LayoutTests/platform/mac/TestExpectations:webkit.org/b/148435 storage/domstorage/events/basic-body-attribute.html [ Pass Failure ]
Comment 7 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-12-20 05:56:32 PST
Created attachment 329913 [details]
Patch
Comment 8 Daniel Bates 2017-12-20 12:58:50 PST
Comment on attachment 329913 [details]
Patch

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

We are currently skipping this test on Mac (bug #148435), Windows (bug #160447), iOS (bug #155201) and WPE. All of these bugs have been open for a year or more. For your consideration I suggest that we try to unskip the test on all of these ports instead of just unskipping it on GTK.

> LayoutTests/storage/domstorage/events/basic-body-attribute.html:98
> +    shouldBeNull("storageEventList[7].key", "");

The second argument is not necessary.
Comment 9 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-12-21 06:11:24 PST
Created attachment 330029 [details]
Patch
Comment 10 WebKit Commit Bot 2017-12-21 06:44:51 PST
Comment on attachment 330029 [details]
Patch

Clearing flags on attachment: 330029

Committed r226219: <https://trac.webkit.org/changeset/226219>
Comment 11 WebKit Commit Bot 2017-12-21 06:44:52 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Nikolas Zimmermann 2019-11-27 04:40:08 PST
This is still flaky according to https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/11947/steps/layout-test/logs/stdio. Will reopen this ticket and adapt the test expectations.
Comment 13 Diego Pino 2020-11-04 06:40:16 PST
This test(s) has been consistenly passing in the last 4000 revisions. Closing bug.