Title: [251485] trunk
Revision
251485
Author
yu...@chromium.org
Date
2019-10-23 12:09:04 -0700 (Wed, 23 Oct 2019)

Log Message

Web Inspector: frontend tests should clear output before resending results
https://bugs.webkit.org/show_bug.cgi?id=203262

Reviewed by Devin Rousso.

Source/WebInspectorUI:

Inspector front-end tests will clear output log before resending teset results. This avoids
race between InspectorTest.testPageDidLoad event and TestPage.addResult calls that may have
already be sent to the new page after navigation. The latter events otherwise would be added
twice.

* UserInterface/Test/FrontendTestHarness.js:
(FrontendTestHarness):
(FrontendTestHarness.prototype.testPageDidLoad):
(FrontendTestHarness.prototype.reloadPage):
(FrontendTestHarness.prototype.reportUnhandledRejection):
(FrontendTestHarness.prototype.reportUncaughtException):
(FrontendTestHarness.prototype._resendResults): Don't resend the results when the page is loaded
first time.

LayoutTests:

Unflake some of the tests that reload inspected page. This is achieved by waiting for
explicit TestPageDidLoad event. At that point it's known that accumulated so far test
output has been resent to the inspected page and the log lines will not change their
order / appear twice.

* http/tests/inspector/resources/inspector-test.js:
(TestPage.clearOutput):
* inspector/debugger/breakpoint-action-eval.html:
* inspector/debugger/breakpoint-action-log-expected.txt:
* inspector/debugger/breakpoint-action-log.html:
* inspector/debugger/probe-manager-add-remove-actions-expected.txt:
* inspector/debugger/probe-manager-add-remove-actions.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (251484 => 251485)


--- trunk/LayoutTests/ChangeLog	2019-10-23 18:49:37 UTC (rev 251484)
+++ trunk/LayoutTests/ChangeLog	2019-10-23 19:09:04 UTC (rev 251485)
@@ -1,3 +1,23 @@
+2019-10-23  Yury Semikhatsky  <yu...@chromium.org>
+
+        Web Inspector: frontend tests should clear output before resending results
+        https://bugs.webkit.org/show_bug.cgi?id=203262
+
+        Reviewed by Devin Rousso.
+
+        Unflake some of the tests that reload inspected page. This is achieved by waiting for
+        explicit TestPageDidLoad event. At that point it's known that accumulated so far test
+        output has been resent to the inspected page and the log lines will not change their
+        order / appear twice.
+
+        * http/tests/inspector/resources/inspector-test.js:
+        (TestPage.clearOutput):
+        * inspector/debugger/breakpoint-action-eval.html:
+        * inspector/debugger/breakpoint-action-log-expected.txt:
+        * inspector/debugger/breakpoint-action-log.html:
+        * inspector/debugger/probe-manager-add-remove-actions-expected.txt:
+        * inspector/debugger/probe-manager-add-remove-actions.html:
+
 2019-10-23  Sihui Liu  <sihui_...@apple.com>
 
         [ Mac WK1 ] REGRESSION (r251261): Layout Test inspector/console/webcore-logging.html is consistently Failing

Modified: trunk/LayoutTests/http/tests/inspector/resources/inspector-test.js (251484 => 251485)


--- trunk/LayoutTests/http/tests/inspector/resources/inspector-test.js	2019-10-23 18:49:37 UTC (rev 251484)
+++ trunk/LayoutTests/http/tests/inspector/resources/inspector-test.js	2019-10-23 19:09:04 UTC (rev 251485)
@@ -173,6 +173,16 @@
 
 TestPage.log = TestPage.addResult;
 
