[Webkit-unassigned] [Bug 237708] [GTK] Add a unit test to check the remote inspector HTTP server
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Mar 11 01:31:35 PST 2022
https://bugs.webkit.org/show_bug.cgi?id=237708
--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 454340
--> https://bugs.webkit.org/attachment.cgi?id=454340
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=454340&action=review
>> Tools/TestWebKitAPI/Tests/WebKitGtk/TestInspectorServer.cpp:67
>> + g_printerr("Failed to connect to inspector server");
>
> This one actually does need to end in \n
Ok.
>> Tools/TestWebKitAPI/Tests/WebKitGtk/TestInspectorServer.cpp:105
>> + }, m_mainLoop);
>
> Hmm, what is the purpose of this? You are trying to fail the test if it takes more than 2.5 seconds for the server to start? I would either use a way larger timeout -- say 30 seconds, to match the standard D-Bus timeout -- or else remove it altogether and let the test time out if it's broken. Otherwise, I would be afraid the test could be flaky if the system is under heavy load.
>
> I see you've already taken care to retry the connection every 100ms and quit immediately if it succeeds, which is good.
I think this was needed before because the server was started in beforeAll() and the test runner handles the timeout per test. Now that the server is started/stopped as part of the test setup/teardown we can probably remove this and let the runner handle the timeout.
>> Tools/TestWebKitAPI/Tests/WebKitGtk/TestInspectorServer.cpp:228
>> + signal(SIGABRT, +[](int) {
>
> Since you're only catching SIGABRT, I would say "if the parent dies because of an assertion failure."
>
> Alternative: you might try prctl(PR_SET_PDEATHSIG) and see if that works, then the child should die even if the parent dies to something other than SIGABRT. But I've seen PR_SET_PDEATHSIG mysteriously fail, so maybe make sure it works for you if you decide to try this.
Yes, I think the idea here was to kill the server when the test fails, not when it crashes.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220311/a4c2327b/attachment-0001.htm>
More information about the webkit-unassigned
mailing list