Bug 215848 – Remove console logs from expected.txt files for some WebSocket tests

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[youenn fablet]

Some WebSocket tests show platform specific console log messages.

[youenn fablet]

Created attachment 407288 [details]
Patch

[Alex Christensen]

Comment on attachment 407288 [details]
Patch

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

> LayoutTests/ChangeLog:9
> +        By removing them, we can keep one expected.txt file for all platforms.

I don't see removal of expected.txt files in this patch.  What platforms and what tests?
This does reduce useful test behavior checking.  Why is that worth it?

[youenn fablet]

> I don't see removal of expected.txt files in this patch.  What platforms and
> what tests?

There is no removal for now but this would allow keeping the same expected file for GTK, macOS Catalina, MacOS BigSur with NSURLSession code path and iOS.

I believe several GTK/GLib web sockets expected.txt files could be removed.
See for instance LayoutTests/http/tests/websocket/tests/hybi/long-control-frame-expected.txt and LayoutTests/platform/glib/http/tests/websocket/tests/hybi/long-control-frame-expected.txt.
I haven't done it since there is no GTK EWS bot.
When we enable NSURLSession code path, this will allow to not create specific expectations for BigSur/iOS.

> This does reduce useful test behavior checking.  Why is that worth it?

The benefit of removing this logging is that we keep it easy to manage the inconsistencies between all the platforms.
If we do a change to WebSocket legacy code path, it will be super easy to see whether it is consistent with GTK or NSURLSession code paths.

The benefit of keeping the console logging is mostly that we can validate that the test is testing what it is supposed to be testing. But it does not bring much coverage on the source code in itself.

[Alex Christensen]

Is there a reason we can't just make the logs for the NSURLSession and CFStream code paths line up?

[youenn fablet]