+TestPage.clearOutput = function()
+{
+    if (!this._resultElement)
+        return;
+
+    let output = this._resultElement;
+    while (output.firstChild)
+        output.removeChild(output.firstChild);
+}
+
 TestPage.dispatchEventToFrontend = function(eventName, data)
 {
     let dispatchEventCodeString = `InspectorTest.dispatchEventToListeners(${JSON.stringify(eventName)}, ${JSON.stringify(data)});`;

Modified: trunk/LayoutTests/inspector/debugger/breakpoint-action-eval.html (251484 => 251485)


--- trunk/LayoutTests/inspector/debugger/breakpoint-action-eval.html	2019-10-23 18:49:37 UTC (rev 251484)
+++ trunk/LayoutTests/inspector/debugger/breakpoint-action-eval.html	2019-10-23 19:09:04 UTC (rev 251485)
@@ -19,6 +19,7 @@
 
 function test()
 {
+    let breakpointPromise = new WI.WrappedPromise;
     WI.debuggerManager.addEventListener(WI.DebuggerManager.Event.ScriptAdded, function(event) {
         var scriptObject = event.data.script;
 
@@ -33,10 +34,19 @@
 
         WI.debuggerManager.addBreakpoint(breakpoint);
 
+        breakpointPromise.resolve()
+    });
+
+    let reloadPromise = InspectorTest.awaitEvent(FrontendTestHarness.Event.TestPageDidLoad);
+    InspectorTest.reloadPage();
+    Promise.all([
+        reloadPromise,
+        breakpointPromise.promise
+    ])
+    .then(() => {
         InspectorTest.evaluateInPage("runBreakpointActions()");
     });
 
-    InspectorTest.reloadPage();
 }
 </script>
 </head>

Modified: trunk/LayoutTests/inspector/debugger/breakpoint-action-log-expected.txt (251484 => 251485)


--- trunk/LayoutTests/inspector/debugger/breakpoint-action-log-expected.txt	2019-10-23 18:49:37 UTC (rev 251484)
+++ trunk/LayoutTests/inspector/debugger/breakpoint-action-log-expected.txt	2019-10-23 19:09:04 UTC (rev 251485)
@@ -10,8 +10,8 @@
 CONSOLE MESSAGE: line 1: 42 and {"x":1,"y":2}
 Testing that "Log" breakpoint actions work correctly.
 
-inside breakpointActions a:(42) b:([object Object])
 
 == Running test suite: Debugger.BreakpointAction.Log
 -- Running test case: TemplateLiteralPlaceholders
+inside breakpointActions a:(42) b:([object Object])
 

Modified: trunk/LayoutTests/inspector/debugger/breakpoint-action-log.html (251484 => 251485)


--- trunk/LayoutTests/inspector/debugger/breakpoint-action-log.html	2019-10-23 18:49:37 UTC (rev 251484)
+++ trunk/LayoutTests/inspector/debugger/breakpoint-action-log.html	2019-10-23 19:09:04 UTC (rev 251485)
@@ -17,6 +17,7 @@
         name: "TemplateLiteralPlaceholders",
         description: "Evaluate breakpoint with logging actions that include placeholders.",
         test(resolve, reject) {
+            let breakpointPromise = new WI.WrappedPromise;
             WI.debuggerManager.addEventListener(WI.DebuggerManager.Event.ScriptAdded, (event) => {
                 let scriptObject = event.data.script;
                 if (!/script\-for\-breakpoint\-actions\.js$/.test(scriptObject.url))
@@ -42,11 +43,19 @@
                 addLogAction("${a} and ${JSON.stringify(b)}");
 
                 WI.debuggerManager.addBreakpoint(breakpoint);
-
-                InspectorTest.evaluateInPage("runBreakpointActions()", resolve);
+                breakpointPromise.resolve();
             });
 
+            let reloadPromise = InspectorTest.awaitEvent(FrontendTestHarness.Event.TestPageDidLoad);
             InspectorTest.reloadPage();
+            Promise.all([
+                reloadPromise,
+                breakpointPromise.promise
+            ])
+            .then(() => {
+                return InspectorTest.evaluateInPage("runBreakpointActions()");
+            })
+            .then(resolve);;
         }
     });
 

Modified: trunk/LayoutTests/inspector/debugger/probe-manager-add-remove-actions-expected.txt (251484 => 251485)


--- trunk/LayoutTests/inspector/debugger/probe-manager-add-remove-actions-expected.txt	2019-10-23 18:49:37 UTC (rev 251484)
+++ trunk/LayoutTests/inspector/debugger/probe-manager-add-remove-actions-expected.txt	2019-10-23 19:09:04 UTC (rev 251485)
@@ -1,6 +1,5 @@
 Testing that the probe manager properly handles addition and removal of related probes.
 
-inside breakpointActions a:(12) b:([object Object])
 Breakpoint autocontinue state changed: true
 Probe set was added. New count: 1
 Probe with identifier 1 was added to probe set.
@@ -12,6 +11,7 @@
 Breakpoint was added.
 PASS: Breakpoint should be resolved.
 Breakpoint resolved state changed: true
+inside breakpointActions a:(12) b:([object Object])
 Probe with identifier 1 added sample: [object Object]
 Hit test checkpoint event #0 with type: probe-object-sample-added
 Probe with identifier 2 added sample: [object Object]

Modified: trunk/LayoutTests/inspector/debugger/probe-manager-add-remove-actions.html (251484 => 251485)


--- trunk/LayoutTests/inspector/debugger/probe-manager-add-remove-actions.html	2019-10-23 18:49:37 UTC (rev 251484)
+++ trunk/LayoutTests/inspector/debugger/probe-manager-add-remove-actions.html	2019-10-23 19:09:04 UTC (rev 251485)
@@ -7,6 +7,7 @@
 <script>
 function test()
 {
+    let breakpointPromise = new WI.WrappedPromise;
     WI.Frame.addEventListener(WI.Frame.Event.MainResourceDidChange, function() {
         InspectorTest.startTracingBreakpoints();
         InspectorTest.startTracingProbes();
@@ -44,11 +45,19 @@
             });
 
 
-            InspectorTest.evaluateInPage("breakpointActions(12, {x:1,y:2})");
+            breakpointPromise.resolve();
           });
     });
 
