Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Glib] Initialize WebProcessExtensions in API tests on demand #16828

Conversation

obyknovenius
Copy link
Contributor

@obyknovenius obyknovenius commented Aug 18, 2023

f7d2305

[Glib] Initialize `WebProcessExtension`s in API tests on demand
https://bugs.webkit.org/show_bug.cgi?id=260382

Reviewed by Carlos Garcia Campos and Michael Catanzaro.

Currently, `WebProcessExtension`s are initialized for each API test.
It's not only unnecessary but can lead to flakiness.

This patch requires API tests using extensions to explicitly state this
by setting `Test::shouldInitializeWebProcessExtensions` to `true`.

* Tools/TestWebKitAPI/Tests/WebKitGLib/TestConsoleMessage.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestDOMElement.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestEditor.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestFrame.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestMultiprocess.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestResources.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitUserContentManager.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebProcessExtensions.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestAutocleanups.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestContextMenu.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestDOMClientRect.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestDOMNode.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestDOMNodeFilter.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestDOMXPathNSResolver.cpp:
(beforeAll):
* Tools/TestWebKitAPI/glib/WebKitGLib/TestMain.cpp:
* Tools/TestWebKitAPI/glib/WebKitGLib/TestMain.h:
(Test::Test):

Canonical link: https://commits.webkit.org/267125@main

30eaf31

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@obyknovenius obyknovenius requested a review from a team as a code owner August 18, 2023 10:35
@obyknovenius obyknovenius self-assigned this Aug 18, 2023
@obyknovenius obyknovenius added the WebKitGTK Bugs related to the Gtk API layer. label Aug 18, 2023
@mcatanzaro
Copy link
Contributor

I agree this change is fine, but I wish we understood how it could cause flakiness. It certainly shouldn't.

@obyknovenius
Copy link
Contributor Author

obyknovenius commented Aug 18, 2023

I can give an example. When a test finishes fast and we're stopping the DBus server, an extension can still be trying to connect to it and we're getting a failing assertion here:

g_assert(G_IS_DBUS_CONNECTION(connection));

I faced it in WebKitGTK/TestWebKitWebContext test but only in debug build with an error that looked totally unrelated.

@obyknovenius obyknovenius force-pushed the api-tests/glib/initialize-web-process-extensions-on-demand branch from a57507b to 7f9058a Compare August 21, 2023 13:07
@obyknovenius obyknovenius force-pushed the api-tests/glib/initialize-web-process-extensions-on-demand branch from 7f9058a to 30eaf31 Compare August 21, 2023 16:37
@obyknovenius obyknovenius added the merge-queue Applied to send a pull request to merge-queue label Aug 22, 2023
https://bugs.webkit.org/show_bug.cgi?id=260382

Reviewed by Carlos Garcia Campos and Michael Catanzaro.

Currently, `WebProcessExtension`s are initialized for each API test.
It's not only unnecessary but can lead to flakiness.

This patch requires API tests using extensions to explicitly state this
by setting `Test::shouldInitializeWebProcessExtensions` to `true`.

* Tools/TestWebKitAPI/Tests/WebKitGLib/TestConsoleMessage.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestDOMElement.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestEditor.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestFrame.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestMultiprocess.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestResources.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitUserContentManager.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebProcessExtensions.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestAutocleanups.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestContextMenu.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestDOMClientRect.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestDOMNode.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestDOMNodeFilter.cpp:
(beforeAll):
* Tools/TestWebKitAPI/Tests/WebKitGtk/TestDOMXPathNSResolver.cpp:
(beforeAll):
* Tools/TestWebKitAPI/glib/WebKitGLib/TestMain.cpp:
* Tools/TestWebKitAPI/glib/WebKitGLib/TestMain.h:
(Test::Test):

Canonical link: https://commits.webkit.org/267125@main
@webkit-commit-queue webkit-commit-queue force-pushed the api-tests/glib/initialize-web-process-extensions-on-demand branch from 30eaf31 to f7d2305 Compare August 22, 2023 08:21
@webkit-commit-queue
Copy link
Collaborator

Committed 267125@main (f7d2305): https://commits.webkit.org/267125@main

Reviewed commits have been landed. Closing PR #16828 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit f7d2305 into WebKit:main Aug 22, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 22, 2023
@obyknovenius obyknovenius deleted the api-tests/glib/initialize-web-process-extensions-on-demand branch August 23, 2023 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKitGTK Bugs related to the Gtk API layer.
Projects
None yet
6 participants