(In reply to Alex Christensen from comment #4)
> Is there a reason we can't just make the logs for the NSURLSession and
> CFStream code paths line up?

That would be difficult. CFStream code path logging is usually more precise, but not always.

Some examples:
-CONSOLE MESSAGE: WebSocket connection to 'ws://localhost:8880/websocket/tests/hybi/interleaved-fragments' failed: Received new data frame but previous continuous frame is unfinished.
+CONSOLE MESSAGE: WebSocket connection to 'ws://localhost:8880/websocket/tests/hybi/interleaved-fragments' failed: The operation couldn’t be completed. (kNWErrorDomainPOSIX error 57 - Socket is not connected)

-CONSOLE MESSAGE: WebSocket network error: The operation couldn’t be completed. Connection refused
+CONSOLE MESSAGE: WebSocket connection to 'ws://localhost/' failed: Could not connect to the server.

-CONSOLE MESSAGE: WebSocket connection to 'ws://127.0.0.1:8880/websocket/tests/hybi/broken-utf8' failed: Could not decode a text frame as UTF-8.
+CONSOLE MESSAGE: WebSocket connection to 'ws://127.0.0.1:8880/websocket/tests/hybi/broken-utf8' failed: The operation couldn’t be completed. (kNWErrorDomainPOSIX error 96 - No message available on STREAM)

-CONSOLE MESSAGE: WebSocket connection to 'ws://localhost:8880/websocket/tests/hybi/deflate-frame-invalid-parameter?x-foo' failed: Received unexpected deflate-frame parameter
+CONSOLE MESSAGE: WebSocket connection to 'ws://localhost:8880/websocket/tests/hybi/deflate-frame-invalid-parameter?x-foo' failed: There was a bad response from the server.

-CONSOLE MESSAGE: WebSocket connection to 'ws://localhost:8880/websocket/tests/hybi/handshake-fail-by-extensions-header' failed: Received unexpected extension: x-foo
+CONSOLE MESSAGE: WebSocket connection to 'ws://localhost:8880/websocket/tests/hybi/handshake-fail-by-extensions-header' failed: There was a bad response from the server.

[Alex Christensen]

Comment on attachment 407288 [details]
Patch

Ah, ok.  Please try to remove any identical files, at least.

[youenn fablet]

Created attachment 407387 [details]
Patch for landing

[youenn fablet]

(In reply to Alex Christensen from comment #6)
> Comment on attachment 407288 [details]
> Patch
> 
> Ah, ok.  Please try to remove any identical files, at least.

I removed some of them from glib.

Lauro, hopefully this will not cause regression on your side.
There might be other glib specific -expected.txt files that could be removed after this patch.

[EWS]

Committed r266230: <https://trac.webkit.org/changeset/266230>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407387 [details].

[Radar WebKit Bug Importer]

<rdar://problem/67864206>

[Lauro Moura]

(In reply to youenn fablet from comment #8)
> (In reply to Alex Christensen from comment #6)
> > Comment on attachment 407288 [details]
> > Patch
> > 
> > Ah, ok.  Please try to remove any identical files, at least.
> 
> I removed some of them from glib.
> 
> Lauro, hopefully this will not cause regression on your side.
> There might be other glib specific -expected.txt files that could be removed
> after this patch.

Sorry for not responding earlier.

We're still getting some glib/soup-specific messages, like:

--- /home/buildbot/worker/gtk-linux-64-release-tests/build/layout-test-results/http/tests/websocket/tests/hybi/long-control-frame-expected.txt
+++ /home/buildbot/worker/gtk-linux-64-release-tests/build/layout-test-results/http/tests/websocket/tests/hybi/long-control-frame-actual.txt
@@ -1,4 +1,3 @@
-CONSOLE MESSAGE: WebSocket connection to 'ws://localhost:8880/websocket/tests/hybi/long-control-frame' failed: Received invalid WebSocket response from the server
 Test whether WebSocket rejects control frames longer than 125 bytes.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


I'll add back the glib baselines where only the error messages differ (i.e. no PASS/FAIL diffs against the baseline)

Regading GTK EWS, there is layout test builder in EWS-UAT: https://ews-build.webkit-uat.org/#/builders/34

We're working on adding more workers and stabilizing the flakies so it can be moved to the main queue.

[Lauro Moura]

(In reply to Lauro Moura from comment #11)
> (In reply to youenn fablet from comment #8)
> > (In reply to Alex Christensen from comment #6)
> > > Comment on attachment 407288 [details]
> > > Patch
> > > 
> > > Ah, ok.  Please try to remove any identical files, at least.
> > 
> > I removed some of them from glib.
> > 
> > Lauro, hopefully this will not cause regression on your side.
> > There might be other glib specific -expected.txt files that could be removed
> > after this patch.
> 
> Sorry for not responding earlier.
> 
> We're still getting some glib/soup-specific messages, like:
> 
> ---
> /home/buildbot/worker/gtk-linux-64-release-tests/build/layout-test-results/
> http/tests/websocket/tests/hybi/long-control-frame-expected.txt
> +++
> /home/buildbot/worker/gtk-linux-64-release-tests/build/layout-test-results/
> http/tests/websocket/tests/hybi/long-control-frame-actual.txt
> @@ -1,4 +1,3 @@
> -CONSOLE MESSAGE: WebSocket connection to
> 'ws://localhost:8880/websocket/tests/hybi/long-control-frame' failed:
> Received invalid WebSocket response from the server
>  Test whether WebSocket rejects control frames longer than 125 bytes.
>  
>  On success, you will see a series of "PASS" messages, followed by "TEST
> COMPLETE".
> 
> 
> I'll add back the glib baselines where only the error messages differ (i.e.
> no PASS/FAIL diffs against the baseline)
> 
> Regading GTK EWS, there is layout test builder in EWS-UAT:
> https://ews-build.webkit-uat.org/#/builders/34
> 
> We're working on adding more workers and stabilizing the flakies so it can
> be moved to the main queue.

Nevermind, I misread the diffs from the bots. I just need to update the rebaselines of failing tests removing the messages.

[Lauro Moura]

Rebaselined the remaining glib files in r266274. Sorry for the noise.