+    let reloadPromise = InspectorTest.awaitEvent(FrontendTestHarness.Event.TestPageDidLoad);
     InspectorTest.reloadPage();
+    Promise.all([
+        reloadPromise,
+        breakpointPromise.promise
+    ])
+    .then(() => {
+        InspectorTest.evaluateInPage("breakpointActions(12, {x:1,y:2})");
+    });
 }
 </script>
 </head>

Modified: trunk/Source/WebInspectorUI/ChangeLog (251484 => 251485)


--- trunk/Source/WebInspectorUI/ChangeLog	2019-10-23 18:49:37 UTC (rev 251484)
+++ trunk/Source/WebInspectorUI/ChangeLog	2019-10-23 19:09:04 UTC (rev 251485)
@@ -1,3 +1,24 @@
+2019-10-23  Yury Semikhatsky  <yu...@chromium.org>
+
+        Web Inspector: frontend tests should clear output before resending results
+        https://bugs.webkit.org/show_bug.cgi?id=203262
+
+        Reviewed by Devin Rousso.
+
+        Inspector front-end tests will clear output log before resending teset results. This avoids
+        race between InspectorTest.testPageDidLoad event and TestPage.addResult calls that may have
+        already be sent to the new page after navigation. The latter events otherwise would be added
+        twice.
+
+        * UserInterface/Test/FrontendTestHarness.js:
+        (FrontendTestHarness):
+        (FrontendTestHarness.prototype.testPageDidLoad):
+        (FrontendTestHarness.prototype.reloadPage):
+        (FrontendTestHarness.prototype.reportUnhandledRejection):
+        (FrontendTestHarness.prototype.reportUncaughtException):
+        (FrontendTestHarness.prototype._resendResults): Don't resend the results when the page is loaded
+        first time.
+
 2019-10-22  Devin Rousso  <drou...@apple.com>
 
         Web Inspector: Sources: keep the function/object name sticky in the object preview popover

Modified: trunk/Source/WebInspectorUI/UserInterface/Test/FrontendTestHarness.js (251484 => 251485)


--- trunk/Source/WebInspectorUI/UserInterface/Test/FrontendTestHarness.js	2019-10-23 18:49:37 UTC (rev 251484)
+++ trunk/Source/WebInspectorUI/UserInterface/Test/FrontendTestHarness.js	2019-10-23 19:09:04 UTC (rev 251485)
@@ -30,7 +30,7 @@
         super();
 
         this._results = [];
-        this._shouldResendResults = true;
+        this._testPageHasLoaded = false;
 
         // Options that are set per-test for debugging purposes.
         this.dumpActivityToSystemConsole = false;
@@ -133,7 +133,10 @@
             InspectorFrontendHost.unbufferedLog("testPageDidLoad()");
 
         this._testPageIsReloading = false;
-        this._resendResults();
+        if (this._testPageHasLoaded)
+            this._resendResults();
+        else
+            this._testPageHasLoaded = true;
 
         this.dispatchEventToListeners(FrontendTestHarness.Event.TestPageDidLoad);
 
@@ -155,7 +158,6 @@
         let target = WI.assumingMainTarget();
         return target.PageAgent.reload.invoke({ignoreCache, revalidateAllResources})
             .then(() => {
-                this._shouldResendResults = true;
                 this._testPageReloadedOnce = true;
 
                 return Promise.resolve(null);
@@ -252,7 +254,7 @@
 
         // If the connection to the test page is not set up, then just dump to console and give up.
         // Errors encountered this early can be debugged by loading Test.html in a normal browser page.
-        if (this._originalConsole && !this._testPageHasLoaded())
+        if (this._originalConsole && !this._testPageHasLoaded)
             this._originalConsole["error"](result);
 
         this.addResult(result);
@@ -288,7 +290,7 @@
 
         // If the connection to the test page is not set up, then just dump to console and give up.
         // Errors encountered this early can be debugged by loading Test.html in a normal browser page.
-        if (this._originalConsole && !this._testPageHasLoaded())
+        if (this._originalConsole && !this._testPageHasLoaded)
             this._originalConsole["error"](result);
 
         this.addResult(result);
@@ -299,19 +301,14 @@
 
     // Private
 
-    _testPageHasLoaded()
-    {
-        return self._shouldResendResults;
-    }
-
     _resendResults()
     {
-        console.assert(this._shouldResendResults);
-        this._shouldResendResults = false;
+        console.assert(this._testPageHasLoaded);
 
         if (this.dumpActivityToSystemConsole)
             InspectorFrontendHost.unbufferedLog("_resendResults()");
 
+        this.evaluateInPage("TestPage.clearOutput()");
         for (let result of this._results)
             this.evaluateInPage(`TestPage.addResult(unescape("${escape(result)}"))`);
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to