Bug 194318

Summary: Web Inspector: Styles: PropertiesChanged shouldn't fire when old and new text are both empty
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, drousso, ews-watchlist, inspector-bugzilla-changes, rniwa, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
drousso: review+, drousso: commit-queue-
Archive of layout-test-results from ews101 for mac-highsierra
none
Patch none

Description Nikita Vasilyev 2019-02-05 16:42:46 PST
inspector/css/modify-css-property-race.html is still failing on debug build (and debug build only!) because of this.
https://webkit-queues.webkit.org/results/11045743

Also, it adds an unnecessary entropy to the universe :)
Comment 1 Nikita Vasilyev 2019-02-05 16:49:48 PST
Created attachment 361249 [details]
Patch
Comment 2 Devin Rousso 2019-02-05 17:05:36 PST
Comment on attachment 361249 [details]
Patch

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

rs=me

> LayoutTests/inspector/css/modify-css-property-race.html:14
> +        let height = parseInt(element.style.height) + 1;

oops :P

> LayoutTests/inspector/css/modify-css-property-race.html:72
> +                    InspectorTest.expectGreaterThan(heightNumber, 11, "Height should be greater than 11px.");

Rather than compare the hardcoded 10 and 11, you could save a `previousHeightNumber` and ensure that the first one is greater than 10 and the second one is greater than `previousHeightNumber` (and change the text to say "Height should have increased from 10px" so you don't have to log the actual increased value).

> LayoutTests/inspector/css/modify-css-property-race.html:77
> +                    InspectorTest.expectGreaterThanOrEqual(heightNumber, 101, "Height should be greater or equal 101px.");

Ditto (72).
Comment 3 EWS Watchlist 2019-02-05 18:09:40 PST
Comment on attachment 361249 [details]
Patch

Attachment 361249 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11046952

New failing tests:
inspector/css/modify-css-property.html
Comment 4 EWS Watchlist 2019-02-05 18:09:41 PST
Created attachment 361260 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 5 Nikita Vasilyev 2019-02-05 18:19:20 PST
This broke other tests :(
Fix incoming.
Comment 6 Nikita Vasilyev 2019-02-05 18:32:16 PST
Created attachment 361262 [details]
Patch
Comment 7 Nikita Vasilyev 2019-02-05 18:43:48 PST
(In reply to Devin Rousso from comment #2)
> > LayoutTests/inspector/css/modify-css-property-race.html:72
> > +                    InspectorTest.expectGreaterThan(heightNumber, 11, "Height should be greater than 11px.");
> 
> Rather than compare the hardcoded 10 and 11, you could save a
> `previousHeightNumber` and ensure that the first one is greater than 10 and
> the second one is greater than `previousHeightNumber` (and change the text
> to say "Height should have increased from 10px" so you don't have to log the
> actual increased value).

I actually find it easier to follow when it's hardcoded there. However, I do like the less "robotic" assert messages!
Comment 8 WebKit Commit Bot 2019-02-05 22:54:48 PST
Comment on attachment 361262 [details]
Patch

Clearing flags on attachment: 361262

Committed r241011: <https://trac.webkit.org/changeset/241011>
Comment 9 WebKit Commit Bot 2019-02-05 22:54:49 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2019-02-05 22:55:30 PST
<rdar://problem/47842941>
Comment 11 Truitt Savell 2019-02-07 09:10:41 PST
After the fixes in https://trac.webkit.org/changeset/241011/webkit

inspector/css/modify-css-property-race.html

has become a flakey failure on Debug. Looks like it it somehow writing another pass message? 

History:
http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Fcss%2Fmodify-css-property-race.html

Diff:
--- /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/inspector/css/modify-css-property-race-expected.txt
+++ /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/inspector/css/modify-css-property-race-actual.txt
@@ -7,4 +7,5 @@
 PASS: Height should have increased from 11px.
 PASS: Height should be 100px or more.
 PASS: Height should be 101px or more.
+PASS: Height should be 101px or more.